[RFC,1/2] gas: also maintain signed-ness for O_big expressions

Message ID 3808733b-a63f-45fa-b53f-568f9bb5b14c@suse.com
State New
Headers
Series gas: O_big signed-ness |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Skipped upon request
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Skipped upon request

Commit Message

Jan Beulich Jan. 22, 2025, 9:09 a.m. UTC
  Interestingly emit_leb128_expr() already assumes X_unsigned is properly
set for O_big. Adjust its conversion-to-bignum to respect the incoming
flag, and have convert_to_bignum() correctly set it on output.

It further can't be quite right that convert_to_bignum() depends on
anything other than the incoming expression. Therefore adjust
emit_expr_with_reloc() to be in line with the other invocation.

This also requires an adjustment for SH, which really should have been
part of 762acf217c40 ("gas: maintain O_constant signedness in more
cases").
---
While for some more values .octa now emits correct data, for some other
inputs its behavior remains questionable. Furthermore while .octa and
.sleb128 now emit the same (wrong) numerical value for e.g.
"0x7fffffffffffffff * -2", this is effectively a regression for .octa
(if we were to assume that .octa with such expressions is supposed to
work in the first place). The problem, aiui, is that X_unsigned and
X_extrabit aren't properly updated when constant expressions are
resolved right away (i.e. while still parsing the expression), unlike
for unary operators. See also
https://sourceware.org/pipermail/binutils/2024-December/138146.html.
  

Patch

--- a/gas/config/tc-sh.c
+++ b/gas/config/tc-sh.c
@@ -544,6 +544,7 @@  sh_optimize_expr (expressionS *l, operat
       add_to_result (l, symval_diff, symval_diff < 0);
       l->X_op = O_constant;
       l->X_add_symbol = 0;
+      l->X_unsigned = 0;
       return 1;
     }
   return 0;
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -632,6 +632,7 @@  integer_constant (int radix, expressionS
       /* Not a small number.  */
       expressionP->X_op = O_big;
       expressionP->X_add_number = number;	/* Number of littlenums.  */
+      expressionP->X_unsigned = 1;
       input_line_pointer--;	/* -> char following number.  */
     }
 }
@@ -707,6 +708,7 @@  mri_char_constant (expressionS *expressi
     {
       expressionP->X_op = O_big;
       expressionP->X_add_number = i;
+      expressionP->X_unsigned = 1;
     }
   else
     {
@@ -1154,6 +1156,8 @@  operand (expressionS *expressionP, enum
 		      if (generic_bignum[i])
 			break;
 		    }
+
+		expressionP->X_unsigned = 0;
 	      }
 	    else if (op == O_logical_not)
 	      {
--- a/gas/expr.h
+++ b/gas/expr.h
@@ -131,10 +131,12 @@  typedef struct expressionS
   unsigned char X_op;
 #endif
 
-  /* Non-zero if X_add_number should be regarded as unsigned.  This is
-     only valid for O_constant expressions.  It is only used when an
-     O_constant must be extended into a bignum (i.e., it is not used
-     when performing arithmetic on these values).
+  /* Non-zero if the expression value should be regarded as unsigned.  This is
+     only valid for
+     - O_constant expressions, where it is only used when an O_constant must be
+       extended into a bignum (i.e., it is not used when performing arithmetic
+       on these values),
+     - O_big integer expressions, i.e. when X_add_number is positive.
      FIXME: This field is not set very reliably.  */
   unsigned int X_unsigned : 1;
   /* This is used to implement "word size + 1 bit" arithmetic, so that e.g.
--- a/gas/read.c
+++ b/gas/read.c
@@ -1477,6 +1477,7 @@  convert_to_bignum (expressionS *exp, int
     generic_bignum[i++] = sign ? LITTLENUM_MASK : 0;
   exp->X_op = O_big;
   exp->X_add_number = i;
+  exp->X_unsigned = !sign;
 }
 
 /* For most MRI pseudo-ops, the line actually ends at the first
@@ -4646,8 +4647,8 @@  emit_expr_with_reloc (expressionS *exp,
      pass to md_number_to_chars, handle it as a bignum.  */
   if (op == O_constant && nbytes > sizeof (valueT))
     {
-      extra_digit = exp->X_unsigned ? 0 : -1;
-      convert_to_bignum (exp, !exp->X_unsigned);
+      extra_digit = exp->X_unsigned ? 0 : -exp->X_extrabit;
+      convert_to_bignum (exp, -extra_digit);
       op = O_big;
     }
 
@@ -5366,12 +5367,14 @@  emit_leb128_expr (expressionS *exp, int
     }
   else if (op == O_constant
 	   && sign
-	   && (exp->X_add_number < 0) == !exp->X_extrabit)
+	   && (exp->X_unsigned
+	       ? exp->X_add_number < 0
+	       : (exp->X_add_number < 0) != exp->X_extrabit))
     {
       /* We're outputting a signed leb128 and the sign of X_add_number
 	 doesn't reflect the sign of the original value.  Convert EXP
 	 to a correctly-extended bignum instead.  */
-      convert_to_bignum (exp, exp->X_extrabit);
+      convert_to_bignum (exp, !exp->X_unsigned && exp->X_extrabit);
       op = O_big;
     }
 
--- a/gas/testsuite/gas/all/octa.d
+++ b/gas/testsuite/gas/all/octa.d
@@ -14,3 +14,11 @@  Contents of section .data:
  [^ ]* (00000000 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 00000000) .*
  [^ ]* (00000080 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 80000000) .*
  [^ ]* (01000000 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 00000001) .*
+ [^ ]* (00000000 000000f0 ffffffff ffffffff|ffffffff ffffffff f0000000 00000000) .*
+ [^ ]* (00000000 00000010 00000000 00000000|00000000 00000000 10000000 00000000) .*
+ [^ ]* (ffffffff ffffffef ffffffff ffffffff|ffffffff ffffffff efffffff ffffffff) .*
+ [^ ]* (01000000 00000010 00000000 00000000|00000000 00000000 10000000 00000001) .*
+ [^ ]* (01000000 000000f0 ffffffff ffffffff|ffffffff ffffffff f0000000 00000001) .*
+ [^ ]* (feffffff ffffff0f 00000000 00000000|00000000 00000000 0fffffff fffffffe) .*
+ [^ ]* (02000000 000000f0 ffffffff ffffffff|ffffffff ffffffff f0000000 00000002) .*
+ [^ ]* (fdffffff ffffff0f 00000000 00000000|00000000 00000000 0fffffff fffffffd) .*
--- a/gas/testsuite/gas/all/octa.s
+++ b/gas/testsuite/gas/all/octa.s
@@ -9,3 +9,13 @@ 
  .octa ~0xffffffff
  .octa 0 - 0x80000000
  .octa 0 - 0xffffffff
+
+ .octa ~0xfffffffffffffff
+ .octa -~0xfffffffffffffff
+ .octa ~-~0xfffffffffffffff
+ .octa -~-~0xfffffffffffffff
+
+ .octa -0xfffffffffffffff
+ .octa ~-0xfffffffffffffff
+ .octa -~-0xfffffffffffffff
+ .octa ~-~-0xfffffffffffffff