Re: [PATCH v3 10/15] gas: Skip SFrame FDE if FP without RA on stack

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

On 4/16/24 06:14, Jens Remus wrote:
> Hello Indu,
> 
> Am 12.04.2024 um 16:47 schrieb Jens Remus:
>> The SFrame format cannot represent the frame pointer (FP) being saved
>> on the stack without the return address (RA) also being saved on the
>> stack, if RA tracking is used.
> 
> [...]
> 
>> diff --git a/gas/gen-sframe.c b/gas/gen-sframe.c
>> index a3b6f75cfe85..87be3eb05ad2 100644
>> --- a/gas/gen-sframe.c
>> +++ b/gas/gen-sframe.c
>> @@ -1439,6 +1439,25 @@ sframe_do_fde (struct sframe_xlate_ctx *xlate_ctx,
>>       = get_dw_fde_end_addrS (xlate_ctx->dw_fde);
>>       }
>> +#ifdef SFRAME_FRE_RA_TRACKING
>> +  if (sframe_ra_tracking_p ())
>> +    {
>> +      struct sframe_row_entry *fre;
>> +
>> +      /* Iterate over the scratchpad FREs and validate them.  */
>> +      for (fre = xlate_ctx->first_fre; fre; fre = fre->next)
>> +    {
>> +      /* SFrame format cannot represent FP on stack without RA on 
>> stack.  */
>> +      if (fre->ra_loc != SFRAME_FRE_ELEM_LOC_STACK
>> +          && fre->bp_loc == SFRAME_FRE_ELEM_LOC_STACK)
>> +        {
>> +          as_warn (_("skipping SFrame FDE due to FP without RA on 
>> stack"));
>> +          return SFRAME_XLATE_ERR_NOTREPRESENTED;
>> +        }
>> +    }
>> +    }
>> +#endif /* SFRAME_FRE_RA_TRACKING  */
>> +
>>     return SFRAME_XLATE_OK;
>>   }
> 
> I noticed that above new warning is erroneously emitted when assembling 
> the following CFI directive sequence with option "-alh" (to output a 
> listing of the assembly; probably any "-a[...]") on a SFrame enabled 
> target, that uses FP and RA tracking.
> 
> .cfi_offset <fp-regno>, <fp-offset>
> .cfi_offset <ra-regno>, <ra-offset>
> 
> The reason is that with listings enabled there is an additional DWARF 
> DW_CFA_advance_loc CFI instruction (with a zero advance) between both 
> DW_CFA_offset instructions, that the DWARF .eh_frame generator is able 
> to process correctly, but causes the .sframe generator to choke.
> 
> Additionally with this patch reverted "bad" SFrame information is 
> generated (see example below), where there are multiple SFrame FREs for 
> the same PC start address.
> Note that the FP-tracking information erroneously being displayed in the 
> RA-tracking column, is why I introduced this new warning message. I will 
> send two alternative patches how to potentially resolve that soon.
> 
> $ cat test_fpra_min.s
>          .cfi_sections .sframe, .eh_frame
>          .cfi_startproc
>          stmg    %r11,%r15,88(%r15)
>          .cfi_rel_offset 11, 88
>          .cfi_rel_offset 14, 112
>          la      %r11,0
>          la      %r14,0
> .Lreturn:
>          lmg     %r11,%r15,88(%r15)
>          .cfi_restore 14
>          .cfi_restore 11
>          br      %r14
>          .cfi_endproc
> 
> $ ojbdump --sframe test_fpra_without-alh.o
> ...
>    Function Index :
> 
>      func idx [0]: pc = 0x0, size = 22 bytes
>      STARTPC         CFA       FP        RA
>      0000000000000000  sp+160    u         u
>      0000000000000006  sp+160    c-72      c-48
>      0000000000000014  sp+160    u         u
> 
> $ objdump --sframe test_fpra_with_alh.o
> ...
>    Function Index :
> 
>      func idx [0]: pc = 0x0, size = 22 bytes
>      STARTPC         CFA       FP        RA
>      0000000000000000  sp+160    u         u
>      0000000000000006  sp+160    u         c-72
>      0000000000000006  sp+160    c-72      c-48
>      0000000000000014  sp+160    u         c-72
>      0000000000000014  sp+160    u         u
> 
> Note that the outputs of "objdump -Wf" and "objdump -WF" are identical 
> in both cases (with and without option "-alh").
> 
> Debugging of the SFrame processing of the DWARF CFI instructions shows 
> that with option "-a" there are additional DW_CFA_advance_loc:
> 
> DW_CFA_def_cfa: reg=15 offset=160
> DW_CFA_advance_loc: lab1=L0, lab2=L0
> DW_CFA_offset: reg=11 offset=-72
> DW_CFA_advance_loc: lab1=L0, lab2=L0   <-- only with -a
> DW_CFA_offset: reg=14 offset=-48
> DW_CFA_advance_loc: lab1=L0, lab2=L0
> DW_CFA_restore: reg=14
> DW_CFA_advance_loc: lab1=L0, lab2=L0   <-- only with -a
> DW_CFA_restore: reg=11
> 
> Debugging of the CFI directive processing in gas/dw2gencfi.c shows the 
> following:
> 
> - With option "-a" cfi_add_advance_loc() is invoked more often in 
> dot_cfi() due to the condition (symbol_get_frag 
> (frchain_now->frch_cfi_data->last_address) != frag_now) evaluating to true.
> 
> - output_cfi_insn() of case DW_CFA_advance_loc enters the condition 
> (symbol_get_frag (to) == symbol_get_frag (from)) without option "-a" and 
> enters the else condition with option "-a". The else path has an 
> interesting comment that suggests that there is logic to relax an 
> advance by zero at a later stage:
> 
> "... Call frag_grow with the sum of room needed by frag_more and 
> frag_var to preallocate space ensuring that the DW_CFA_advance_loc4 is 
> in the fixed part of the rs_cfa frag, so that the relax machinery can 
> remove the advance_loc should it advance by zero."
> 
> I don't have a clue how to resolve this potential issue in the SFrame 
> generation. I could not figure out how to detect the advance of zero in 
> the SFrame processing of DW_CFA_advance_loc, so that it could be treated 
> special.
> I can open a ticket in the Sourceware Bugzilla, if you agree that this 
> is an issue.
> 

Hi Jens,

Confirming that its reproducible and that this is an issue in the 
existing implementation.  Please go ahead and create a bugzilla.

Thanks