Re: [RFC PATCH 0/1] sframe: Represent FP without RA on stack (bitmap)

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

On 4/23/24 02:54, Jens Remus wrote:
> Am 23.04.2024 um 01:59 schrieb Indu Bhagat:
>> On 4/22/24 08:59, Jens Remus wrote:
>>> This patch series adds support in SFrame to represent the frame pointer
>>> (FP) without the return address (RA) being saved on the stack (and/or on
>>> s390x in another register).
>>>
>>> This is the second of two proposed alternatives:
>>> 1. The alternative patch series uses a dummy padding offset (invalid
>>>     offset from CFA value of zero) as RA offset to represent FP without
>>>     RA on saved on the stack.
>>> 2. This patch series changes the SFrame FRE count field into
>>>     a bitmap, to convey which offsets follow the FRE.
>>>
>>> Note that it currently applies on top of my v3 patch series series that
>>> adds initial support to generate .sframe from CFI directives on s390x,
>>> although it is independent of that.
>>>
>>> A SFrame FRE currently has a 4-bit field containing the offset count
>>> that follow the FRE. While this could account for up to 15 offsets (or
>>> 16, when excluding the mandatory CFA offset from CFA base register), it
>>> cannot represent which of these offsets actually follow.
>>>
>>> Redefining the 4-bit count field into a 4-bit offset bitmap allows to
>>> track up to 4 offsets (or 5, when excluding the mandatory CFA offset
>>> from CFA base register).
>>>
>>
>> This approach, in its current form, immensely confines the future 
>> adaptability of the format.
>>
>> My recommendation would be to avoid such changes to format where is 
>> becomes more restrictive for future needs.  While it is generally 
>> recommended to not add more registers to track, confining it now to 4 
>> (or 5) offsets now seems rather limiting.
> 
> Isn't it rather confining that using a simple offset count cannot 
> represent which of the tracked offsets follow a FRE, which effectively 
> enforces a precedence in which they can be tracked? I just wonder 
> whether it is really that useful to theoretically be able to track 15 
> (or 16) offsets with this limitation instead of just 3 (or 4) without 
> any limitations. The number of bits could also be increased in a future 
> SFrame version with a different FRE layout.
> 

While you do have a point, changing the 4-bits which are used to 
indicate "number of offsets" to "bitmap of offsets" does limit the 
flexibility of the format to support other architectures/ABIs in future.

We sort of envisioned that this array of stack offsets (a.k.a FRE 
offsets) can even be interpreted differently by each arch/ABI:

For AMD64:
offset1 = CFA offset
offset2 if present = FP offset

For aarch64:
offset1 = CFA offset
offset2 if present is RA offset
offset3 if present is FP offset
This works for aarch64, because Frame record (if created) saves both FP 
and LR on stack.

For s390:
The stack tracer may interpret offsets as needed. You may even store 
some metadata in one of the offsets:

offset1 = CFA offset
offset2 = metadata (= read next as FP)
offset3 = FP offset

offset1 = CFA offset
offset2 = metadata (= read next as RA)
offset3 = RA offset

offset1 = CFA offset
offset2 = metadata (= read next as FP, RA)
offset3 = FP offset
offset4 = RA offset

offset1 = CFA offset
offset2 = metadata (= read next as registers)
offset3 = RA register number
offset4 = FP register number

etc.

Your solution with padding to resolve the issue of "FP without RA" (and 
storing register number for leaf functions) in the FRE offset is what 
this is in spirit.  It is useful for flexibility as you see.

> I don't have enough knowledge of other architectures to judge, whether 
> the s390x case of an architecture using both FP and RA tracking while 
> not always saving both at once or restoring them individually is really 
> exotic or might be more widespread.
> 
> I would understand if changing SFrame V2 to a bitmap is too much of a 
> change and would need to wait until the next version. Using the dummy 
> padding offsets in V2 (on s390x) in the meantime, which are just a minor 
> change, would be a good compromise.
> 

Format bump if needed should be done.  My main concern about the bitmap 
solution is the loss of flexibility.  Its hard to predict the future 
needs, so it makes sense to not lose flexibility for adaptation.

>> I have two suggestions to resolve this issue of "FP without RA on 
>> stack".  I will reply on the other thread.
> 
> Thanks a lot for your feedback! I will follow up there.
> 
>>> The main downside of this approach is that this is potentially a major
>>> change to the SFrame V2 format, which may require a bump to V3. The
>>> benefits are that (1) it does not add any dummy padding offsets, which
>>> would unnecessarily add bloat to the SFrame information, and that (2)
>>> it does not change any of the external SFrame API.
>>> Using a lookup table the bitmap can easily be translated into an offset
>>> count. Similar any logic that checks the presence of an offset can
>>> easily be implemented using a bit test.
>>>
>>> Note that there is a minor implementation issue with regards to the
>>> internal API methods (callbacks), due to the change in SFrame format
>>> changing a method argument from count to bitmap.
>>> Additionally this initial implementation lacks better naming of the
>>> tracked register IDs and any update of the SFrame format specification.
>>>
>>> Thanks and regards,
>>> Jens
>>>
>>>
>>> Jens Remus (1):
>>>    sframe: Represent FP without RA on stack
>>>
>>>   gas/gen-sframe.c   | 66 ++++++++++++++++++++++++++++------------------
>>>   include/sframe.h   | 13 ++++++---
>>>   libsframe/sframe.c | 51 +++++++++++++++++++++++++++--------
>>>   3 files changed, 90 insertions(+), 40 deletions(-)
>>>
>>
> 
> Regards,
> Jens