[PATCH] gas: leave expression symbols alone when processing equates

Jan Beulich jbeulich@suse.com
Tue Feb 25 11:02:34 GMT 2025


PR gas/32721
In this bogus piece of code distilled from fuzzing and slightly edited:

	A=%eax|%!
	Y=A
	Z=A
	or $6,Z

the first of the equates with A on the rhs changes A's section (due to
the use of S_GET_VALUE()), from expression to register, thus yielding Y
in the expression section (and X_op being O_symbol), but Z in the
register section (and X_op being O_register with X_add_value being -1).
There shouldn't be random O_register expressions, though, for targets
setting md_register_arithmetic to false. Plus both Y and Z would better
be exchangeable.

In pseudo_set() wire handling of O_symbol expressions referencing a
symbol in the expression section to that of other stuff ending up in
this section.

Also avoid bogus O_register expressions to be created, for targets
setting md_register_arithmetic to false: S_GET_VALUE() would resolve
any arithmetic, which must not happen for such targets. To be on the
safe side for such targets, also amend resolve_register(). Correct
another earlier oversight there too (affecting at least Z80), by using
the new expr_copy() helper there as well.

Undo 46b9f07dfe79 ("PR 32721, internal error in
tc-i386.c:parse_register"), albeit without losing the simplification it
did.
---
Note that either of the two fixes eliminates the internal error. Still
I think both changes want making.

A more thorough (yet perhaps also more impactful) approach may be to
avoid involving S_SET_SEGMENT() at the end of resolve_symbol_value().
The comment there suggests it's needed for establishing defined-ness of
symbols, but that's - aiui - only relevant for symbols presently in
undefined_section. Symbols in expr_section, in particular, could be left
alone without violating what the comment says is intended. (To limit the
impact, S_GET_SEGMENT() may then also need to invoke
resolve_symbol_value(), just like S_GET_VALUE() does.)

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -16847,9 +16847,9 @@ parse_register (const char *reg_string,
 	{
 	  const expressionS *e = symbol_get_value_expression (symbolP);
 
-	  if (e->X_op == O_register
-	      && (valueT) e->X_add_number < i386_regtab_size)
+	  if (e->X_op == O_register)
 	    {
+	      know ((valueT) e->X_add_number < i386_regtab_size);
 	      r = i386_regtab + e->X_add_number;
 	      *end_op = (char *) reg_string + (input_line_pointer - buf);
 	    }
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -771,6 +771,16 @@ expr_build_dot (void)
   return expr_build_uconstant (abs_section_offset);
 }
 
+/* Copy an expression, preserving X_md.  */
+
+static void expr_copy (expressionS *dst, const expressionS *src)
+{
+  unsigned short md = dst->X_md;
+
+  *dst = *src;
+  dst->X_md = md;
+}
+
 #ifndef md_register_arithmetic
 # define md_register_arithmetic 1
 #endif
@@ -1400,8 +1410,17 @@ operand (expressionS *expressionP, enum
 	    }
 	  else if (!expr_defer_p (mode) && segment == reg_section)
 	    {
-	      expressionP->X_op = O_register;
-	      expressionP->X_add_number = S_GET_VALUE (symbolP);
+	      if (md_register_arithmetic)
+		{
+		  expressionP->X_op = O_register;
+		  expressionP->X_add_number = S_GET_VALUE (symbolP);
+		}
+	      else
+		{
+		  expr_copy (expressionP,
+			     symbol_get_value_expression (symbolP));
+		  resolve_register (expressionP);
+		}
 	    }
 	  else
 	    {
@@ -2493,6 +2512,8 @@ void resolve_register (expressionS *expP
 
   do
     {
+      if (!md_register_arithmetic && e->X_add_number)
+	break;
       sym = e->X_add_symbol;
       acc += e->X_add_number;
       e = symbol_get_value_expression (sym);
@@ -2501,7 +2522,7 @@ void resolve_register (expressionS *expP
 
   if (e->X_op == O_register)
     {
-      *expP = *e;
+      expr_copy (expP, e);
       expP->X_add_number += acc;
     }
 }
--- a/gas/read.c
+++ b/gas/read.c
@@ -4115,6 +4115,8 @@ pseudo_set (symbolS *symbolP)
 
     case O_symbol:
       seg = S_GET_SEGMENT (exp.X_add_symbol);
+      if (seg == expr_section)
+	goto expr;
       /* For x=undef+const, create an expression symbol.
 	 For x=x+const, just update x except when x is an undefined symbol
 	 For x=defined+const, evaluate x.  */
@@ -4146,6 +4148,7 @@ pseudo_set (symbolS *symbolP)
       break;
 
     default:
+    expr:
       /* The value is some complex expression.  */
       S_SET_SEGMENT (symbolP, expr_section);
       symbol_set_value_expression (symbolP, &exp);
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -848,6 +848,9 @@ run_list_test "unspec" ""
 run_dump_test "fp"
 run_dump_test "cond"
 run_dump_test pr30248
+if {![is_aout_format]} then {
+    run_list_test pr32721 ""
+}
 if {[is_elf_format] || [istarget "*-*-vxworks*"]} then {
     run_list_test_stdin "list-1" "-al"
     run_list_test_stdin "list-2" "-al"
--- /dev/null
+++ b/gas/testsuite/gas/i386/pr32721.l
@@ -0,0 +1,7 @@
+.*: Assembler messages:
+.*:1: Error: bad expression
+.*:1: Warning: missing operand; zero assumed
+.*:1: Error: invalid operands .* for `|'
+#?.*: Error: invalid operands .* when setting .*
+#?.*: Error: invalid operands .* when setting .*
+#?.*: Error: invalid operands .* when setting .*
--- /dev/null
+++ b/gas/testsuite/gas/i386/pr32721.s
@@ -0,0 +1,7 @@
+	A=%eax|%!
+	Y=A
+	Z=A
+
+	.text
+pr32721:
+	orl $6,Z


More information about the Binutils mailing list