[PATCH 1/5] gas: add --enable-default-sframe configure option

[Sam James]

Jan Beulich <jbeulich@suse.com> writes:

> On 15.08.2025 16:56, Sam James wrote:
>> SFrames make the most sense when userland as a whole is built with them,
>> so add a --enable-default-sframe configure flag to facilitate distributors
>> and vendors doing that.
>> 
>> The primary motivation for a configure-time flag is that we don't support
>> SFrame for 32-bit x86 but some packaging uses the same flags (with some
>> added on top) for multilib builds (to support old binaries like games),
>> and simply adding `-Wa,--gsframe` to the standard build flags isn't an
>> option (*).
>> 
>> That aside, I believe it'll be helpful for testing and eventual adoption
>> in any case.
>> 
>> In summary, combined with the recent --gsframe=[yes|no] support:
>> * Configured with --enable-default-sframe and nothing is passed
>>   => SFrames (previously no SFrames)
>> * Configured with --enable-default-sframe and --gsframe=yes is passed
>>   => SFrames (no change from before)
>> * Configured with --enable-default-sframe and --gsframe=no is passed
>>   => No SFrames (no change from before)
>> * Configured with --enable-default-sframe and --gsframe is passed
>>   => SFrames (no change from before)
>> 
>> * Configured with --disable-default-sframe and nothing is passed
>>   => No SFrames (no change from before)
>> * Configured with --disable-default-sframe and --gsframe=yes is passed
>>   => SFrames (no change from before)
>> * Configured with --disable-default-sframe and --gsframe=no is passed
>>   => No SFrames (no change from before)
>> * Configured with --disable-default-sframe and --gsframe is passed
>>   => SFrames (no change from before)
>
> Since you mention them separately: Am I under the wrong impression that
> --gsframe and --gsframe=yes are actually synonyms?

Your impression is correct, I only listed it explicitly because of some
confusion earlier and wanted to be clear to everybody what the semantics
are.

IIRC when adding --gsframe=yes|no support, Indu had a question about
what it meant --gsframe did, but I'm happy to drop it too. I'll think
about it and may (may not) tweak the commit message accordingly before pushing.

>
>> I've introduced a sframe_as_bad macro on Indu's suggestion. A following
>> patch uses its sibling sframe_as_warn heavily and having symmetry plus
>> the macro as a form of documentation of intent seems useful.
>> 
>> (*) It gets added to multilib builds too and then we hit the
>>     `.sframe not supported for target` error in gas/dw2gencfi.c.
>> 
>> gas/
>> 	PR gas/33126
>> 	* as.c (enum gen_sframe_option): Initialize if DEFAULT_SFRAME.
>> 	* config.in (DEFAULT_SFRAME): New.
>> 	* configure: Regenerate.
>> 	* configure.ac: Add --enable-default-sframe.
>> 	* doc/as.texi: Document --enable-default-sframe.
>> 	* dw2gencfi.c (cfi_finish): Don't warn if SFrames are enabled
>> 	by default but unavailable for this target.
>> 	* gen-sframe.h (sframe_as_bad): New macro.
>
> Looks okay to me, just one nit:
>
>> --- a/gas/dw2gencfi.c
>> +++ b/gas/dw2gencfi.c
>> @@ -2598,9 +2598,10 @@ cfi_finish (void)
>>  
>>    /* Generate SFrame section if the user:
>>  	- enables via the command line option, or
>> +	- default-enabled at configure-time via --enable-default-sframe, or
>>  	- specifies .sframe in the .cfi_sections directive and does not disable
>>  	  via the command line.  */
>> -  if (flag_gen_sframe == GEN_SFRAME_ENABLED
>> +  if (flag_gen_sframe == GEN_SFRAME_ENABLED || flag_gen_sframe == GEN_SFRAME_CONFIG_ENABLED
>
> Line too long.
>
> Feel free to put in with this adjusted.

Thanks for the review. I'll fix that up and push this one in a little bit.

>
> Jan

sam