[binutils-gdb] gas: initialisation of expressionS in operand()

[Alan Modra]

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

commit aa8956639252a6af92e41fdc57401471b919b96a
Author: Alan Modra <amodra@gmail.com>
Date:   Sat Oct 4 08:37:02 2025 +0930

    gas: initialisation of expressionS in operand()
    
    This patch removes clean_up_expression which runs just before operand()
    returns.  clean_up_expression sets as yet uninitialised fields of
    expressionS.  Well, it sets fields based on the value of X_op,
    trusting that others have been written, and has one notable exception:
    X_md is not initialised.
    
    Instead initialise expressionS fully inside operand(), which is called
    at the start of expr(), and introduce md_expr_init for the odd
    backends that want to mess with X_md.
    
    This is in response to an oss-fuzz report that read.c:pseudo_set calls
    expr() leaving exp.X_md uninitialised and can copy that to a symbol
    via symbol_set_value_expression.  tc-i386-intel.c:565 is one place
    that later tests the uninitialised X_md.
    
            * config/tc-z80.h (md_expr_init, md_expr_init_rest): Define.
            * config/tc-microblaze.h: Likewise.
            * expr.c (clean_up_expression): Delete.
            (operand): Init expression early.
            (expr): Use md_expr_init_rest to init X_md when necessary.

Diff:
---
 gas/config/tc-microblaze.h |  6 ++++++
 gas/config/tc-z80.h        |  6 ++++++
 gas/expr.c                 | 51 +++++++++-------------------------------------
 3 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/gas/config/tc-microblaze.h b/gas/config/tc-microblaze.h
index 6ac1b4a8235..1d0b56af033 100644
--- a/gas/config/tc-microblaze.h
+++ b/gas/config/tc-microblaze.h
@@ -57,6 +57,12 @@ extern bfd_reloc_code_real_type parse_cons_expression_microblaze
 #define tc_fix_adjustable(X)  tc_microblaze_fix_adjustable(X)
 extern int tc_microblaze_fix_adjustable (struct fix *);
 
+/* X_md is managed by the backend.  */
+#define md_expr_init(exp) \
+  do memset ((exp), 0, offsetof (expressionS, X_md)); while (0)
+#define md_expr_init_rest(exp) \
+  do (exp)->X_md = 0; while (0)
+
 extern const struct relax_type md_relax_table[];
 #define TC_GENERIC_RELAX_TABLE md_relax_table
 
diff --git a/gas/config/tc-z80.h b/gas/config/tc-z80.h
index 25f58e607c6..50e09124ad6 100644
--- a/gas/config/tc-z80.h
+++ b/gas/config/tc-z80.h
@@ -39,6 +39,12 @@
    will point to the start of the expression.  */
 #define md_operand(x)
 
+/* X_md is managed by the backend.  */
+#define md_expr_init(exp) \
+  do memset ((exp), 0, offsetof (expressionS, X_md)); while (0)
+#define md_expr_init_rest(exp) \
+  do (exp)->X_md = 0; while (0)
+
 /* This should just call either `number_to_chars_bigendian' or
    `number_to_chars_littleendian', whichever is appropriate.  On
    targets like the MIPS which support options to change the
diff --git a/gas/expr.c b/gas/expr.c
index aaad288a8c8..b75cce9f781 100644
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -35,8 +35,6 @@
 
 bool literal_prefix_dollar_hex = false;
 
-static void clean_up_expression (expressionS * expressionP);
-
 /* We keep a mapping of expression symbols to file positions, so that
    we can provide better error messages.  */
 
@@ -801,14 +799,19 @@ operand (expressionS *expressionP, enum expr_mode mode)
   segT segment;
   operatorT op = O_absent; /* For unary operators.  */
 
+#ifdef md_expr_init
+  md_expr_init (expressionP);
+#else
+  memset (expressionP, 0, sizeof (*expressionP));
+#endif
+
   /* All integers are regarded as unsigned unless they are negated.
      This is because the only thing which cares whether a number is
      unsigned is the code in emit_expr which extends constants into
      bignums.  It should only sign extend negative numbers, so that
      something like ``.quad 0x80000000'' is not sign extended even
      though it appears negative if valueT is 32 bits.  */
-  expressionP->X_unsigned = 1;
-  expressionP->X_extrabit = 0;
+  expressionP->X_unsigned = 1;				\
 
   /* Digits, assume it is a bignum.  */
 
@@ -1451,9 +1454,6 @@ operand (expressionS *expressionP, enum expr_mode mode)
       break;
     }
 
-  /* It is more 'efficient' to clean up the expressionS when they are
-     created.  Doing it here saves lines of code.  */
-  clean_up_expression (expressionP);
   SKIP_ALL_WHITESPACE ();		/* -> 1st char after operand.  */
   know (!is_whitespace (*input_line_pointer));
 
@@ -1480,39 +1480,6 @@ operand (expressionS *expressionP, enum expr_mode mode)
     }
 }
 

-/* Internal.  Simplify a struct expression for use by expr ().  */
-
-/* In:	address of an expressionS.
-	The X_op field of the expressionS may only take certain values.
-	Elsewise we waste time special-case testing. Sigh. Ditto SEG_ABSENT.
-
-   Out:	expressionS may have been modified:
-	Unused fields zeroed to help expr ().  */
-
-static void
-clean_up_expression (expressionS *expressionP)
-{
-  switch (expressionP->X_op)
-    {
-    case O_illegal:
-    case O_absent:
-      expressionP->X_add_number = 0;
-      /* Fall through.  */
-    case O_big:
-    case O_constant:
-    case O_register:
-      expressionP->X_add_symbol = NULL;
-      /* Fall through.  */
-    case O_symbol:
-    case O_uminus:
-    case O_bit_not:
-      expressionP->X_op_symbol = NULL;
-      break;
-    default:
-      break;
-    }
-}
-

 /* Expression parser.  */
 
 /* We allow an empty expression, and just assume (absolute,0) silently.
@@ -1888,7 +1855,9 @@ expr (int rankarg,		/* Larger # is higher rank.  */
 
       input_line_pointer += op_chars;	/* -> after operator.  */
 
-      right.X_md = 0;
+#ifdef md_expr_init_rest
+      md_expr_init_rest (&right);
+#endif
       rightseg = expr (op_rank[op_left], &right, mode);
       if (right.X_op == O_absent)
 	{