[PATCH 1/2] libsframe: add new dump_sframe_reloc

[Indu Bhagat]

On 2/21/25 8:21 AM, Jens Remus wrote:
> 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:
> 

(A = 0 is incorrect above).

S is 0 in the context of textual dump, A is the addend. E.g.  for an 
object where func1, func2, func3 are at .text + 0, .text + 30, and .text 
0x40 respectively, we say:

Sym. Name + Addend
   .text + 0
   .text + 30
   .text + 40

> 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?
> 

Yes.

>    /* 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