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,
- ppc's md_assemble() needs to "look through" equates (which also helps
transitive forward equates).
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.
For ppc I wonder whether the "looking through" equates really should be
limited to registers only: This might be as applicable to constants
(including O_big), and finding e.g. O_illegal right away might be
helpful too.
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.
Comments
On Fri, Apr 21, 2023 at 02:17:54PM +0200, Jan Beulich wrote:
> For ppc I wonder whether the "looking through" equates really should be
> limited to registers only: This might be as applicable to constants
> (including O_big), and finding e.g. O_illegal right away might be
> helpful too.
I'm happy enough with the patch as-is, or with this further
simplification. The only concern I have is that there may be backends
other than ppc that need symbol equate resolved early. (But I haven't
looked, I'm leaving that to you!)
On 24.04.2023 02:05, Alan Modra wrote:
> On Fri, Apr 21, 2023 at 02:17:54PM +0200, Jan Beulich wrote:
>> For ppc I wonder whether the "looking through" equates really should be
>> limited to registers only: This might be as applicable to constants
>> (including O_big), and finding e.g. O_illegal right away might be
>> helpful too.
>
> I'm happy enough with the patch as-is, or with this further
> simplification. The only concern I have is that there may be backends
> other than ppc that need symbol equate resolved early. (But I haven't
> looked, I'm leaving that to you!)
Well, so far I've gone by testsuite results (albeit on ppc I expected
that I'd need to make a change somewhere, and running the testsuite was
mainly to help find where the change is needed), but of course there's
a fair chance that some backends simply lack respective testing. Yet to
be honest, trying to understand every target's backend code just to
figure whether they also need something similar is a little beyond of
what I'm feeling up to. (I did look for certain patterns, and I'll try
to think of more which might be relevant ...)
As to the further simplification - I meanwhile think that I perhaps
better shouldn't do that: Registers (when looking for insn operands of
certain types) are quite special, and forward reference equates aren't
(normally) okay to use in such places. Forward reference equates are,
otoh, at least sometimes okay to use for e.g. constants (when a
suitable internal relocation type is available to express it), and
even finding O_illegal might (via some magic) disappear by the time we
process relocations.
Do you have any thoughts on the other two remarks (equates of constants
not being recognized as equates, and the place where to set O_register)?
Jan
On Mon, Apr 24, 2023 at 10:12:51AM +0200, Jan Beulich wrote:
> Do you have any thoughts on the other two remarks (equates of constants
> not being recognized as equates, and the place where to set O_register)?
No, sorry.
@@ -13832,11 +13832,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;
}
@@ -3475,6 +3475,28 @@ md_assemble (char *str)
str = input_line_pointer;
input_line_pointer = hold;
+ /* "Look through" register equates. */
+ if (ex.X_op == O_symbol)
+ {
+ symbolS *sym = ex.X_add_symbol;
+ offsetT acc = ex.X_add_number;
+ const expressionS *e = symbol_get_value_expression (sym);
+
+ while (symbol_equated_p (sym))
+ {
+ if (e->X_op != O_symbol)
+ break;
+ sym = e->X_add_symbol;
+ acc += e->X_add_number;
+ e = symbol_get_value_expression (sym);
+ }
+ if (e->X_op == O_register)
+ {
+ ex = *e;
+ ex.X_add_number += acc;
+ }
+ }
+
if (ex.X_op == O_illegal)
as_bad (_("illegal operand"));
else if (ex.X_op == O_absent)
@@ -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