[binutils-gdb/binutils-2_34-branch] Fix tight loop on recursively-defined symbols

Alan Modra amodra@sourceware.org
Fri May 15 22:50:48 GMT 2020


https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5768460022b2928f1a5bd6d14dcc6176319a1c01

commit 5768460022b2928f1a5bd6d14dcc6176319a1c01
Author: Alan Modra <amodra@gmail.com>
Date:   Fri May 15 18:06:05 2020 +0930

    Fix tight loop on recursively-defined symbols
    
    This patch fixes a bug in GAS where the assembler enters a tight loop
    when attempting to resolve recursively-defined symbols, e.g. when
    trying to assemble "a=a".
    
    This is a regression introduced between binutils 2.32 and 2.33,
    by commit 1903f1385bff9
    
            * symbols.c (struct local_symbol): Update comment.
            (resolve_symbol_value): For resolved symbols equated to other
            symbols, verify that the referenced symbol is not a local_symbol
            before accessing sy_value.  Don't leave symbol loops during
            finalize_syms resolution.
            * testsuite/gas/all/assign-bad-recursive.d: New test.
            * testsuite/gas/all/assign-bad-recursive.l: Error output for test.
            * testsuite/gas/all/assign-bad-recursive.s: Assembly for test.
            * testsuite/gas/all/gas.exp: Run it.
    
    (cherry picked from commit 2a50b401465f74d7f3ee1654915b9070b4dc0fee)

Diff:
---
 gas/ChangeLog                                | 13 +++++++++++++
 gas/symbols.c                                | 18 +++++++++++++++---
 gas/testsuite/gas/all/assign-bad-recursive.d |  4 ++++
 gas/testsuite/gas/all/assign-bad-recursive.l |  7 +++++++
 gas/testsuite/gas/all/assign-bad-recursive.s |  8 ++++++++
 gas/testsuite/gas/all/gas.exp                |  1 +
 6 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 901af53f540..7a623e8b86e 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,16 @@
+2020-05-15  Alan Modra  <amodra@gmail.com>
+	    Alex Coplan  <alex.coplan@arm.com>
+
+	* symbols.c (struct local_symbol): Update comment.
+	(resolve_symbol_value): For resolved symbols equated to other
+	symbols, verify that the referenced symbol is not a local_symbol
+	before accessing sy_value.  Don't leave symbol loops during
+	finalize_syms resolution.
+	* testsuite/gas/all/assign-bad-recursive.d: New test.
+	* testsuite/gas/all/assign-bad-recursive.l: Error output for test.
+	* testsuite/gas/all/assign-bad-recursive.s: Assembly for test.
+	* testsuite/gas/all/gas.exp: Run it.
+
 2020-05-07  Andre Vieira  <andre.simoesdiasvieira@arm.com>
 
 	Backport from mainline.
diff --git a/gas/symbols.c b/gas/symbols.c
index 79cdb82ba8c..9bae085e683 100644
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -114,7 +114,7 @@ struct symbol
 /* A pointer in the symbol may point to either a complete symbol
    (struct symbol above) or to a local symbol (struct local_symbol
    defined here).  The symbol code can detect the case by examining
-   the first field.  It is always NULL for a local symbol.
+   the first field which is present in both structs.
 
    We do this because we ordinarily only need a small amount of
    information for a local symbol.  The symbol table takes up a lot of
@@ -1237,11 +1237,18 @@ resolve_symbol_value (symbolS *symp)
   if (symp->sy_flags.sy_resolved)
     {
       final_val = 0;
-      while (symp->sy_value.X_op == O_symbol
-	     && symp->sy_value.X_add_symbol->sy_flags.sy_resolved)
+      while (symp->sy_value.X_op == O_symbol)
 	{
 	  final_val += symp->sy_value.X_add_number;
 	  symp = symp->sy_value.X_add_symbol;
+	  if (LOCAL_SYMBOL_CHECK (symp))
+	    {
+	      struct local_symbol *locsym = (struct local_symbol *) symp;
+	      final_val += locsym->lsy_value;
+	      return final_val;
+	    }
+	  if (!symp->sy_flags.sy_resolved)
+	    return 0;
 	}
       if (symp->sy_value.X_op == O_constant)
 	final_val += symp->sy_value.X_add_number;
@@ -1380,6 +1387,11 @@ resolve_symbol_value (symbolS *symp)
 	      break;
 	    }
 
+	  /* Don't leave symbol loops.  */
+	  if (finalize_syms
+	      && add_symbol->sy_flags.sy_resolving)
+	    break;
+
 	  if (finalize_syms && final_val == 0)
 	    {
 	      if (LOCAL_SYMBOL_CHECK (add_symbol))
diff --git a/gas/testsuite/gas/all/assign-bad-recursive.d b/gas/testsuite/gas/all/assign-bad-recursive.d
new file mode 100644
index 00000000000..aeec5d55f8a
--- /dev/null
+++ b/gas/testsuite/gas/all/assign-bad-recursive.d
@@ -0,0 +1,4 @@
+#name: bad recursive assignments
+#source: assign-bad-recursive.s
+#xfail: bfin-*-*
+#error_output: assign-bad-recursive.l
diff --git a/gas/testsuite/gas/all/assign-bad-recursive.l b/gas/testsuite/gas/all/assign-bad-recursive.l
new file mode 100644
index 00000000000..70eaf0fe2e3
--- /dev/null
+++ b/gas/testsuite/gas/all/assign-bad-recursive.l
@@ -0,0 +1,7 @@
+[^:]*: Assembler messages:
+.*: Error: symbol definition loop encountered at `aaa'
+#...
+.*: Error: symbol definition loop encountered at `iii'
+#...
+.*: Error: symbol definition loop encountered at `xxx'
+#pass
diff --git a/gas/testsuite/gas/all/assign-bad-recursive.s b/gas/testsuite/gas/all/assign-bad-recursive.s
new file mode 100644
index 00000000000..21917d91ff6
--- /dev/null
+++ b/gas/testsuite/gas/all/assign-bad-recursive.s
@@ -0,0 +1,8 @@
+ aaa = aaa
+
+ iii = jjj
+ jjj = iii
+
+ xxx = yyy
+ yyy = zzz
+ zzz = xxx
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 78cd3eb3ea9..93e6e79b9df 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -97,6 +97,7 @@ if { ![istarget "bfin-*-*"] } then {
     gas_test "assign-ok.s" "" "" "== assignment support"
 }
 gas_test_error "assign-bad.s" "" "== assignment for symbol already set"
+run_dump_test assign-bad-recursive
 
 run_dump_test simple-forward
 run_dump_test forward


More information about the Binutils-cvs mailing list