From: Jens Remus <jremus@linux.ibm.com>
To: Indu Bhagat <indu.bhagat@oracle.com>
Cc: Andreas Krebbel <krebbel@linux.ibm.com>, binutils@sourceware.org
Subject: Re: [PATCH v3 10/15] gas: Skip SFrame FDE if FP without RA on stack
Date: Thu, 18 Apr 2024 12:27:51 +0200 [thread overview]
Message-ID: <cf09606f-c770-4c7f-a418-1c195583da77@linux.ibm.com> (raw)
In-Reply-To: <9bbfb6a8-c83c-484c-922b-cef90603f663@oracle.com>
Am 18.04.2024 um 01:56 schrieb Indu Bhagat:
> 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
Hello Indu,
thanks for confirming! Opened:
https://sourceware.org/bugzilla/show_bug.cgi?id=31654
Regards,
Jens
--
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+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/
next prev parent reply other threads:[~2024-04-18 10:28 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 14:47 [PATCH v3 00/15] sframe: Enhancements to SFrame info generation Jens Remus
2024-04-12 14:47 ` [PATCH v3 01/15] x86: Remove unused SFrame CFI RA register variable Jens Remus
2024-04-12 14:47 ` [PATCH v3 02/15] gas: Enhance arch-specific SFrame configuration descriptions Jens Remus
2024-04-18 7:39 ` Indu Bhagat
2024-05-03 12:30 ` Jens Remus
2024-04-12 14:47 ` [PATCH v3 03/15] readelf/objdump: Dump SFrame CFA fixed FP and RA offsets Jens Remus
2024-04-18 7:39 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 04/15] readelf/objdump: Display SFrame fixed RA offset as 'f' in dump Jens Remus
2024-04-18 7:40 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 05/15] gas: Print DWARF call frame insn name in SFrame warning message Jens Remus
2024-04-18 7:40 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 06/15] gas: Skip SFrame FDE if CFI specifies non-FP/SP base register Jens Remus
2024-04-18 7:40 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 07/15] gas: Warn if SFrame FDE is skipped due to non-default return column Jens Remus
2024-04-18 7:40 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 08/15] gas: Refactor SFrame CFI opcode DW_CFA_register processing Jens Remus
2024-04-18 7:40 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 09/15] gas: User readable warnings if SFrame FDE is not generated Jens Remus
2024-04-18 20:33 ` Indu Bhagat
2024-05-03 12:30 ` Jens Remus
2024-05-03 23:41 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 10/15] gas: Skip SFrame FDE if FP without RA on stack Jens Remus
2024-04-16 13:14 ` Jens Remus
2024-04-17 23:56 ` Indu Bhagat
2024-04-18 10:27 ` Jens Remus [this message]
2024-04-18 20:35 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 11/15] gas: Skip SFrame FDE if .cfi_window_save Jens Remus
2024-04-18 20:36 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 12/15] gas: Don't skip SFrame FDE if .cfi_register specifies RA w/o tracking Jens Remus
2024-04-18 20:36 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 13/15] gas: Don't skip SFrame FDE if .cfi_register specifies SP register Jens Remus
2024-04-18 20:37 ` Indu Bhagat
2024-04-19 13:13 ` Jens Remus
2024-04-23 8:15 ` Indu Bhagat
2024-04-25 22:22 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 14/15] gas: Test predicate whether SFrame RA tracking is used Jens Remus
2024-04-18 20:37 ` Indu Bhagat
2024-04-12 14:47 ` [PATCH v3 15/15] gas: Validate SFrame RA tracking and fixed RA offset Jens Remus
2024-04-18 20:38 ` Indu Bhagat
2024-05-03 16:40 ` Jens Remus
2024-05-04 0:22 ` Indu Bhagat
2024-05-06 11:41 ` Jens Remus
2024-05-06 14:39 ` Jens Remus
2024-05-16 20:45 ` Indu Bhagat
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cf09606f-c770-4c7f-a418-1c195583da77@linux.ibm.com \
--to=jremus@linux.ibm.com \
--cc=binutils@sourceware.org \
--cc=indu.bhagat@oracle.com \
--cc=krebbel@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).