[PATCH] gas: sframe: command line option takes precedence

[Jan Beulich]

On 18.07.2025 21:06, Indu Bhagat wrote:
> On 7/18/25 11:33 AM, Indu Bhagat wrote:
>> On 7/18/25 1:25 AM, Jan Beulich wrote:
>>> On 18.07.2025 09:21, Indu Bhagat via Binutils wrote:
>>>> --- a/gas/as.h
>>>> +++ b/gas/as.h
>>>> @@ -355,8 +355,15 @@ COMMON int flag_execstack;
>>>>   /* TRUE if .note.GNU-stack section with SEC_CODE should be created */
>>>>   COMMON int flag_noexecstack;
>>>> -/* TRUE if .sframe section should be created.  */
>>>> -COMMON int flag_gen_sframe;
>>>> +enum gen_sframe_option
>>>> +{
>>>> +  GEN_SFRAME_NONE = 0,
>>>
>>> Maybe better GEN_SFRAME_DEFAULT, seeing Sam's plan to allow controlling the
>>> default from configure?
>>>
>>
>> I think GEN_SFRAME_DEFAULT will be confusing.  Related response below..

As will be NONE - it can be read as similar/equivalent to DISABLED.

>>>> --- a/gas/dw2gencfi.c
>>>> +++ b/gas/dw2gencfi.c
>>>> @@ -2596,10 +2596,13 @@ cfi_finish (void)
>>>>         flag_traditional_format = save_flag_traditional_format;
>>>>       }
>>>> -  /* Generate SFrame section if the user specifies:
>>>> -    - the command line option to gas, or
>>>> -    - .sframe in the .cfi_sections directive.  */
>>>> -  if (flag_gen_sframe || (all_cfi_sections & CFI_EMIT_sframe) != 0)
>>>> +  /* Generate SFrame section if the user:
>>>> +    - enables via the command line option, or
>>>> +    - specifies .sframe in the .cfi_sections directive and does not disable
>>>> +      via the command line.  */
>>>> +  if (flag_gen_sframe == GEN_SFRAME_ENABLED
>>>> +      || ((all_cfi_sections & CFI_EMIT_sframe) != 0
>>>> +      && flag_gen_sframe != GEN_SFRAME_DISABLED))
>>>
>>> With said plan, I'm not sure about this. What's the intended behavior if
>>> the default is "disabled"? It feels like you need to resolve the default
>>> anyway after having parsed the command line. At which point I'm not even
>>> sure anymore that a tristate is needed here.
>>>
>>
>> The intended behavior IMO should be:
>>    - command line takes precedence over configure time setting and cfi_section directive.
>>    - .cfi_section directive takes precedence over configure time setting.
>>
>> (The command line takes precedence over configure time defaults.)  So Sam's patch to add configure-time enablement will basically just initialize the value of flag_gen_sframe to GEN_SFRAME_ENABLED or GEN_SFRAME_DISABLED based on --enable-sframe/--disable-sframe respectively at an appropriate time.
>>
> 
> Correction: IMO Sam's patch to add configure-time enablement will basically just initialize the value of flag_gen_sframe to GEN_SFRAME_ENABLED or GEN_SFRAME_NONE based on --enable-sframe/--disable-sframe respectively at an appropriate time.
> 
> When --disable-sframe is used, the desired behavior is equivalent to 'passing no --gsframe', and not '--gsframe=no'.

That's one of (at least) two possible behaviors. Sam, can you clarify what
the plan is?

Jan