[PATCH v2] gas: equates of registers

[PATCH v2] gas: equates of registers

[Jan Beulich]

There are two problems: symbol_equated_p() doesn't recognize equates of
registers, and S_CAN_BE_REDEFINED() goes by section rather than by
expression type. Both together undermine .eqv and .equiv clearly meaning
to guard the involved symbols against re-definition (both ways).

To compensate pseudo_set() now using O_symbol and S_CAN_BE_REDEFINED()
now checking for O_register,
- for targets creating register symbols through symbol_{new,create}() ->
  symbol_init() -> S_SET_VALUE() (alpha, arc, dlx, ia64, m68k, mips,
  mmix, tic4x, tic54x, plus anything using cgen or itbl-ops), have
  symbol_init() set their expressions to O_register,
- x86'es parse_register() also can't go by section anymore when
  trying to "look through" equates; probably symbol_equated_p() should
  have been used there from the beginning, if only that had worked for
  equates of registers,
- various targets need to "look through" equates when parsing insn
  operands (which also helps transitive forward equates); perhaps even
  more ought to, but many don't look to consider the possibility of
  register equates in the first place.

This was uncovered by code reported in PR gas/30274 (duplicating
PR gas/30272), except that there .eqv was used when really .equ was
meant. Therefore that bug report is addressed here only in so far as
gas wouldn't crash anymore; the code there still won't assemble
successfully, just that now the issues there are properly diagnosed.
---
Clearly equates of constants have the same issue of not being viewed as
equates by symbol_equated_p(). Changing that isn't the purpose of the
change here, and I'm afraid is also yet more likely to trigger issues
elsewhere.

If the setting to O_register was to occur in S_SET_VALUE() instead of in
symbol_init() (which overall would seem more consistent), all callers
would need to make sure that they call S_SET_SEGMENT() (if at all) ahead
of S_SET_VALUE(), not afterwards.
---
v2: Slightly simplify ppc logic, convert it to a function, and re-use it
    elsewhere.

--- a/gas/cgen.c
+++ b/gas/cgen.c
@@ -385,6 +385,8 @@ gas_cgen_parse_operand (CGEN_CPU_DESC cd
 
   /* FIXME: Need to check `want'.  */
 
+  resolve_register (&exp);
+
   switch (exp.X_op)
     {
     case O_illegal:
--- a/gas/config/tc-alpha.c
+++ b/gas/config/tc-alpha.c
@@ -987,6 +987,7 @@ tokenize_arguments (char *str,
 
 	    /* First try for parenthesized register ...  */
 	    expression (tok);
+	    resolve_register (tok);
 	    if (*input_line_pointer == ')' && tok->X_op == O_register)
 	      {
 		tok->X_op = (saw_comma ? O_cpregister : O_pregister);
@@ -1010,6 +1011,8 @@ tokenize_arguments (char *str,
 	  if (tok->X_op == O_illegal || tok->X_op == O_absent)
 	    goto err;
 
+	  resolve_register (tok);
+
 	  saw_comma = 0;
 	  saw_arg = 1;
 	  ++tok;
--- a/gas/config/tc-arc.c
+++ b/gas/config/tc-arc.c
@@ -1312,6 +1312,8 @@ tokenize_arguments (char *str,
 	     relocation type as well.  */
 	  if (*input_line_pointer == '@')
 	    parse_reloc_symbol (tok);
+	  else
+	    resolve_register (tok);
 
 	  debug_exp (tok);
 
--- a/gas/config/tc-dlx.c
+++ b/gas/config/tc-dlx.c
@@ -632,6 +632,7 @@ parse_operand (char *s, expressionS *ope
       /* Normal operand parsing.  */
       input_line_pointer = s;
       (void) expression (operandp);
+      resolve_register (operandp);
     }
 
   new_pos = input_line_pointer;
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -13830,11 +13830,11 @@ parse_register (const char *reg_string,
       input_line_pointer = buf;
       get_symbol_name (&name);
       symbolP = symbol_find (name);
-      while (symbolP && S_GET_SEGMENT (symbolP) != reg_section)
+      while (symbolP && symbol_equated_p (symbolP))
 	{
 	  const expressionS *e = symbol_get_value_expression(symbolP);
 
-	  if (e->X_op != O_symbol || e->X_add_number)
+	  if (e->X_add_number)
 	    break;
 	  symbolP = e->X_add_symbol;
 	}
--- a/gas/config/tc-ia64.c
+++ b/gas/config/tc-ia64.c
@@ -5987,6 +5987,7 @@ parse_operand (expressionS *e, int more)
   e->X_op = O_absent;
   SKIP_WHITESPACE ();
   expression (e);
+  resolve_register (e);
   sep = *input_line_pointer;
   if (more && (sep == ',' || sep == more))
     ++input_line_pointer;
--- a/gas/config/tc-mmix.c
+++ b/gas/config/tc-mmix.c
@@ -624,6 +624,8 @@ get_putget_operands (struct mmix_opcode
   regno = get_spec_regno (sregp);
   *sregend = c;
 
+  resolve_register (expp_reg);
+
   /* Let the caller issue errors; we've made sure the operands are
      invalid.  */
   if (expp_reg->X_op != O_illegal
--- a/gas/config/tc-mn10200.c
+++ b/gas/config/tc-mn10200.c
@@ -1025,6 +1025,7 @@ md_assemble (char *str)
 	  else
 	    {
 	      expression (&ex);
+	      resolve_register (&ex);
 	    }
 
 	  switch (ex.X_op)
--- a/gas/config/tc-mn10300.c
+++ b/gas/config/tc-mn10300.c
@@ -1669,6 +1669,7 @@ md_assemble (char *str)
 	  else
 	    {
 	      expression (&ex);
+	      resolve_register (&ex);
 	    }
 
 	  switch (ex.X_op)
--- a/gas/config/tc-msp430.c
+++ b/gas/config/tc-msp430.c
@@ -415,6 +415,8 @@ parse_exp (char * s, expressionS * op)
   expression (op);
   if (op->X_op == O_absent)
     as_bad (_("missing operand"));
+  else
+    resolve_register (op);
 
   /* Our caller is likely to check that the entire expression was parsed.
      If we have found a hex constant with an 'h' suffix, ilp will be left
--- a/gas/config/tc-nds32.c
+++ b/gas/config/tc-nds32.c
@@ -2519,6 +2519,7 @@ parse_expression (char *str, expressionS
   tmp = input_line_pointer;	/* Save line pointer.  */
   input_line_pointer = str;
   expression (exp);
+  resolve_register (exp);
   s = input_line_pointer;
   input_line_pointer = tmp;	/* Restore line pointer.  */
 
@@ -4571,6 +4572,7 @@ nds32_asm_parse_operand (struct nds32_as
   hold = input_line_pointer;
   input_line_pointer = *pstr;
   expression (pexp);
+  resolve_register (pexp);
   *pstr = input_line_pointer;
   input_line_pointer = hold;
 
--- a/gas/config/tc-ppc.c
+++ b/gas/config/tc-ppc.c
@@ -3475,6 +3475,8 @@ md_assemble (char *str)
       str = input_line_pointer;
       input_line_pointer = hold;
 
+      resolve_register (&ex);
+
       if (ex.X_op == O_illegal)
 	as_bad (_("illegal operand"));
       else if (ex.X_op == O_absent)
--- a/gas/config/tc-s390.c
+++ b/gas/config/tc-s390.c
@@ -1308,7 +1308,10 @@ md_gather_operands (char *str,
 
       /* Parse the operand.  */
       if (! register_name (&ex))
-	expression (&ex);
+	{
+	  expression (&ex);
+	  resolve_register (&ex);
+	}
 
       str = input_line_pointer;
       input_line_pointer = hold;
--- a/gas/config/tc-spu.c
+++ b/gas/config/tc-spu.c
@@ -573,6 +573,7 @@ get_reg (const char *param, struct spu_i
       expression (&ex);
       param = input_line_pointer;
       input_line_pointer = save_ptr;
+      resolve_register (&ex);
       if (ex.X_op == O_register || ex.X_op == O_constant)
 	{
 	  insn->opcode |= ex.X_add_number << arg_encode[arg].pos;
--- a/gas/config/tc-tic4x.c
+++ b/gas/config/tc-tic4x.c
@@ -649,6 +649,7 @@ tic4x_expression (char *str, expressionS
   t = input_line_pointer;	/* Save line pointer.  */
   input_line_pointer = str;
   expression (exp);
+  resolve_register (exp);
   s = input_line_pointer;
   input_line_pointer = t;	/* Restore line pointer.  */
   return s;			/* Return pointer to where parsing stopped.  */
--- a/gas/config/tc-v850.c
+++ b/gas/config/tc-v850.c
@@ -2909,6 +2909,7 @@ md_assemble (char *str)
 	      else
 		{
 		  expression (&ex);
+		  resolve_register (&ex);
 
 		  if ((operand->flags & V850_NOT_IMM0)
 		      && ex.X_op == O_constant
--- a/gas/config/tc-xgate.c
+++ b/gas/config/tc-xgate.c
@@ -893,6 +893,8 @@ xgate_parse_exp (char *s, expressionS *
   expression (op);
   if (op->X_op == O_absent)
     as_bad (_("missing operand"));
+  else
+    resolve_register (op);
   return input_line_pointer;
 }
 
--- a/gas/config/tc-z80.c
+++ b/gas/config/tc-z80.c
@@ -926,6 +926,7 @@ parse_exp_not_indexed (const char *s, ex
     }
   input_line_pointer = (char*) s ;
   expression (op);
+  resolve_register (op);
   switch (op->X_op)
     {
     case O_absent:
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -2382,6 +2382,31 @@ resolve_expression (expressionS *express
 
   return 1;
 }
+
+/* "Look through" register equates.  */
+void resolve_register (expressionS *expP)
+{
+  symbolS *sym;
+  offsetT acc = 0;
+  const expressionS *e = expP;
+
+  if (expP->X_op != O_symbol)
+    return;
+
+  do
+    {
+      sym = e->X_add_symbol;
+      acc += e->X_add_number;
+      e = symbol_get_value_expression (sym);
+    }
+  while (symbol_equated_p (sym));
+
+  if (e->X_op == O_register)
+    {
+      *expP = *e;
+      expP->X_add_number += acc;
+    }
+}
 \f
 /* This lives here because it belongs equally in expr.c & read.c.
    expr.c is just a branch office read.c anyway, and putting it
--- a/gas/expr.h
+++ b/gas/expr.h
@@ -190,5 +190,6 @@ extern symbolS *expr_build_dot (void);
 extern uint32_t generic_bignum_to_int32 (void);
 extern uint64_t generic_bignum_to_int64 (void);
 extern int resolve_expression (expressionS *);
+extern void resolve_register (expressionS *);
 
 extern bool literal_prefix_dollar_hex;
--- a/gas/read.c
+++ b/gas/read.c
@@ -4000,6 +4000,10 @@ pseudo_set (symbolS *symbolP)
 	  return;
 	}
 #endif
+      /* Make sure symbol_equated_p() recognizes the symbol as an equate.  */
+      exp.X_add_symbol = make_expr_symbol (&exp);
+      exp.X_add_number = 0;
+      exp.X_op = O_symbol;
       symbol_set_value_expression (symbolP, &exp);
       S_SET_SEGMENT (symbolP, reg_section);
       set_zero_frag (symbolP);
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -387,6 +387,8 @@ symbol_init (symbolS *symbolP, const cha
     }
 
   S_SET_VALUE (symbolP, valu);
+  if (sec == reg_section)
+    symbolP->x->value.X_op = O_register;
 
   symbol_clear_list_pointers (symbolP);
 
@@ -2463,7 +2465,7 @@ S_CAN_BE_REDEFINED (const symbolS *s)
     return (((struct local_symbol *) s)->frag
 	    == &predefined_address_frag);
   /* Permit register names to be redefined.  */
-  return s->bsym->section == reg_section;
+  return s->x->value.X_op == O_register;
 }
 
 int

Re: [PATCH v2] gas: equates of registers

[Hans-Peter Nilsson]

On Tue, 2 May 2023, Jan Beulich wrote:

> There are two problems: symbol_equated_p() doesn't recognize equates of
> registers, and S_CAN_BE_REDEFINED() goes by section rather than by
> expression type. Both together undermine .eqv and .equiv clearly meaning
> to guard the involved symbols against re-definition (both ways).
> 
> To compensate pseudo_set() now using O_symbol and S_CAN_BE_REDEFINED()
> now checking for O_register,
> - for targets creating register symbols through symbol_{new,create}() ->
>   symbol_init() -> S_SET_VALUE() (alpha, arc, dlx, ia64, m68k, mips,
>   mmix, tic4x, tic54x, plus anything using cgen or itbl-ops), have
>   symbol_init() set their expressions to O_register,

(Heads-up for maintainers or asking for approval though being a 
global maintainer?  Anyway...)

Ok for mmix iff the binutils test-suites (ld, gas, binutils) 
shows no regressions for mmix with this patch.

brgds, H-P

Re: [PATCH v2] gas: equates of registers

[Jan Beulich]

On 03.05.2023 01:37, Hans-Peter Nilsson wrote:
> On Tue, 2 May 2023, Jan Beulich wrote:
> 
>> There are two problems: symbol_equated_p() doesn't recognize equates of
>> registers, and S_CAN_BE_REDEFINED() goes by section rather than by
>> expression type. Both together undermine .eqv and .equiv clearly meaning
>> to guard the involved symbols against re-definition (both ways).
>>
>> To compensate pseudo_set() now using O_symbol and S_CAN_BE_REDEFINED()
>> now checking for O_register,
>> - for targets creating register symbols through symbol_{new,create}() ->
>>   symbol_init() -> S_SET_VALUE() (alpha, arc, dlx, ia64, m68k, mips,
>>   mmix, tic4x, tic54x, plus anything using cgen or itbl-ops), have
>>   symbol_init() set their expressions to O_register,
> 
> (Heads-up for maintainers or asking for approval though being a 
> global maintainer?  Anyway...)

Well, I'd prefer to have arch maintainer agreement, but I wouldn't wait
indefinitely with a change like this one.

> Ok for mmix iff the binutils test-suites (ld, gas, binutils) 
> shows no regressions for mmix with this patch.

Thanks; no regressions seen here.

Jan