[PATCH] Arm64: re-work PR gas/27217 fix
Jan Beulich
jbeulich@suse.com
Thu Jul 28 11:37:24 GMT 2022
On 29.06.2022 10:20, Jan Beulich wrote:
> The original approach has resulted in anomalies when . is involved in an
> operand of one of the affected insns. We cannot leave . unresolved, or
> else it'll be resolved at the end of assembly, then pointing to the
> address of a section rather than at the insn of interest. Undo part of
> the original change and instead check whether a relocation cannot be
> omitted in md_apply_fix().
>
> By resolving the expressions again, equates (see the adjustment of the
> respective testcase) will now be evaluated, and hence relocations
> against absolute addresses be emitted. This ought to be okay as long as
> the equates aren't global (and hence can't be overridden). If a need
> for such arises, quite likely the only way to address this would be to
> invent yet another expression evaluation mode, leaving everything
> _except_ . un-evaluated.
>
> There's a further anomaly in how transitive equates are handled. In
>
> .set x, 0x12345678
> .eqv bar, x
> foo:
> adrp x0, x
> add x0, x0, :lo12:x
>
> adrp x0, bar
> add x0, x0, :lo12:bar
>
> the first two relocations are now against *ABS*:0x12345678 (as said
> above), whereas the latter two relocations would be against x. (Before
> the change here, the first two relocations are against x and the latter
> two against bar.) But this is an issue seen elsewhere as well, and would
> likely require adjustments in the target-independent parts of the
> assembler instead of trying to hack around this for every target.
Before the release is cut, may I ask what the plans here are? Should I
commit this patch, are there alternative plans to address the regression,
or is the regression intended to remain present in 2.39?
Thanks, Jan
> --- a/gas/config/tc-aarch64.c
> +++ b/gas/config/tc-aarch64.c
> @@ -565,25 +565,18 @@ static bool in_aarch64_get_expression =
> #define ALLOW_ABSENT false
> #define REJECT_ABSENT true
>
> -/* Fifth argument to aarch64_get_expression. */
> -#define NORMAL_RESOLUTION false
> -
> /* Return TRUE if the string pointed by *STR is successfully parsed
> as an valid expression; *EP will be filled with the information of
> such an expression. Otherwise return FALSE.
>
> If ALLOW_IMMEDIATE_PREFIX is true then skip a '#' at the start.
> - If REJECT_ABSENT is true then trat missing expressions as an error.
> - If DEFER_RESOLUTION is true, then do not resolve expressions against
> - constant symbols. Necessary if the expression is part of a fixup
> - that uses a reloc that must be emitted. */
> + If REJECT_ABSENT is true then trat missing expressions as an error. */
>
> static bool
> aarch64_get_expression (expressionS * ep,
> char ** str,
> bool allow_immediate_prefix,
> - bool reject_absent,
> - bool defer_resolution)
> + bool reject_absent)
> {
> char *save_in;
> segT seg;
> @@ -603,10 +596,7 @@ aarch64_get_expression (expressionS * e
> save_in = input_line_pointer;
> input_line_pointer = *str;
> in_aarch64_get_expression = true;
> - if (defer_resolution)
> - seg = deferred_expression (ep);
> - else
> - seg = expression (ep);
> + seg = expression (ep);
> in_aarch64_get_expression = false;
>
> if (ep->X_op == O_illegal || (reject_absent && ep->X_op == O_absent))
> @@ -1035,8 +1025,7 @@ parse_typed_reg (char **ccp, aarch64_reg
>
> atype.defined |= NTA_HASINDEX;
>
> - aarch64_get_expression (&exp, &str, GE_NO_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION);
> + aarch64_get_expression (&exp, &str, GE_NO_PREFIX, REJECT_ABSENT);
>
> if (exp.X_op != O_constant)
> {
> @@ -1239,8 +1228,7 @@ parse_vector_reg_list (char **ccp, aarch
> {
> expressionS exp;
>
> - aarch64_get_expression (&exp, &str, GE_NO_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION);
> + aarch64_get_expression (&exp, &str, GE_NO_PREFIX, REJECT_ABSENT);
> if (exp.X_op != O_constant)
> {
> set_first_syntax_error (_("constant expression required."));
> @@ -2187,8 +2175,7 @@ parse_immediate_expression (char **str,
> return false;
> }
>
> - aarch64_get_expression (exp, str, GE_OPT_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION);
> + aarch64_get_expression (exp, str, GE_OPT_PREFIX, REJECT_ABSENT);
>
> if (exp->X_op == O_absent)
> {
> @@ -2422,8 +2409,7 @@ parse_big_immediate (char **str, int64_t
> return false;
> }
>
> - aarch64_get_expression (&inst.reloc.exp, &ptr, GE_OPT_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION);
> + aarch64_get_expression (&inst.reloc.exp, &ptr, GE_OPT_PREFIX, REJECT_ABSENT);
>
> if (inst.reloc.exp.X_op == O_constant)
> *imm = inst.reloc.exp.X_add_number;
> @@ -3320,8 +3306,7 @@ parse_shift (char **str, aarch64_opnd_in
> p++;
> exp_has_prefix = 1;
> }
> - (void) aarch64_get_expression (&exp, &p, GE_NO_PREFIX, ALLOW_ABSENT,
> - NORMAL_RESOLUTION);
> + aarch64_get_expression (&exp, &p, GE_NO_PREFIX, ALLOW_ABSENT);
> }
> if (kind == AARCH64_MOD_MUL_VL)
> /* For consistency, give MUL VL the same shift amount as an implicit
> @@ -3385,7 +3370,7 @@ parse_shifter_operand_imm (char **str, a
>
> /* Accept an immediate expression. */
> if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_OPT_PREFIX,
> - REJECT_ABSENT, NORMAL_RESOLUTION))
> + REJECT_ABSENT))
> return false;
>
> /* Accept optional LSL for arithmetic immediate values. */
> @@ -3509,8 +3494,7 @@ parse_shifter_operand_reloc (char **str,
>
> /* Next, we parse the expression. */
> if (! aarch64_get_expression (&inst.reloc.exp, str, GE_NO_PREFIX,
> - REJECT_ABSENT,
> - aarch64_force_reloc (entry->add_type) == 1))
> + REJECT_ABSENT))
> return false;
>
> /* Record the relocation type (use the ADD variant here). */
> @@ -3656,8 +3640,7 @@ parse_address_main (char **str, aarch64_
> }
>
> /* #:<reloc_op>: */
> - if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
> - aarch64_force_reloc (ty) == 1))
> + if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT))
> {
> set_syntax_error (_("invalid relocation expression"));
> return false;
> @@ -3673,8 +3656,7 @@ parse_address_main (char **str, aarch64_
> /* =immediate; need to generate the literal in the literal pool. */
> inst.gen_lit_pool = 1;
>
> - if (!aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION))
> + if (!aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT))
> {
> set_syntax_error (_("invalid address"));
> return false;
> @@ -3780,8 +3762,7 @@ parse_address_main (char **str, aarch64_
> /* We now have the group relocation table entry corresponding to
> the name in the assembler source. Next, we parse the
> expression. */
> - if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
> - aarch64_force_reloc (entry->ldst_type) == 1))
> + if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT))
> {
> set_syntax_error (_("invalid relocation expression"));
> return false;
> @@ -3794,8 +3775,7 @@ parse_address_main (char **str, aarch64_
> }
> else
> {
> - if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION))
> + if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT))
> {
> set_syntax_error (_("invalid expression in the address"));
> return false;
> @@ -3851,8 +3831,7 @@ parse_address_main (char **str, aarch64_
> operand->addr.offset.regno = reg->number;
> operand->addr.offset.is_reg = 1;
> }
> - else if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION))
> + else if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT))
> {
> /* [Xn],#expr */
> set_syntax_error (_("invalid expression in the address"));
> @@ -3980,8 +3959,7 @@ parse_half (char **str, int *internal_fi
> else
> *internal_fixup_p = 1;
>
> - if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
> - aarch64_force_reloc (inst.reloc.type) == 1))
> + if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_NO_PREFIX, REJECT_ABSENT))
> return false;
>
> *str = p;
> @@ -4023,8 +4001,7 @@ parse_adrp (char **str)
> inst.reloc.type = BFD_RELOC_AARCH64_ADR_HI21_PCREL;
>
> inst.reloc.pc_rel = 1;
> - if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
> - aarch64_force_reloc (inst.reloc.type) == 1))
> + if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_NO_PREFIX, REJECT_ABSENT))
> return false;
> *str = p;
> return true;
> @@ -6696,8 +6673,7 @@ parse_operands (char *str, const aarch64
> goto failure;
> str = saved;
> po_misc_or_fail (aarch64_get_expression (&inst.reloc.exp, &str,
> - GE_OPT_PREFIX, REJECT_ABSENT,
> - NORMAL_RESOLUTION));
> + GE_OPT_PREFIX, REJECT_ABSENT));
> /* The MOV immediate alias will be fixed up by fix_mov_imm_insn
> later. fix_mov_imm_insn will try to determine a machine
> instruction (MOVZ, MOVN or ORR) for it and will issue an error
> @@ -8824,7 +8800,8 @@ md_apply_fix (fixS * fixP, valueT * valP
>
> /* Note whether this will delete the relocation. */
>
> - if (fixP->fx_addsy == 0 && !fixP->fx_pcrel)
> + if (fixP->fx_addsy == 0 && !fixP->fx_pcrel
> + && aarch64_force_reloc (fixP->fx_r_type) <= 0)
> fixP->fx_done = 1;
>
> /* Process the relocations. */
> --- a/gas/testsuite/gas/aarch64/pr27217.d
> +++ b/gas/testsuite/gas/aarch64/pr27217.d
> @@ -7,9 +7,9 @@
> Disassembly of section \.text:
>
> 0+000 <.*>:
> -[ ]+0:[ ]+90000000[ ]+adrp[ ]+x0, 12345678[ ]+<bar>
> -[ ]+0:[ ]+R_AARCH64(|_P32)_ADR_PREL_PG_HI21[ ]+bar
> +[ ]+0:[ ]+90000000[ ]+adrp[ ]+x0, [0-9]*[ ]+<.*>
> +[ ]+0:[ ]+R_AARCH64(|_P32)_ADR_PREL_PG_HI21[ ]+\*ABS\*\+0x12345678
> [ ]+4:[ ]+91000000[ ]+add[ ]+x0, x0, #0x0
> -[ ]+4:[ ]+R_AARCH64(|_P32)_ADD_ABS_LO12_NC[ ]+bar
> +[ ]+4:[ ]+R_AARCH64(|_P32)_ADD_ABS_LO12_NC[ ]+\*ABS\*\+0x12345678
> [ ]+8:[ ]+d65f03c0[ ]+ret
> #pass
More information about the Binutils
mailing list