This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH 2/4] GAS: Make new fake labels when cloning a symbol
- From: "Maciej W. Rozycki" <macro at codesourcery dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: binutils at sourceware dot org, Catherine Moore <clm at codesourcery dot com>, gnu-mips-sgxx at codesourcery dot com
- Date: Fri, 27 Aug 2010 10:28:32 +0100 (BST)
- Subject: Re: [PATCH 2/4] GAS: Make new fake labels when cloning a symbol
- References: <alpine.DEB.1.10.1007241721120.29495@tp.orcam.me.uk> <87wrscdo34.fsf@firetop.home>
On Sat, 31 Jul 2010, Richard Sandiford wrote:
> Unfortunately, keying off FAKE_LABEL_NAME isn't enough to prove that
> that the symbol is ".". FAKE_LABEL_NAME is also used in the DWARF
> machinery, and to hold the result of nested expressions.
Tricky. ;)
> It's a bit
> of a convoluted example, but after the patch:
>
> .set noreorder
> .eqv foo,(x+bar)-frob
> .set bar,0
> .set frob,0
> x:
> nop
> b foo
> nop
> nop
> b foo
> nop
>
> gives:
>
> 00000000 <x>:
> 0: 00000000 nop
> 4: 1000ffff b 4 <x+0x4>
> 8: 00000000 nop
> c: 00000000 nop
> 10: 1000ffff b 10 <x+0x10>
> 14: 00000000 nop
Correct. :(
> Also, as far as:
>
> - if (symbolP->bsym->section == expr_section && !symbolP->sy_resolving)
> + if (!symbolP->sy_resolving)
>
> goes, I'm not sure that we really should be walking through other
> _non-eqv_ symbol definitions.
Fair enough.
> E.g. after the patch:
>
> .set noreorder
> .eqv foo,bar
> .eqv baz,.
> .set bar,baz
> x:
> nop
> b foo
> nop
> .set bar,baz
> nop
> b foo
> nop
>
> assembles as:
>
> 00000000 <baz>:
> 0: 00000000 nop
> 4: 1000ffff b 4 <baz+0x4>
> 8: 00000000 nop
>
> 0000000c <bar>:
> c: 00000000 nop
> 10: 1000ffff b 10 <bar+0x4>
> 14: 00000000 nop
>
> whereas the branches should be to 0x0 and 0xc respectively.
Correct again. :(
> I'm not saying the current code's right, of course. I agree that:
>
> .set noreorder
> .eqv foo,bar
> .eqv bar,baz
> .set baz,.
> nop
> b foo
> nop
> .set baz,.
> nop
> b foo
> nop
>
> ought to be the same as the previous example, and at the moment it isn't.
> I just think the recursiveness should be limited to expressions and
> forward references. It might help if we split the problem into two:
> fixing the chained-.eqv evaluation, and fixing "." references.
I have split the patch into two. While that doesn't seem particularly
useful here (and caused me some hassle with development), it fulfils the
principle of one logical change per patch.
I made further analysis of what's going on here and took conclusions from
this observation: symbol_clone_if_forward_ref() is called both when an
equated symbol is defined and when it is referred to. The conclusion is
all the cloning must only be made whenever the symbol is referred to and
not when it's defined. This comes from the definition of the equation
operation.
Taking the conclusion and your concerns into account I have come up with
the below. It works for all the three cases plus this one:
.data
.word 0
.eqv foobar, fnord + 4
.eqv foobaz, foobar - 16
.set fnord, .
.word fnord
.set fnord, .
.word fnord
.set fnord, .
.word foobaz
.set fnord, .
.word foobaz
It has passed regression testing with mips-sde-elf too. If we agree on
this change, then I'll see how to integrate the tests into the testsuite
-- it'll be worth having all of them, as generic ones if possible too.
Comments?
2010-08-27 Maciej W. Rozycki <macro@codesourcery.com>
gas/
* symbols.c (symbol_clone_if_forward_ref): Add is_deferred
argument. Clone forwarded symbols too, when referenced.
* symbols.h (symbol_clone_if_forward_ref): Update prototype
and macro definition accordingly.
* expr.c (operand): Let symbol_clone_if_forward_ref know if
a deferred or actual reference is being made.
2010-08-27 Maciej W. Rozycki <macro@codesourcery.com>
gas/
* symbols.c (symbol_clone_if_forward_ref): Make fresh fake
labels for dot special symbol references.
* write.c (TC_FAKE_LABEL): Move over to...
* write.h (TC_FAKE_LABEL): ... here.
Maciej
binutils-gas-eqv.diff
Index: binutils-fsf-trunk-quilt/gas/symbols.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/symbols.c 2010-08-26 23:31:55.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/symbols.c 2010-08-27 02:03:42.000000000 +0100
@@ -622,7 +622,7 @@ symbol_clone (symbolS *orgsymP, int repl
#undef symbol_clone_if_forward_ref
symbolS *
-symbol_clone_if_forward_ref (symbolS *symbolP, int is_forward)
+symbol_clone_if_forward_ref (symbolS *symbolP, int is_deferred, int is_forward)
{
if (symbolP && !LOCAL_SYMBOL_CHECK (symbolP))
{
@@ -645,18 +645,23 @@ symbol_clone_if_forward_ref (symbolS *sy
/* Re-using sy_resolving here, as this routine cannot get called from
symbol resolution code. */
- if (symbolP->bsym->section == expr_section && !symbolP->sy_resolving)
+ if ((symbolP->bsym->section == expr_section
+ || (!is_deferred && symbolP->sy_forward_ref))
+ && !symbolP->sy_resolving)
{
symbolP->sy_resolving = 1;
- add_symbol = symbol_clone_if_forward_ref (add_symbol, is_forward);
- op_symbol = symbol_clone_if_forward_ref (op_symbol, is_forward);
+ add_symbol = symbol_clone_if_forward_ref (add_symbol, 0, is_forward);
+ op_symbol = symbol_clone_if_forward_ref (op_symbol, 0, is_forward);
symbolP->sy_resolving = 0;
}
if (symbolP->sy_forward_ref
|| add_symbol != symbolP->sy_value.X_add_symbol
|| op_symbol != symbolP->sy_value.X_op_symbol)
- symbolP = symbol_clone (symbolP, 0);
+ {
+ symbolP = symbol_clone (symbolP, 0);
+ symbolP->sy_resolving = 0;
+ }
symbolP->sy_value.X_add_symbol = add_symbol;
symbolP->sy_value.X_op_symbol = op_symbol;
Index: binutils-fsf-trunk-quilt/gas/expr.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/expr.c 2010-08-26 23:31:55.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/expr.c 2010-08-27 00:05:40.000000000 +0100
@@ -1366,8 +1366,12 @@ operand (expressionS *expressionP, enum
if (expressionP->X_add_symbol)
symbol_mark_used (expressionP->X_add_symbol);
- expressionP->X_add_symbol = symbol_clone_if_forward_ref (expressionP->X_add_symbol);
- expressionP->X_op_symbol = symbol_clone_if_forward_ref (expressionP->X_op_symbol);
+ expressionP->X_add_symbol
+ = symbol_clone_if_forward_ref (expressionP->X_add_symbol,
+ mode == expr_defer);
+ expressionP->X_op_symbol
+ = symbol_clone_if_forward_ref (expressionP->X_op_symbol,
+ mode == expr_defer);
switch (expressionP->X_op)
{
Index: binutils-fsf-trunk-quilt/gas/symbols.h
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/symbols.h 2010-08-26 23:31:55.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/symbols.h 2010-08-27 00:05:40.000000000 +0100
@@ -51,8 +51,8 @@ symbolS *symbol_create (const char *name
fragS * frag);
symbolS *symbol_clone (symbolS *, int);
#undef symbol_clone_if_forward_ref
-symbolS *symbol_clone_if_forward_ref (symbolS *, int);
-#define symbol_clone_if_forward_ref(s) symbol_clone_if_forward_ref (s, 0)
+symbolS *symbol_clone_if_forward_ref (symbolS *, int, int);
+#define symbol_clone_if_forward_ref(s, d) symbol_clone_if_forward_ref (s, d, 0)
symbolS *symbol_temp_new (segT, valueT, fragS *);
symbolS *symbol_temp_new_now (void);
symbolS *symbol_temp_make (void);
binutils-gas-dot.diff
Index: binutils-fsf-trunk-quilt/gas/symbols.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/symbols.c 2010-08-27 02:24:02.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/symbols.c 2010-08-27 03:02:23.000000000 +0100
@@ -659,8 +659,29 @@ symbol_clone_if_forward_ref (symbolS *sy
|| add_symbol != symbolP->sy_value.X_add_symbol
|| op_symbol != symbolP->sy_value.X_op_symbol)
{
+ symbolS *temp_symbol;
+ int is_temp_add;
+ int is_temp_op;
+
symbolP = symbol_clone (symbolP, 0);
symbolP->sy_resolving = 0;
+ if (!is_deferred)
+ {
+ is_temp_add = (add_symbol
+ && SEG_NORMAL (add_symbol->bsym->section)
+ && TC_FAKE_LABEL (S_GET_NAME (add_symbol)));
+ is_temp_op = (op_symbol
+ && SEG_NORMAL (op_symbol->bsym->section)
+ && TC_FAKE_LABEL (S_GET_NAME (op_symbol)));
+ if (is_temp_add || is_temp_op)
+ {
+ temp_symbol = symbol_temp_new_now ();
+ if (is_temp_add)
+ add_symbol = temp_symbol;
+ if (is_temp_op)
+ op_symbol = temp_symbol;
+ }
+ }
}
symbolP->sy_value.X_add_symbol = add_symbol;
Index: binutils-fsf-trunk-quilt/gas/write.c
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/write.c 2010-08-27 02:16:06.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/write.c 2010-08-27 02:24:05.000000000 +0100
@@ -102,10 +102,6 @@
#define MD_PCREL_FROM_SECTION(FIX, SEC) md_pcrel_from (FIX)
#endif
-#ifndef TC_FAKE_LABEL
-#define TC_FAKE_LABEL(NAME) (strcmp ((NAME), FAKE_LABEL_NAME) == 0)
-#endif
-
/* Positive values of TC_FX_SIZE_SLACK allow a target to define
fixups that far past the end of a frag. Having such fixups
is of course most most likely a bug in setting fx_size correctly.
Index: binutils-fsf-trunk-quilt/gas/write.h
===================================================================
--- binutils-fsf-trunk-quilt.orig/gas/write.h 2010-08-27 02:16:06.000000000 +0100
+++ binutils-fsf-trunk-quilt/gas/write.h 2010-08-27 02:24:05.000000000 +0100
@@ -29,6 +29,10 @@
#define FAKE_LABEL_NAME "L0\001"
#endif
+#ifndef TC_FAKE_LABEL
+#define TC_FAKE_LABEL(NAME) (strcmp ((NAME), FAKE_LABEL_NAME) == 0)
+#endif
+
#include "bit_fix.h"
/*