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

[Jan Beulich]

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.

> 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?

>   /* 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

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