[PATCH v3 2/5] elf: Add SFrame stack tracing

Claudiu Zissulescu-Ianculescu claudiu.zissulescu-ianculescu@oracle.com
Thu Apr 17 09:53:55 GMT 2025


Hello Jens,

>> +       location.  */
> 
> Nit: Bad indentation.
> 
>> +      ra_stack_loc = cfa + ra_offset;
>> +      return_addr = read_stack_value (ra_stack_loc);
>> +
>> +      ra_lst[i] = (void *)return_addr;
>> +
>> +      /* Set up for the next frame.  */
> 
> Maybe move comment (see below)?
> 
>> +      /* Get the FP offset from the FRE.  If the offset is
>> +     unavailable, sets err.  */
>> +      fp_offset = sframe_fre_get_fp_offset (dctx, frep, &err);
>> +      if (err == _URC_NO_REASON)
>> +    {
>> +      /* Frame is aavailable, get the value stored in the stack
>> +         location.  */
> 
> Nit: s/aavailable/available/. Maybe also s/Frame/FP offset/?
> 
>> +      fp_stack_loc = cfa + fp_offset;
>> +      frame->fp = read_stack_value (fp_stack_loc);
> 
> This is the only frame field being set immediately.  While it cannot
> occur that frame->fp gets set and below is not executed, I wonder
> whether introducing a local variable frame_ptr (or fp and renaming
> return_addr to ra) and changing below as follows would be cleaner?
> 
>> +    }
>> +
> 
>       /* Set up for the next frame.  */
> 
>> +      frame->sp = cfa + SFRAME_SP_VAL_OFFSET;
> 
>       frame->fp = frame_ptr;
> 

Thank you for your feedback! I will send a new version asap, which
includes your fixes and 32bit build & run fixes.

Best wishes,
Claudiu


More information about the Libc-alpha mailing list