[PATCH 1/2] libsframe: add new dump_sframe_reloc

[Jens Remus]

On 21.02.2025 15:48, Indu Bhagat wrote:
> On 2/21/25 12:54 AM, Jan Beulich wrote:
>> On 21.02.2025 01:43, Indu Bhagat wrote:
>>> On 2/19/25 1:38 AM, Jan Beulich wrote:
>>>> On 17.02.2025 17:58, Indu Bhagat wrote:
>>>>> --- a/libsframe/sframe.c
>>>>> +++ b/libsframe/sframe.c
>>>>> @@ -102,6 +102,37 @@ sframe_ret_set_errno (int *errp, int error)
>>>>>      return NULL;
>>>>>    }
>>>>> +/* If the input buffer containing the SFrame section has been relocated, there
>>>>> +   will be a need to do fixups too.  The fixup merely accounts for the offset
>>>>> +   of the byte from the start of the section.
>>>>> +
>>>>> +   Currently used by dump_sframe_reloc.  The caller must have decoded (and
>>>>> +   hence, endian flipped) the input buffer before calling this function.  */
>>>>> +
>>>>> +int
>>>>> +sframe_fde_tbl_reloc_fixup (sframe_decoder_ctx *dctx)
>>>>> +{
>>>>> +  uint8_t sframe_ver = sframe_decoder_get_version (dctx);
>>>>> +  uint32_t num_fdes = sframe_decoder_get_num_fidx (dctx);
>>>>> +  unsigned int buf_offset = 0;
>>>>> +  sframe_func_desc_entry *fde;
>>>>> +  uint32_t i = 0;
>>>>> +
>>>>> +  if (sframe_ver != SFRAME_VERSION_2 || !dctx->sfd_funcdesc)
>>>>> +    return SFRAME_ERR;
>>>>> +
>>>>> +  buf_offset += sframe_decoder_get_hdr_size (dctx);
>>>>> +  while (i < num_fdes)
>>>>> +    {
>>>>> +      fde = &dctx->sfd_funcdesc[i];
>>>>> +      fde->sfde_func_start_address += buf_offset;
>>>>> +      buf_offset += sizeof (sframe_func_desc_entry);
>>>>> +      i++;
>>>>> +    }
>>>>> +
>>>>> +  return 0;
>>>>> +}
>>>>
>>>> In the bug report, comment 1, you specifically provided
>>>>
>>>> $ readelf -r file.o
>>>> Relocation section '.rela.sframe' at offset 0x8728 contains 3 entries:
>>>>     Offset          Info           Type           Sym. Value    Sym. Name + Addend
>>>> 00000000001c  000200000002 R_X86_64_PC32     0000000000000000 .text + 0
>>>> 000000000030  000200000002 R_X86_64_PC32     0000000000000000 .text + 30
>>>> 000000000044  000200000002 R_X86_64_PC32     0000000000000000 .text + 40
>>>>
>>>> Having peeked also at patch 2, I fail to see where these relocations
>>>> are actually applied. The function above appears to be making assumptions
>>>> about where relocations need applying, and why type they are. Am I
>>>> overlooking anything?
>>>>
>>>
>>> The reloc machinery is invoked in the load_specific_debug_section ()
>>> functions.
>>
>> Yet then why is further fixing up necessary? And how to you know how to
>> do the fixups, when you don't further look at the relocations coming from
>> the object.
>>
> 
> IIUC, load_specific_debug_section () loads the debug section at address 0.  For PC-relative offsets, where the .text start address is also 0, load_specific_debug_section () calculates the S + A - P correctly.
> 
> In SFrame FDE start address, we would like to indicate the function start address (hence, the PC-relative relocation).  In that respect, the function does assume that there is PC-relative relocation at the start of every FDE (for function start address); something I think is OK to do as this is baked into the format.

I tried to wrap my head around it and came to the following conclusion:

Each FDE sfde_func_start_address is assumed to have a PC-relative
relocation (e.g. PC32), which gets calculated as S + A - P.

With S being the address of the function symbol, A = 0, and P being the
address of the field sfde_func_start_address the value of each FDE
sfde_func_start_address after relocation by load_specific_debug_section
is:

sfde_func_start_address = addr(function symbol) - addr(fde_func_start_address)

This requires fixup by address of sfde_func_start_address.  With the
assumption that the .sframe section got loaded at 0 the address of
sfde_func_start_address is identical to its offset within the section.
This is why sframe_fde_tbl_reloc_fixup() does add that to each.
With that:

sfde_func_start_address = addr(function symbol)

I guess the following in gas/gen-sframe.c, output_sframe_funcdesc()
implies that a PC-relative relocation is emitted?

   /* Start address of the function.  */
   exp.X_op = O_subtract;
   exp.X_add_symbol = dw_fde_start_addrS; /* to location.  */
   exp.X_op_symbol = symbol_temp_new_now (); /* from location.  */
   exp.X_add_number = 0;
   emit_expr (&exp, addr_size);

Did I get this correct?

Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/