[PATCH 4/4] MIPS/GAS: Propagate symbol attributes
Maciej W. Rozycki
macro@codesourcery.com
Mon Jul 26 10:48:00 GMT 2010
Hi,
There are several cases where ELF symbol attributes are not correctly set
for symbols leading to all kinds of odd side effects for MIPS16 code (and
with the upcoming change also for microMIPS code).
For example this program:
.text
foo:
xor $16, $17
.set fnord, . + 2
addu $2, $3, $4
xor $5, $6
bar:
subu $7, $16
assembles to this:
Disassembly of section .text:
00000000 <foo>:
0: e82e xor s0,s1
2: e389 addu v0,v1,a0
00000004 <fnord>:
4: edcee71f swc3 $14,-6369(t6)
00000006 <bar>:
6: e71f subu a3,s0
Notice how fnord's ELF attributes have not been set to indicate a MIPS16
symbol and the resulting confusion (this symbol, of course, if used as a
jump target will not set the ISA bit correctly). Similar symptoms are
seen with equated symbols (defined with .eqv) although the attributes are
lost at a different stage of assembly.
Here's a fix for these problems and a test case covering hopefully most
of them as well as those fixed by patches submitted previously in this series.
There are two new functions defined:
- mips_elf_copy_symbol_attributes() -- used for symbols defined with .set.
These, if calculated from a label that has been defined for the current
location (the "dot" special symbol being a prominent example, but any
label will actually do) will not have their ELF attributes set, because
the original label will only get them set once an instruction has been
emitted. A solution is to place the newly defined symbol on the list of
labels too.
- mips_elf_propagate_symbol_attributes() -- used for symbols defined with
.eqv. They are processed late and currently attributes are copied only
for exact copies of other symbols, i.e.:
.eqv foo, bar
but not:
.eqv foo, bar + 2
In the case of MIPS16 (and microMIPS) ELF attribute we want it to be
propagated even for offsetted symbols, hence the new hook in
resolve_symbol_value().
Finally I realised the uniqueness of the -mmips:16 option to `objdump'
that causes all the disassembled to be treated as MIPS16 code, regardless
of symbol annotations found. This actually covers the bugs addressed
here, hence I'm removing it. The implication is all MIPS16 tests need to
have a label at the beginning to set the ISA mode of the disassembler
correctly.
Perhaps the behaviour of the -mmips:16 option should be changed instead
so that it respects symbol annotations. I think it would make sense --
the option should normally only be needed for binary objects with no
sufficient symbol information. OTOH, the current behaviour is good in
case `objdump' gets confused because of a bug or a broken binary, hence I
have no strong preference actually towards making the change.
If we agree to make it after all, then the option can be added back. I
believe -mmips:micromips behaves the same. Additionally odd text symbols
with no ELF annotation are unconditionally treated as MIPS16 code. This
should probably be changed.
2010-07-26 Maciej W. Rozycki <macro@codesourcery.com>
gas/
* config/tc-mips.c (insn_label_recording): New variable.
(md_begin): Set insn_label_recording.
(md_mips_end): Clear insn_label_recording.
(mips_elf_copy_symbol_attributes): New function.
(mips_elf_propagate_symbol_attributes): New function.
* config/tc-mips.h (TC_COPY_SYMBOL_ATTRIBUTES): New macro.
(TC_PROPAGATE_SYMBOL_ATTRIBUTES): Likewise.
(mips_elf_copy_symbol_attributes): New prototype.
(mips_elf_propagate_symbol_attributes): Likewise.
* symbols.c (resolve_symbol_value): Call
TC_PROPAGATE_SYMBOL_ATTRIBUTES for symbols that are not exact
copies of originals.
gas/testsuite/
* gas/mips/branch-self.d: New test for various definitions of
labels.
* gas/mips/branch-self.s: Source for the new test.
* gas/mips/mips.exp: Run the new test. Remove -mmips:16 from
"mips16" architecture.
OK to apply?
Maciej
binutils-gas-propagate-attrs.diff
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.c 2010-07-25 03:52:42.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.c 2010-07-25 23:49:09.000000000 +0100
@@ -1254,6 +1254,8 @@ struct insn_label_list
static struct insn_label_list *free_insn_labels;
#define label_list tc_segment_info_data.labels
+static int insn_label_recording;
+
static void mips_clear_insn_labels (void);
static inline void
@@ -2093,11 +2095,15 @@ md_begin (void)
if (mips_fix_vr4120)
init_vr4120_conflicts ();
+
+ insn_label_recording = 1;
}
void
md_mips_end (void)
{
+ insn_label_recording = 0;
+
if (! ECOFF_DEBUGGING)
md_obj_end ();
}
@@ -14770,9 +14776,38 @@ mips_symbol_new_hook (symbolS *sym)
&& S_GET_VALUE (sym) == frag_now_fix ())
mips_record_label (sym);
}
-
+
#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
+/* Record copies of instruction labels made, for later MIPS16 symbol
+ annotation. */
+
+void
+mips_elf_copy_symbol_attributes (symbolS *dest, symbolS *src ATTRIBUTE_UNUSED)
+{
+ if (!IS_ELF || !insn_label_recording)
+ return;
+
+ if (S_GET_SEGMENT (src) == now_seg
+ && symbol_get_frag (src) == frag_now
+ && symbol_constant_p (src)
+ && S_GET_VALUE (src) == frag_now_fix ()
+ && symbol_constant_p (dest))
+ mips_record_label (dest);
+}
+
+/* Propagate MIPS16 symbol annotation. */
+
+void
+mips_elf_propagate_symbol_attributes (symbolS *dest, symbolS *src)
+{
+ if (!IS_ELF)
+ return;
+
+ if (ELF_ST_IS_MIPS16 (S_GET_OTHER (src)))
+ S_SET_OTHER (dest, ELF_ST_SET_MIPS16 (S_GET_OTHER (dest)));
+}
+
/* Some special processing for a MIPS ELF file. */
void
Index: binutils-fsf-trunk-quilt/gas/config/tc-mips.h
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/config/tc-mips.h 2010-07-25 03:35:59.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/config/tc-mips.h 2010-07-25 18:48:50.000000000 +0100
@@ -121,6 +121,14 @@ extern void mips_frob_file (void);
#if defined (OBJ_ELF) || defined (OBJ_MAYBE_ELF)
#define tc_frob_file_after_relocs mips_frob_file_after_relocs
extern void mips_frob_file_after_relocs (void);
+
+#define TC_COPY_SYMBOL_ATTRIBUTES(dest, src) \
+ mips_elf_copy_symbol_attributes (dest, src)
+extern void mips_elf_copy_symbol_attributes (symbolS *, symbolS *);
+
+#define TC_PROPAGATE_SYMBOL_ATTRIBUTES(dest, src) \
+ mips_elf_propagate_symbol_attributes (dest, src)
+extern void mips_elf_propagate_symbol_attributes (symbolS *, symbolS *);
#endif
#define tc_fix_adjustable(fixp) mips_fix_adjustable (fixp)
Index: binutils-fsf-trunk-quilt/gas/symbols.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/symbols.c 2010-07-25 03:35:59.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/symbols.c 2010-07-25 18:48:50.000000000 +0100
@@ -1182,12 +1182,19 @@ resolve_symbol_value (symbolS *symp)
break;
}
- if (finalize_syms && final_val == 0)
+ if (finalize_syms)
{
- if (LOCAL_SYMBOL_CHECK (add_symbol))
- add_symbol = local_symbol_convert ((struct local_symbol *)
- add_symbol);
- copy_symbol_attributes (symp, add_symbol);
+ if (final_val == 0)
+ {
+ if (LOCAL_SYMBOL_CHECK (add_symbol))
+ add_symbol = local_symbol_convert ((struct local_symbol *)
+ add_symbol);
+ copy_symbol_attributes (symp, add_symbol);
+ }
+#ifdef TC_PROPAGATE_SYMBOL_ATTRIBUTES
+ else
+ TC_PROPAGATE_SYMBOL_ATTRIBUTES (symp, add_symbol);
+#endif
}
/* If we have equated this symbol to an undefined or common
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-self.d
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-self.d 2010-07-25 03:56:22.000000000 +0100
@@ -0,0 +1,29 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS branches to self
+#as: -32
+#source: branch-self.s
+
+# Test various ways to request a branch to self.
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+([0-9a-f]+) <[^>]*> ac620000 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <.*>
+([0-9a-f]+) <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> ac620000 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <.*>
+([0-9a-f]+) <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> ac620000 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <.*>
+([0-9a-f]+) <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> ac620000 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <.*>
+([0-9a-f]+) <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> ac620000 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <.*>
+([0-9a-f]+) <[^>]*> 00000000 nop
+([0-9a-f]+) <[^>]*> ac620000 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 1000ffff b \1 <.*>
+([0-9a-f]+) <[^>]*> 00000000 nop
+ \.\.\.
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-self.s
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/branch-self.s 2010-07-25 03:56:22.000000000 +0100
@@ -0,0 +1,30 @@
+# Source file used to test branches to self.
+ .text
+foo:
+ sw $2, ($3)
+ b .
+
+ sw $2, ($3)
+0:
+ b 0b
+
+ sw $2, ($3)
+bar:
+ b bar
+
+ sw $2, ($3)
+ .set frob, .
+ b frob
+
+ .eqv fnord, .
+ sw $2, ($3)
+ b fnord
+
+ .eqv foobar, fnord + 4
+ .eqv foobaz, foobar - 16
+ sw $2, ($3)
+ b foobaz + 12
+
+# Force at least 8 (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .align 2
+ .space 8
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/testsuite/gas/mips/mips.exp 2010-07-25 03:35:59.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips.exp 2010-07-25 18:48:50.000000000 +0100
@@ -368,7 +368,7 @@ mips_arch_create mips64r2 64 mips64 { mi
{ -mmips:isa64r2 } \
{ mipsisa64r2-*-* mipsisa64r2el-*-* }
mips_arch_create mips16 32 {} {} \
- { -march=mips1 -mips16 } { -mmips:16 }
+ { -march=mips1 -mips16 } {}
mips_arch_create r3000 32 mips1 {} \
{ -march=r3000 -mtune=r3000 } { -mmips:3000 }
mips_arch_create r3900 32 mips1 { gpr_ilocks } \
@@ -448,6 +448,7 @@ if { [istarget mips*-*-vxworks*] } {
run_dump_test_arches "branch-misc-2pic-64" [mips_arch_list_matching mips3]
run_dump_test "branch-misc-3"
run_dump_test "branch-swap"
+ run_dump_test_arches "branch-self" [mips_arch_list_all]
run_dump_test "div"
if { !$addr32 } {
Index: binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips16@branch-self.d
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ binutils-fsf-trunk-quilt/gas/testsuite/gas/mips/mips16@branch-self.d 2010-07-25 03:56:22.000000000 +0100
@@ -0,0 +1,23 @@
+#objdump: -dr --prefix-addresses --show-raw-insn
+#name: MIPS branches to self
+#as: -32
+#source: branch-self.s
+
+# Test various ways to request a branch to self (MIPS16).
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+([0-9a-f]+) <[^>]*> db40 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 17ff b \1 <.*>
+([0-9a-f]+) <[^>]*> db40 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 17ff b \1 <.*>
+([0-9a-f]+) <[^>]*> db40 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 17ff b \1 <.*>
+([0-9a-f]+) <[^>]*> db40 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 17ff b \1 <.*>
+([0-9a-f]+) <[^>]*> db40 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 17ff b \1 <.*>
+([0-9a-f]+) <[^>]*> db40 sw v0,0\(v1\)
+([0-9a-f]+) <[^>]*> 17ff b \1 <.*>
+ \.\.\.
More information about the Binutils
mailing list