[PATCH] gas: fix silent failure of .cfi_sections .sframe on unsupported targets

[Indu Bhagat]

On 3/10/26 5:23 AM, Jens Remus wrote:
> On 3/10/2026 7:07 AM, Indu Bhagat via Binutils wrote:
>> [Resending as I messed up one of the email in the to list in the
>> previous attempt].
>>
>> Starting in Binutils 2.46, the .cfi_sections .sframe directive failed
>> silently when targeting unsupported architectures (e.g., i386 or even
>> x32 on x86_64).  This happened because the sframe_as_bad macro only
>> checked for the command-line flag --gsframe (GEN_SFRAME_ENABLED) and
>> ignored the explicit request via the assembler directive
>> (CFI_EMIT_sframe).
>>
>> This patch ensures that an error is emitted (for unsupported targets) if
>> SFrame is requested via either the command line or the .cfi_sections
>> directive, while maintaining silence for 'opportunistic' sframe
>> generation (GEN_SFRAME_CONFIG_ENABLED).
>>
>> Add two testcases:
>>    - cfi-sframe-x86_64-err-1.d: usage of --32 with .cfi_sections .sframe,
>>    - cfi-sframe-x86_64-err-2.d: usage of --gsframe --32 without
>>      .cfi_sections .sframe
>> GAS is expected to error out in both the cases.
>>
>> This fixes PR gas/33962 - .cfi_sections directive with .sframe on
>> unsupported targets does not error.
>>
>> gas/
>> 	PR gas/33962
>> 	* dw2gencfi.c (dot_cfi_sections): Ensure as_bad is called if
>> 	SFrame was explicitly requested via directive.
>> 	* gen-sframe.h: Remove sframe_as_bad macro.
>> gas/testsuite/
>> 	PR gas/33962
>> 	* gas/cfi-sframe/cfi-sframe-x86_64-err-1.s: New test.
>> 	* gas/cfi-sframe/cfi-sframe-x86_64-err-1.d: New test.
>> 	* gas/cfi-sframe/cfi-sframe-x86_64-err-2.s: New test.
>> 	* gas/cfi-sframe/cfi-sframe-x86_64-err-1.d: New test.
>> 	* gas/cfi-sframe/cfi-sframe.exp: Run the new tests.
> 
> Thank you for fixing this issue!
> 
> 
>> diff --git a/gas/dw2gencfi.c b/gas/dw2gencfi.c
> 
>> @@ -2620,9 +2620,18 @@ cfi_finish (void)
>>   	}
>>         else
>>   #endif
>> -	/* Avoid erroring with DEFAULT_SFRAME for non-default options, like
>> -	   -32 on x86_64.  */
>> -	sframe_as_bad ("%s", _(".sframe not supported for target"));
>> +	/* Issue an error for unsupported targets, like --32 on x86_64.  Avoid
>> +	   erroring when default-enabled at configure-time though, because we
>> +	   interpret default-enabled as "opportunistic SFrames".  Users don't
>> +	   want to be bothered by something preventing emission of SFrames in
>> +	   such a case.  */
>> +	{
>> +	  if (flag_gen_sframe == GEN_SFRAME_ENABLED
> 
> This issues an error for --gsframe[-N].
> 

Yes. Because command line takes precedence.

>> +	      || ((all_cfi_sections & CFI_EMIT_sframe) != 0
>> +		  && flag_gen_sframe != GEN_SFRAME_DISABLED
>> +		  && flag_gen_sframe != GEN_SFRAME_CONFIG_ENABLED))
> 
> IIUC this issues an error for .cfi_sections .sframe, but only if not
> --gsframe=no, which takes precedence, and not 'opportunistic' SFrame
> generation (GEN_SFRAME_CONFIG_ENABLED).
> 

Correct.

> Why should .cfi_sections .sframe be ignored when used with
> 'opportunistic' SFrame generation (GEN_SFRAME_CONFIG_ENABLED)?
> 

Umm, .cfi_sections .sframe is not being ignored with 
GEN_SFRAME_CONFIG_ENABLED, its only that the error is silenced when this 
occurs for unsupported target.

I think you meant to ask why silence that error:
'Opportunistic' SFrame is for the use case of distro wide builds.  If 
there is user code that builds with --32 or --x32 (in case x86_64), and 
gas is built with --enable-default-sframe, there will be errors of 
unsupported target.  These are considered noise in this setup, and hence 
the desire to silence them.

Does that make sense ?

>> +	    as_bad (_(".sframe not supported for target"));
>> +	}
>>       }
>>   
>>     if ((all_cfi_sections & CFI_EMIT_debug_frame) != 0)
> 
>> diff --git a/gas/gen-sframe.h b/gas/gen-sframe.h
> 
>> @@ -21,16 +21,6 @@
>>   #ifndef GENSFRAME_H
>>   #define GENSFRAME_H
>>   
>> -/* Errors shouldn't be emitted either if SFrames are default-enabled, as
>> -   we interpret default-enabled as "opportunistic SFrames".  Users don't
>> -   want to be bothered by something preventing emission of SFrames in
>> -   such a case.  */
>> -#define sframe_as_bad(format, ...) \
>> -  do {					       \
>> -    if (flag_gen_sframe == GEN_SFRAME_ENABLED) \
>> -      as_bad (format, __VA_ARGS__);            \
>> -  } while (0)
>> -
>>   /* The entity is not tracked.  */
>>   #define SFRAME_FRE_ELEM_LOC_NONE	0
>>   /* The location of the tracked entity is based on a register.  May or may not
> Thanks and regards,
> Jens