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

[Indu Bhagat]

On 7/24/25 10:32 PM, Jan Beulich wrote:
> On 24.07.2025 18:14, Indu Bhagat wrote:
>> On 7/24/25 8:52 AM, Jan Beulich wrote:
>>> On 24.07.2025 07:52, Indu Bhagat via Binutils wrote:
>>>> over gas directive to emit .sframe section.
>>>>
>>>> Fix PR gas/33175 sframe: --gsframe=no does not disable when
>>>> .cfi_sections directive with .sframe
>>>>
>>>> --gsframe=no should also disable generation of SFrame section when
>>>> explicit CFI directive:
>>>>     .cfi_sections .sframe
>>>> is specified in the input.  This means we need to track whether SFrame
>>>> generation was explcitly disabled by the user.  Introduce a new enum to
>>>> facilitate disambiguation between GEN_SFRAME_CONFIG_DISABLED and
>>>> GEN_SFRAME_DISABLED.
>>>>
>>>> While fixing the bug by adding the enum, keep the upcoming requirement
>>>> in mind: we will also need to disambiguate between
>>>> --enable-default-sframe and user-specified --gsframe/--gsframe=yes.  The
>>>> intent is to not display SFrame related warnings or errors like:
>>>>
>>>>     as_bad (_(".sframe not supported for target"));
>>>>
>>>> for unsupported targets if --enable-default-sframe is in effect.
>>>>
>>>> This implies we need to have a four state enum (
>>>> GEN_SFRAME_CONFIG_DISABLED, GEN_SFRAME_CONFIG_ENABLED,
>>>> GEN_SFRAME_DISABLED, GEN_SFRAME_ENABLED)
>>>
>>> Before I look at the change in detail: What is the supposed behavior for
>>> every one of these? (A code comment on the enumerators would have been
>>> nice.)
>>>
>>
>> I can add code comments to reflect that.  Perhaps like following (hopefully that helps answer your question too).
> 
> Yes, it addresses my question.
> 
>> /* PR gas/33175.
>>     Add enumerators to disambiguate between configure-time
>>     enablement/disablement vs user-specficied enablement/disablement (the latter
>>     via command line).  The expected usage of these states is:  command line
>>     takes precedence over configure time setting.  */
> 
> This could do with making things explicit as to auto-generation vs.
> .cfi_* directive induced handling.
> 

OK.

Now I have:

/* PR gas/33175.
    Add enumerators to disambiguate between configure-time
    enablement/disablement vs user-specficied enablement/disablement 
(the latter
    via command line).  The expected usage of these states is:
      - user-specified command line takes precedence over configure-time
        setting and .cfi_sections directive usage.
      - .cfi_sections usage takes precedence over configure-time 
setting.  */

>> enum gen_sframe_option
>> {
>>    /* SFrame generation disabled at configure time.  Default.  */
> 
> And the behavior then is? By the wording, it would mean no SFrame at all,
> command line option or not. Just that there is the comment ahead of the
> enumeration. Personally, as written I'd read these as contradictory with
> one another.
> 
>>    GEN_SFRAME_CONFIG_DISABLED,
> 
> Maybe GEN_SFRAME_CONFIG_DISABLED simply isn't a good name, as it suggests
> stronger disabling than there is. Maybe GEN_SFRAME_DEFAULT_OFF or
> GEN_SFRAME_DEFAULT_DISABLED?
> 

Right. As such there will be a --enable-default-sframe or such, not 
really a configure-time disablement.  So, GEN_SFRAME_DEFAULT_OFF is 
good.  Also GEN_SFRAME_DEFAULT_NONE is OK ?

(I can send V3 soon after this is settled).  But this is how it is now 
after addressing your comments :

/* PR gas/33175.
    Add enumerators to disambiguate between configure-time
    enablement/disablement vs user-specficied enablement/disablement 
(the latter
    via command line).  The expected usage of these states is:
      - user-specified command line takes precedence over configure-time
        setting and .cfi_sections directive usage.
      - .cfi_sections usage takes precedence over configure-time 
setting.  */
enum gen_sframe_option
{
   /* Default. SFrame generation not enabled at configure time.  GNU as will
      not generate SFrame sections by default, unless enabled by user via
      command line.  */
   GEN_SFRAME_DEFAULT_NONE,
   /* SFrame generation enabled at configure time.  GNU as will generate 
SFrame
      sections for all objects, unless disabled by user via command 
line.  */
   GEN_SFRAME_CONFIG_ENABLED,
   /* User specified disablement via --gsframe=no.  */
   GEN_SFRAME_DISABLED,
   /* User specified enablement via --gsframe or --gsframe=yes.  */
   GEN_SFRAME_ENABLED,
};





>>    /* SFrame generation enabled at configure time.  GNU as will generate SFrame
>>       sections for all objects, unless disabled by user via command line.  */
> 
> I think the equivalent of the 2nd sentence here would want to be in the
> earlier comment.
> 
> Jan
> 

OK.

>>    GEN_SFRAME_CONFIG_ENABLED,
>>    /* User specified disablement via --gsframe=no.  */
>>    GEN_SFRAME_DISABLED,
>>    /* User specified enablement via --gsframe or --gsframe=yes.  */
>>    GEN_SFRAME_ENABLED,
>> };
> 
>