[binutils-gdb] gas: sframe: fine tune the fragment fixup for SFrame func info

[binutils-gdb] gas: sframe: fine tune the fragment fixup for SFrame func info

[Indu Bhagat]

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

commit 989aabcb564dccc7804f174fc582b0639357a9bb
Author: Indu Bhagat <indu.bhagat@oracle.com>
Date:   Fri Dec 9 10:25:14 2022 -0800

    gas: sframe: fine tune the fragment fixup for SFrame func info
    
    SFrame function info is an unsigned 8-bit field comprising of the following
    (from LSB to MSB):
      - 4-bits: FRE type
      - 1-bit: FRE start address encoding
      - 3-bits: Unused
    
    At the moment, the most-significat 4-bits are zero (The FRE start
    address encoding of SFRAME_FDE_TYPE_PCINC has a value of zero, and the upper
    3-bits are unused). So the current implementation works without this patch.
    
    To be precise, however, the fragment fixup logic is meant to fixup only the
    least-significant 4-bits (i.e., only the FRE type needs to be updated
    according to the function size).
    
    This patch makes the gas implementation a bit more resilient: In the
    future, when the format does evolve to make use of the currently unused
    3-bits in various ways, the values in those 3-bits can be propagated
    unchanged while the fragment fixup continues to update the lowermost
    4-bits to indicate the selected FRE type.
    
    ChangeLog:
    
            * gas/gen-sframe.c (create_func_info_exp): New definition.
            (output_sframe_funcdesc): Call create_func_info_exp.
            * gas/sframe-opt.c (sframe_estimate_size_before_relax): The
            associated fragment uses O_modulus now.
            (sframe_convert_frag): Adjust the fragment fixup code according
            to the new composite exp.

Diff:
---
 gas/gen-sframe.c | 55 ++++++++++++++++++++++++++++++++++++++++++++-----------
 gas/sframe-opt.c | 34 ++++++++++++++++++++++++----------
 2 files changed, 68 insertions(+), 21 deletions(-)

diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
index 896fbe44c6b..075720facd6 100644
--- a/gas/gen-sframe.c
+++ b/gas/gen-sframe.c
@@ -396,8 +396,8 @@ sframe_get_fre_offset_size (struct sframe_row_entry *sframe_fre)
     - <val> and <width> are themselves expressionS.
     - <val> stores the expression which when evaluated gives the value of the
       start address offset of the FRE.
-    - <width> stores the expression when when evaluated gives the number of
-      bytes needed to encode the start address offset of the FRE.
+    - <width> stores the expression when evaluated gives the number of bytes
+      needed to encode the start address offset of the FRE.
 
    The use of OP_absent as the X_op_symbol helps identify this expression
    later when fragments are fixed up.  */
@@ -431,6 +431,41 @@ create_fre_start_addr_exp (expressionS *cexp, symbolS *fre_pc_begin,
   cexp->X_add_number = 0;
 }
 
+/* Create a composite exression CEXP (for SFrame FDE function info) such that:
+
+      exp = <rest_of_func_info> OP_modulus <width>, where,
+
+    - <rest_of_func_info> and <width> are themselves expressionS.
+    - <rest_of_func_info> stores a constant expression where X_add_number is
+    used to stash away the func_info.  The upper 4-bits of the func_info are copied
+    back to the resulting byte by the fragment fixup logic.
+    - <width> stores the expression when evaluated gives the size of the
+    funtion in number of bytes.
+
+   The use of OP_modulus as the X_op_symbol helps identify this expression
+   later when fragments are fixed up.  */
+
+static void
+create_func_info_exp (expressionS *cexp, symbolS *dw_fde_end_addrS,
+		      symbolS *dw_fde_start_addrS, uint8_t func_info)
+{
+  expressionS width;
+  expressionS rest_of_func_info;
+
+  width.X_op = O_subtract;
+  width.X_add_symbol = dw_fde_end_addrS;
+  width.X_op_symbol = dw_fde_start_addrS;
+  width.X_add_number = 0;
+
+  rest_of_func_info.X_op = O_constant;
+  rest_of_func_info.X_add_number = func_info;
+
+  cexp->X_op = O_modulus;
+  cexp->X_add_symbol = make_expr_symbol (&rest_of_func_info);
+  cexp->X_op_symbol = make_expr_symbol (&width);
+  cexp->X_add_number = 0;
+}
+
 #endif
 
 static void
@@ -538,19 +573,17 @@ output_sframe_funcdesc (symbolS *start_of_fre_section,
   out_four (sframe_fde->num_fres);
 
   /* SFrame FDE function info.  */
+  unsigned char func_info;
+  func_info = sframe_set_func_info (SFRAME_FDE_TYPE_PCINC,
+				    SFRAME_FRE_TYPE_ADDR4);
 #if SFRAME_FRE_TYPE_SELECTION_OPT
-  expressionS width;
-  width.X_op = O_subtract;
-  width.X_add_symbol = dw_fde_end_addrS;
-  width.X_op_symbol = dw_fde_start_addrS;
-  width.X_add_number = 0;
+  expressionS cexp;
+  create_func_info_exp (&cexp, dw_fde_end_addrS, dw_fde_start_addrS,
+			func_info);
   frag_grow (1); /* Size of func info is unsigned char.  */
   frag_var (rs_sframe, 1, 0, (relax_substateT) 0,
-	    make_expr_symbol (&width), 0, (char *) frag_now);
+	    make_expr_symbol (&cexp), 0, (char *) frag_now);
 #else
-  unsigned char func_info;
-  func_info = sframe_set_func_info (SFRAME_FDE_TYPE_PCINC,
-				    SFRAME_FRE_TYPE_ADDR4);
   out_one (func_info);
 #endif
 }
diff --git a/gas/sframe-opt.c b/gas/sframe-opt.c
index 6901aa82a77..f08a424fd88 100644
--- a/gas/sframe-opt.c
+++ b/gas/sframe-opt.c
@@ -40,10 +40,10 @@ sframe_estimate_size_before_relax (fragS *frag)
      The two kind of fragments can be differentiated based on the opcode
      of the symbol.  */
   exp = symbol_get_value_expression (frag->fr_symbol);
-  gas_assert ((exp->X_op == O_subtract) || (exp->X_op == O_absent));
+  gas_assert ((exp->X_op == O_modulus) || (exp->X_op == O_absent));
   /* Fragment for function info in an SFrame FDE will always write
      only one byte.  */
-  if (exp->X_op == O_subtract)
+  if (exp->X_op == O_modulus)
     ret = 1;
   /* Fragment for the start address in an SFrame FRE may write out
      1/2/4 bytes depending on the value of the diff.  */
@@ -92,8 +92,12 @@ sframe_convert_frag (fragS *frag)
   offsetT fsize;
   offsetT diff;
   offsetT value;
-  unsigned char func_info = SFRAME_FRE_TYPE_ADDR4;
+
+  offsetT rest_of_data;
+  uint8_t fde_type, fre_type;
+
   expressionS *exp;
+  symbolS *dataS;
   symbolS *fsizeS, *diffS;
 
   /* We are dealing with two different kind of fragments here which need
@@ -103,19 +107,29 @@ sframe_convert_frag (fragS *frag)
      The two kind of fragments can be differentiated based on the opcode
      of the symbol.  */
   exp = symbol_get_value_expression (frag->fr_symbol);
-  gas_assert ((exp->X_op == O_subtract) || (exp->X_op == O_absent));
+  gas_assert ((exp->X_op == O_modulus) || (exp->X_op == O_absent));
   /* Fragment for function info in an SFrame FDE.  */
-  if (exp->X_op == O_subtract)
+  if (exp->X_op == O_modulus)
     {
-      fsizeS = frag->fr_symbol;
+      /* Gather the existing value of the rest of the data except
+	 the fre_type.  */
+      dataS = exp->X_add_symbol;
+      rest_of_data = (symbol_get_value_expression(dataS))->X_add_number;
+      fde_type = SFRAME_V1_FUNC_FDE_TYPE (rest_of_data);
+      gas_assert (fde_type == SFRAME_FDE_TYPE_PCINC);
+
+      /* Calculate the applicable fre_type.  */
+      fsizeS = exp->X_op_symbol;
       fsize = resolve_symbol_value (fsizeS);
       if (fsize < SFRAME_FRE_TYPE_ADDR1_LIMIT)
-	func_info = SFRAME_FRE_TYPE_ADDR1;
+	fre_type = SFRAME_FRE_TYPE_ADDR1;
       else if (fsize < SFRAME_FRE_TYPE_ADDR2_LIMIT)
-	func_info = SFRAME_FRE_TYPE_ADDR2;
+	fre_type = SFRAME_FRE_TYPE_ADDR2;
       else
-	func_info = SFRAME_FRE_TYPE_ADDR4;
-      value = func_info;
+	fre_type = SFRAME_FRE_TYPE_ADDR4;
+
+      /* Create the new function info.  */
+      value = SFRAME_V1_FUNC_INFO (fde_type, fre_type);
 
       frag->fr_literal[frag->fr_fix] = value;
     }