Re: [PATCH v3] x86: adjust which Dwarf2 register numbers to use

[Indu Bhagat <indu.bhagat@oracle.com>]

On 2/23/24 03:07, Jan Beulich wrote:
> Consumers can't know which execution mode is in effect for a certain
> piece of code; they can only go from object file properties. Hence which
> register numbers to encode ought to depend solely on object file type.
> 
> In tc_x86_frame_initial_instructions() do away with parsing a register
> name: We have a symbolic constant already for the 64-bit case, and the
> 32-bit number isn't going to change either. Said constant's definition
> needs moving, though, to be available also for non-ELF. While moving
> also adjust the comment to clarify that it's applicable to 64-bit mode
> only.

I have no further comments.

Thanks for the patch,
Indu

> ---
> x86_cie_data_alignment, independent of this change, likely needs
> adjusting as flag_code changes.
> 
> The COFF/PE setting of x86_dwarf2_return_column looks bogus as well: 32
> is already in use for %xmm15 for 64-bit. Commit ca19b261ecc3 sadly has
> no explanation at all. Nor did it adjust objdump accordingly. Given that
> the author has moved on (couldn't find an applicable email address), I'm
> inclined to simply revert that change. If anything a proper complete set
> of Windows register number mappings (wherever those are formally
> documented) would need putting in place.
> ---
> v3: Also adjust tc_x86_frame_initial_instructions().
> v2: Also adjust md_begin().
> 
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -3276,7 +3276,7 @@ md_begin (void)
>         operand_chars[(unsigned char) *p] = *p;
>     }
>   
> -  if (flag_code == CODE_64BIT)
> +  if (object_64bit)
>       {
>   #if defined (OBJ_COFF) && defined (TE_PE)
>         x86_dwarf2_return_column = (OUTPUT_FLAVOR == bfd_target_coff_flavour
> @@ -5410,7 +5410,7 @@ ginsn_dw2_regnum (const reg_entry *ireg)
>     if (ireg->reg_num == RegIP || ireg->reg_num == RegIZ)
>       return GINSN_DW2_REGNUM_RSI_DUMMY;
>   
> -  dwarf_reg = ireg->dw2_regnum[flag_code >> 1];
> +  dwarf_reg = ireg->dw2_regnum[object_64bit];
>   
>     if (dwarf_reg == Dw2Inval)
>       {
> @@ -17548,7 +17548,7 @@ tc_x86_parse_to_dw2regnum (expressionS *
>         if ((addressT) exp->X_add_number < i386_regtab_size)
>   	{
>   	  exp->X_add_number = i386_regtab[exp->X_add_number]
> -			      .dw2_regnum[flag_code >> 1];
> +			      .dw2_regnum[object_64bit];
>   	  if (exp->X_add_number != Dw2Inval)
>   	    exp->X_op = O_constant;
>   	}
> @@ -17558,22 +17558,7 @@ tc_x86_parse_to_dw2regnum (expressionS *
>   void
>   tc_x86_frame_initial_instructions (void)
>   {
> -  static unsigned int sp_regno[2];
> -
> -  if (!sp_regno[flag_code >> 1])
> -    {
> -      char *saved_input = input_line_pointer;
> -      char sp[][4] = {"esp", "rsp"};
> -      expressionS exp;
> -
> -      input_line_pointer = sp[flag_code >> 1];
> -      tc_x86_parse_to_dw2regnum (&exp);
> -      gas_assert (exp.X_op == O_constant);
> -      sp_regno[flag_code >> 1] = exp.X_add_number;
> -      input_line_pointer = saved_input;
> -    }
> -
> -  cfi_add_CFA_def_cfa (sp_regno[flag_code >> 1], -x86_cie_data_alignment);
> +  cfi_add_CFA_def_cfa (object_64bit ? REG_SP : 4, -x86_cie_data_alignment);
>     cfi_add_CFA_offset (x86_dwarf2_return_column, x86_cie_data_alignment);
>   }
>   
> --- a/gas/config/tc-i386.h
> +++ b/gas/config/tc-i386.h
> @@ -396,6 +396,11 @@ extern void tc_x86_parse_to_dw2regnum (e
>   #define tc_cfi_frame_initial_instructions tc_x86_frame_initial_instructions
>   extern void tc_x86_frame_initial_instructions (void);
>   
> +/* DWARF register number of the frame-pointer register in 64-bit mode.  */
> +#define REG_FP 6
> +/* DWARF register number of the stack-pointer register in 64-bit mode.  */
> +#define REG_SP 7
> +
>   #define md_elf_section_type(str,len) i386_elf_section_type (str, len)
>   extern int i386_elf_section_type (const char *, size_t);
>   
> @@ -425,10 +430,6 @@ extern void x86_cleanup (void);
>      R15 (15).  Use SCFI_CALLEE_SAVED_REG_P to identify which registers
>      are callee-saved from this set.  */
>   #define SCFI_MAX_REG_ID 15
> -/* Identify the DWARF register number of the frame-pointer register.  */
> -#define REG_FP 6
> -/* Identify the DWARF register number of the stack-pointer register.  */
> -#define REG_SP 7
>   /* Some ABIs, like AMD64, use stack for call instruction.  For such an ABI,
>      identify the initial (CFA) offset from RSP at the entry of function.  */
>   #define SCFI_INIT_CFA_OFFSET 8