[v2] gas: equates of registers
Checks
Commit Message
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.
Comments
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
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
@@ -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:
@@ -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;
@@ -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);
@@ -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;
@@ -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;
}
@@ -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;
@@ -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
@@ -1025,6 +1025,7 @@ md_assemble (char *str)
else
{
expression (&ex);
+ resolve_register (&ex);
}
switch (ex.X_op)
@@ -1669,6 +1669,7 @@ md_assemble (char *str)
else
{
expression (&ex);
+ resolve_register (&ex);
}
switch (ex.X_op)
@@ -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
@@ -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;
@@ -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)
@@ -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;
@@ -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;
@@ -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. */
@@ -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
@@ -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;
}
@@ -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:
@@ -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;
+ }
+}
/* 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
@@ -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;
@@ -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);
@@ -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