public inbox for binutils-cvs@sourceware.org
help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@sourceware.org>
To: bfd-cvs@sourceware.org
Subject: [binutils-gdb] Arm64: re-work PR gas/27217 fix
Date: Fri, 29 Jul 2022 07:27:34 +0000 (GMT)	[thread overview]
Message-ID: <20220729072734.D937E3858D39@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c1723a8118f0d02b01a061095f48e75264b2ca4f

commit c1723a8118f0d02b01a061095f48e75264b2ca4f
Author: Jan Beulich <jbeulich@suse.com>
Date:   Fri Jul 29 09:26:47 2022 +0200

    Arm64: re-work PR gas/27217 fix
    
    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.

Diff:
---
 gas/config/tc-aarch64.c             | 63 ++++++++++++-------------------------
 gas/testsuite/gas/aarch64/pr27217.d |  6 ++--
 2 files changed, 23 insertions(+), 46 deletions(-)

diff --git a/gas/config/tc-aarch64.c b/gas/config/tc-aarch64.c
index 779db31828b..e12b68ed2c8 100644
--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -565,25 +565,18 @@ static bool in_aarch64_get_expression = false;
 #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 *  ep,
   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_type type, aarch64_reg_type *rtype,
 
       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, aarch64_reg_type type,
 	{
 	  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, expressionS *exp,
       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 *imm, aarch64_reg_type reg_type)
       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_info *operand, enum parse_shift_mode mode)
 	  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, aarch64_opnd_info *operand,
 
   /* 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, aarch64_opnd_info *operand,
 
       /* 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_opnd_info *operand,
 	    }
 
 	  /* #:<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_opnd_info *operand,
 	    /* =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_opnd_info *operand,
 	      /* 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_opnd_info *operand,
 	    }
 	  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_opnd_info *operand,
 	  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_fixup_p)
   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;
@@ -6706,8 +6683,7 @@ parse_operands (char *str, const aarch64_opcode *opcode)
 	      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
@@ -8834,7 +8810,8 @@ md_apply_fix (fixS * fixP, valueT * valP, segT seg)
 
   /* 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.  */
diff --git a/gas/testsuite/gas/aarch64/pr27217.d b/gas/testsuite/gas/aarch64/pr27217.d
index abbc48717b4..3397dfa2481 100644
--- 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


                 reply	other threads:[~2022-07-29  7:27 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220729072734.D937E3858D39@sourceware.org \
    --to=jbeulich@sourceware.org \
    --cc=bfd-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).