This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH][GAS][AARCH64]Emit DATA_MAP in order within text section, align code when transformed from MAP_DATA state into MAP_INSN state.
- From: Renlin Li <renlin dot li at arm dot com>
- To: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: Nicholas Clifton <nickc at redhat dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>
- Date: Fri, 27 Mar 2015 15:28:01 +0000
- Subject: [PATCH][GAS][AARCH64]Emit DATA_MAP in order within text section, align code when transformed from MAP_DATA state into MAP_INSN state.
- Authentication-results: sourceware.org; auth=none
Hi,
This is a patch doing three related things.
1), Emit MAP_DATA symbol for data in text section in order.
Previously, if a section start with data (MAP_UNDEFINED --> MAP_DATA),
this MAP_DATA symbol is not emitted until an instruction is processed.
Now mapping symbols will be emitted in order within text section.
It reverts the change made here:
https://sourceware.org/ml/binutils/2015-03/msg00331.html
In that change, data mapping symbols are emitted, but not in correct
order in a few cases. mapping_state_2 will be called while finishing
subsegs. A MAP_DATA will be emitted there if there is a postpone one.
The order it not correct, although there is no behavior difference.
for the following snippet:
.text
label1:
.long 1
label2:
the symbols dumped should be in the correct order: label1, $d, label2
Really sorry for the NOISE!
2), Align frag when the state is transformed from MAP_DATA to MAP_INSN,
in this
case, a new frag is created and the start address is 4 bytes aligned.
Previously, this is not considered. Thanks to Nick's explanation for .inst
directive.
for the following code snippet.
.text
.short 0xffff
ret
.short 0xffff
ret
Before the patch, when object dumped, the following code is generated:
0: ffff .short 0xffff
2: d65f03c0 ret
6: 1111 .short 0x1111
8: d65f03c0 ret
After the patch:
0: ffff .short 0xffff
2: 0000 .short 0x0000
4: d65f03c0 ret
8: 1111 .short 0x1111
a: 0000 .short 0x0000
c: d65f03c0 ret
3), Because now mapping symbols are emitted in order in text
section(because of the first change). I made the assumption that, when
the code is transformed from MAP_UNDEFINED --> MAP_INSN, it's alway
aligned(there is no pending MAP_DATA symbol anymore), we don't have to
aligned it again. So frag_align_code is called only during MAP_DATA -->
MAP_INSN state transition.
This fixes the bug mentioned here:
https://sourceware.org/ml/binutils/2015-03/msg00339.html
binutils and aarch64-none-elf regress tests run without new issues.
An arm one should also be prepared.
Okay to commit?
Regards,
Renlin Li
gas/ChangeLog:
2015-03-27 Renlin Li <renlin.li@arm.com>
* config/tc-aarch64.c (mapping_state): Emit MAP_DATA within text
section in order.
(mapping_state_2): Don't emit MAP_DATA here.
(s_aarch64_inst): Align frag during state transition.
(md_assemble): Likewise.
diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index ef4fae8..c813f1c 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -1467,7 +1467,6 @@ static void mapping_state_2 (enum mstate state, int max_chars);
/* Set the mapping state to STATE. Only call this when about to
emit some STATE bytes to the file. */
-#define TRANSITION(from, to) (mapstate == (from) && state == (to))
void
mapping_state (enum mstate state)
{
@@ -1484,9 +1483,25 @@ mapping_state (enum mstate state)
alignment. */
record_alignment (now_seg, 2);
- if (TRANSITION (MAP_UNDEFINED, MAP_DATA))
- /* This case will be evaluated later in the next else. */
+#define TRANSITION(from, to) (mapstate == (from) && state == (to))
+ if (TRANSITION (MAP_UNDEFINED, MAP_DATA) && now_seg != text_section)
+ /* Emit MAP_DATA within text section in order. Otherwise, it will be
+ evaluated later in the next else. */
return;
+ else if (TRANSITION (MAP_UNDEFINED, MAP_INSN))
+ {
+ /* Only add the symbol if the offset is > 0:
+ if we're at the first frag, check it's size > 0;
+ if we're not at the first frag, then for sure
+ the offset is > 0. */
+ struct frag *const frag_first = seg_info (now_seg)->frchainP->frch_root;
+ const int add_symbol = (frag_now != frag_first)
+ || (frag_now_fix () > 0);
+
+ if (add_symbol)
+ make_mapping_symbol (MAP_DATA, (valueT) 0, frag_first);
+ }
+#undef TRANSITION
mapping_state_2 (state, 0);
}
@@ -1507,24 +1522,9 @@ mapping_state_2 (enum mstate state, int max_chars)
There is nothing else to do. */
return;
- if (TRANSITION (MAP_UNDEFINED, MAP_INSN))
- {
- /* Only add the symbol if the offset is > 0:
- if we're at the first frag, check it's size > 0;
- if we're not at the first frag, then for sure
- the offset is > 0. */
- struct frag *const frag_first = seg_info (now_seg)->frchainP->frch_root;
- const int add_symbol = (frag_now != frag_first)
- || (frag_now_fix () > 0);
-
- if (add_symbol)
- make_mapping_symbol (MAP_DATA, (valueT) 0, frag_first);
- }
-
seg_info (now_seg)->tc_segment_info_data.mapstate = state;
make_mapping_symbol (state, (valueT) frag_now_fix () - max_chars, frag_now);
}
-#undef TRANSITION
#else
#define mapping_state(x) /* nothing */
#define mapping_state_2(x, y) /* nothing */
@@ -1867,8 +1867,15 @@ s_aarch64_inst (int ignored ATTRIBUTE_UNUSED)
return;
}
- if (!need_pass_2)
+ /* Sections are assumed to start aligned. In text section, there is no
+ MAP_DATA symbol pending. So we only align the address during
+ MAP_DATA --> MAP_INSN transition.
+ For other sections, this is not guaranteed, align it anyway. */
+ enum mstate mapstate = seg_info (now_seg)->tc_segment_info_data.mapstate;
+ if (!need_pass_2 && ((now_seg == text_section && mapstate == MAP_DATA)
+ || now_seg != text_section))
frag_align_code (2, 0);
+
#ifdef OBJ_ELF
mapping_state (MAP_INSN);
#endif
@@ -5710,6 +5717,15 @@ md_assemble (char *str)
dump_opcode_operands (opcode);
#endif /* DEBUG_AARCH64 */
+ /* Sections are assumed to start aligned. In text section, there is no
+ MAP_DATA symbol pending. So we only align the address during
+ MAP_DATA --> MAP_INSN transition.
+ For other sections, this is not guaranteed, align it anyway. */
+ enum mstate mapstate = seg_info (now_seg)->tc_segment_info_data.mapstate;
+ if (!need_pass_2 && ((now_seg == text_section && mapstate == MAP_DATA)
+ || now_seg != text_section))
+ frag_align_code (2, 0);
+
mapping_state (MAP_INSN);
inst_base = &inst.base;