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

[Indu Bhagat]

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..
> 
>>> --- 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'.

> The options processing code in gas/as.c can remain the same:
> 
>          case OPTION_SFRAME:
>            if (optarg)
>              {
>                if (strcasecmp (optarg, "no") == 0)
>                  flag_gen_sframe = GEN_SFRAME_DISABLED;
>                else if (strcasecmp (optarg, "yes") == 0)
>                  flag_gen_sframe = GEN_SFRAME_ENABLED;
>                else
>                  as_fatal (_("Invalid --gsframe option: `%s'"), optarg);
>              }
>            else
>              flag_gen_sframe = GEN_SFRAME_ENABLED;
>            break;
> 
> Basically, with ".cfi_sections .sframe" we need to distinguish between 
> the two states:
>    - No command line option (=> generate SFrame)
>    - explicit --gsframe=no (=> no generate SFrame)
> and hence, the tristate.
> 
>>> --- a/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-1.d
>>> +++ b/gas/testsuite/gas/cfi-sframe/cfi-sframe-common-1.d
>>> @@ -1,4 +1,4 @@
>>> -#as: --gsframe
>>> +#as:
>>>   #objdump: --sframe=.sframe
>>>   #name: SFrame generation using CFI directive .cfi_sections
>>>   #...
>>
>> Wasn't this option just recently added here, to prepare for default-off
>> configurations?
>>
> 
> No, I dont see it.