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

[Indu Bhagat]

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.

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.