[5/5] aarch64: re-work PR gas/27217 fix again

Message ID 1a9cfc5a-03ca-47cd-a3d8-abc00b779fad@suse.com
State New
Headers
Series gas: dot handling |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich Nov. 22, 2024, 12:47 p.m. UTC
  Commit c1723a8118f0 ("Arm64: re-work PR gas/27217 fix") really was only
a band-aid; Nick's original solution to the problem was technically
preferable, yet didn't work when . came into play. Undo most of that
change, but use the new expr_defer_latch_dot expression parsing mode.

Also add testing for the . case, which I should have done already back
at the time.
---
Quite likely tc-spu.c will want to use latched_dot_expression() as well,
perhaps allowing the double parsing to be dropped. Question really is:
Should the new mode become the default, with just pseudo_set() using the
variant not evaluating dot? In which case names likely would want
changing (e.g. expr_defer and expr_defer_dot, with the latter better not
even given a wrapper, i.e. truly only for pseudo_set() to use).

I wasn't certain whether to keep the md_apply_fix() hunk, or rather
leave that code untouched here. The extra check was necessary to add
back at the time, and I wonder whether it really wants/needs removing
again.
  

Patch

--- a/gas/config/tc-aarch64.c
+++ b/gas/config/tc-aarch64.c
@@ -666,18 +666,25 @@  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 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.  */
 
 static bool
 aarch64_get_expression (expressionS *  ep,
 			char **        str,
 			bool           allow_immediate_prefix,
-			bool           reject_absent)
+			bool           reject_absent,
+			bool           defer_resolution)
 {
   char *save_in;
   segT seg;
@@ -697,7 +704,10 @@  aarch64_get_expression (expressionS *  e
   save_in = input_line_pointer;
   input_line_pointer = *str;
   in_aarch64_get_expression = true;
-  seg = expression (ep);
+  if (defer_resolution)
+    seg = latched_dot_expression (ep);
+  else
+    seg = expression (ep);
   in_aarch64_get_expression = false;
 
   if (ep->X_op == O_illegal || (reject_absent && ep->X_op == O_absent))
@@ -1163,7 +1173,8 @@  parse_index_expression (char **str, int6
 {
   expressionS exp;
 
-  aarch64_get_expression (&exp, str, GE_NO_PREFIX, REJECT_ABSENT);
+  aarch64_get_expression (&exp, str, GE_NO_PREFIX, REJECT_ABSENT,
+			  NORMAL_RESOLUTION);
   if (exp.X_op != O_constant)
     {
       first_error (_("constant expression required"));
@@ -2546,7 +2557,8 @@  parse_immediate_expression (char **str,
       return false;
     }
 
-  aarch64_get_expression (exp, str, GE_OPT_PREFIX, REJECT_ABSENT);
+  aarch64_get_expression (exp, str, GE_OPT_PREFIX, REJECT_ABSENT,
+			  NORMAL_RESOLUTION);
 
   if (exp->X_op == O_absent)
     {
@@ -2780,7 +2792,8 @@  parse_big_immediate (char **str, int64_t
       return false;
     }
 
-  aarch64_get_expression (&inst.reloc.exp, &ptr, GE_OPT_PREFIX, REJECT_ABSENT);
+  aarch64_get_expression (&inst.reloc.exp, &ptr, GE_OPT_PREFIX, REJECT_ABSENT,
+			  NORMAL_RESOLUTION);
 
   if (inst.reloc.exp.X_op == O_constant)
     *imm = inst.reloc.exp.X_add_number;
@@ -3677,7 +3690,8 @@  parse_shift (char **str, aarch64_opnd_in
 	  p++;
 	  exp_has_prefix = 1;
 	}
-      aarch64_get_expression (&exp, &p, GE_NO_PREFIX, ALLOW_ABSENT);
+      aarch64_get_expression (&exp, &p, GE_NO_PREFIX, ALLOW_ABSENT,
+			      NORMAL_RESOLUTION);
     }
   if (kind == AARCH64_MOD_MUL_VL)
     /* For consistency, give MUL VL the same shift amount as an implicit
@@ -3741,7 +3755,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))
+				REJECT_ABSENT, NORMAL_RESOLUTION))
     return false;
 
   /* Accept optional LSL for arithmetic immediate values.  */
@@ -3900,7 +3914,8 @@  parse_shifter_operand_reloc (char **str,
 
       /* Next, we parse the expression.  */
       if (! aarch64_get_expression (&inst.reloc.exp, str, GE_NO_PREFIX,
-				    REJECT_ABSENT))
+				    REJECT_ABSENT,
+				    aarch64_force_reloc (entry->add_type) == 1))
 	return false;
 
       /* Record the relocation type (use the ADD variant here).  */
@@ -4095,7 +4110,8 @@  parse_address_main (char **str, aarch64_
 	    }
 
 	  /* #:<reloc_op>:  */
-	  if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT))
+	  if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
+					aarch64_force_reloc (ty) == 1))
 	    {
 	      set_syntax_error (_("invalid relocation expression"));
 	      return false;
@@ -4111,7 +4127,8 @@  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))
+	  if (!aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
+				       NORMAL_RESOLUTION))
 	    {
 	      set_syntax_error (_("invalid address"));
 	      return false;
@@ -4225,7 +4242,8 @@  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))
+	      if (! aarch64_get_expression (exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
+					    aarch64_force_reloc (entry->ldst_type) == 1))
 		{
 		  set_syntax_error (_("invalid relocation expression"));
 		  return false;
@@ -4238,7 +4256,8 @@  parse_address_main (char **str, aarch64_
 	    }
 	  else
 	    {
-	      if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT))
+	      if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT,
+					    NORMAL_RESOLUTION))
 		{
 		  set_syntax_error (_("invalid expression in the address"));
 		  return false;
@@ -4294,7 +4313,8 @@  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))
+      else if (! aarch64_get_expression (exp, &p, GE_OPT_PREFIX, REJECT_ABSENT,
+					 NORMAL_RESOLUTION))
 	{
 	  /* [Xn],#expr */
 	  set_syntax_error (_("invalid expression in the address"));
@@ -4422,7 +4442,8 @@  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))
+  if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
+				aarch64_force_reloc (inst.reloc.type) == 1))
     return false;
 
   *str = p;
@@ -4464,7 +4485,8 @@  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))
+  if (! aarch64_get_expression (&inst.reloc.exp, &p, GE_NO_PREFIX, REJECT_ABSENT,
+				aarch64_force_reloc (inst.reloc.type) == 1))
     return false;
   *str = p;
   return true;
@@ -7223,7 +7245,8 @@  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));
+						     GE_OPT_PREFIX, REJECT_ABSENT,
+						     NORMAL_RESOLUTION));
 	    /* 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
@@ -9605,8 +9628,7 @@  md_apply_fix (fixS * fixP, valueT * valP
 
   /* Note whether this will delete the relocation.  */
 
-  if (fixP->fx_addsy == 0 && !fixP->fx_pcrel
-      && aarch64_force_reloc (fixP->fx_r_type) <= 0)
+  if (fixP->fx_addsy == 0 && !fixP->fx_pcrel)
     fixP->fx_done = 1;
 
   /* Process the relocations.  */
--- a/gas/expr.h
+++ b/gas/expr.h
@@ -163,6 +163,7 @@  enum expr_mode
 #define expression(result) expr (0, result, expr_normal)
 #define expression_and_evaluate(result) expr (0, result, expr_evaluate)
 #define deferred_expression(result) expr (0, result, expr_defer)
+#define latched_dot_expression(result) expr (0, result, expr_defer_latch_dot)
 
 /* If an expression is O_big, look here for its value. These common
    data may be clobbered whenever expr() is called.  */
--- a/gas/testsuite/gas/aarch64/pr27217.d
+++ b/gas/testsuite/gas/aarch64/pr27217.d
@@ -10,8 +10,19 @@  Disassembly of section \.text:
 
 0+000 <.*>:
 [ 	]+0:[ 	]+90000000[ 	]+adrp[ 	]+x0, [0-9]*[ 	]+<.*>
-[ 	]+0:[ 	]+R_AARCH64(|_P32)_ADR_PREL_PG_HI21[ 	]+\*ABS\*\+0x12345678
+[ 	]+0:[ 	]+R_AARCH64(|_P32)_ADR_PREL_PG_HI21[ 	]+bar
 [ 	]+4:[ 	]+91000000[ 	]+add[ 	]+x0, x0, #0x0
-[ 	]+4:[ 	]+R_AARCH64(|_P32)_ADD_ABS_LO12_NC[ 	]+\*ABS\*\+0x12345678
+[ 	]+4:[ 	]+R_AARCH64(|_P32)_ADD_ABS_LO12_NC[ 	]+bar
 [ 	]+8:[ 	]+d65f03c0[ 	]+ret
+#...
+0+010 <.*>:
+[ 	]+10:[ 	]+90000000[ 	]+adrp[ 	]+x0, [0-9]*[ 	]+<.*>
+[ 	]+10:[ 	]+R_AARCH64(|_P32)_ADR_PREL_PG_HI21[ 	]+\.text\+0x10
+[ 	]+14:[ 	]+91000000[ 	]+add[ 	]+x0, x0, #0x0
+[ 	]+14:[ 	]+R_AARCH64(|_P32)_ADD_ABS_LO12_NC[ 	]+\.text\+0x10
+[ 	]+18:[ 	]+90000001[ 	]+adrp[ 	]+x1, [0-9]*[ 	]+<.*>
+[ 	]+18:[ 	]+R_AARCH64(|_P32)_ADR_PREL_PG_HI21[ 	]+\.text\+0x1c
+[ 	]+1c:[ 	]+91000021[ 	]+add[ 	]+x1, x1, #0x0
+[ 	]+1c:[ 	]+R_AARCH64(|_P32)_ADD_ABS_LO12_NC[ 	]+\.text\+0x1c
+[ 	]+20:[ 	]+d65f03c0[ 	]+ret
 #pass
--- a/gas/testsuite/gas/aarch64/pr27217.s
+++ b/gas/testsuite/gas/aarch64/pr27217.s
@@ -13,4 +13,13 @@  foo:
 	add	x0, x0, :lo12:bar
 	ret
 	.size	foo, .-foo
-	.ident	"GCC: (GNU) 10.2.1 20201030 (RTEMS 6, RSB "
+
+	.p2align 4
+	.type	dot, %function
+dot:
+	adrp	x0, .
+	add	x0, x0, :lo12:. - 4
+	adrp	x1, . + 4
+	add	x1, x1, :lo12:.
+	ret
+	.size	dot, .-dot