diff mbox series

[RFC,5/9] gas: add new command line option --scfi[=all,none]

Message ID 20230920230401.1739139-6-indu.bhagat@oracle.com
State New
Headers
Series SCFI implementation in GNU assembler |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Testing passed

Commit Message

Indu Bhagat Sept. 20, 2023, 11:03 p.m. UTC 
  When the command line option --scfi (default is --scfi=all) is passed to
the GNU assembler, it will synthesize DWARF call frame information (CFI)
for the assembly.

The option --scfi=all ignores any existing .cfi_* directives already
contained in the provided assembler file.

To use SCFI, a target will need to:
    - define TARGET_USE_SCFI and TARGET_USE_GINSN, and other necessary
    definitions,
    - provide implementation of some APIs to help GAS understand the
    target specific semantics.

The --scfi=[all,none] may see more options added in future.  For
example, --scfi=inline, for dealing with inline asm may be added in the
future.  In this option, the GNU assembler will consume (and not ignore)
the compiler generated CFI for the code surrounding the inline asm.

Also document the option.

gas/
        * as.c (parse_args): Add support for --scfi.
        * as.h (enum synth_cfi_type): Define new type.
        * doc/as.texi: Document the new option.
---
 gas/as.c        | 20 +++++++++++++++++++-
 gas/as.h        |  8 ++++++++
 gas/doc/as.texi |  5 +++++
 3 files changed, 32 insertions(+), 1 deletion(-)

Comments

Nick Clifton Sept. 28, 2023, 2:04 p.m. UTC | #1 
Hi Indu,

> When the command line option --scfi (default is --scfi=all) is passed to
> the GNU assembler, it will synthesize DWARF call frame information (CFI)
> for the assembly.

> Also document the option.

Please could you also add an entry in the gas/NEWS file about this new feature.

> +# ifdef TARGET_USE_SCFI
> +  fprintf (stream, _("\
> +  --scfi=[all,none]	  synthesize DWARF CFI for hand-written asm (not inline)\n\
> +			  (default --scfi=all)\n"));
> +# endif

I was wondering how the code would detect inline assembler.
Presumably it looks for the #NOAPP preprocessor directive ?
Or is the point of the comment in the above delta that the
user should not use this new option in conjunction with actual
compiled code ?  Either way I think that you need to extend
the documentation to include describing what happens when
compiled code and this option are used together.

Also - I have not checked this yet - if this option is enabled
and no CFI statements are generated (or maybe if an incomplete
sequence is generated) - presumably because of badly written
code - will there be an error message generated ?

Cheers
   Nick
Indu Bhagat Sept. 30, 2023, 6:13 a.m. UTC | #2 
On 9/28/23 07:04, Nick Clifton wrote:
> Hi Indu,
> 
>> When the command line option --scfi (default is --scfi=all) is passed to
>> the GNU assembler, it will synthesize DWARF call frame information (CFI)
>> for the assembly.
> 
>> Also document the option.
> 
> Please could you also add an entry in the gas/NEWS file about this new 
> feature.
> 

OK.

>> +# ifdef TARGET_USE_SCFI
>> +  fprintf (stream, _("\
>> +  --scfi=[all,none]      synthesize DWARF CFI for hand-written asm 
>> (not inline)\n\
>> +              (default --scfi=all)\n"));
>> +# endif
> 
> I was wondering how the code would detect inline assembler.
> Presumably it looks for the #NOAPP preprocessor directive ?

Yes.

> Or is the point of the comment in the above delta that the
> user should not use this new option in conjunction with actual
> compiled code ?  Either way I think that you need to extend
> the documentation to include describing what happens when
> compiled code and this option are used together.
> 

The intent was to convey to user that they must use:
   --scfi=all for hand-written asm,
   --scfi=inline if they have functions with inline asm.

I agree this is not currently reflected clearly in documentation. I will 
update it.

If --scfi=all is used for inline asm, this means SCFI machinery is being 
fed the compiler generated assembly. When --scfi=all is specified, most 
of the existing CFI directives are ignored.  Now, with using --scfi=all 
for inline asm, there is risk that SCFI machinery is not able to 
generate CFI, as compiler generated code may be "too complex" for SCFI 
to handle (like indirect jumps, jump table, DRAP register to realign 
stack etc, ...), and no CFI is synthesized (and the compiler generated 
CFI for the compiler generated asm also gets thrown away).

Hence the recommendation to use --scfi=inline for inline asm, where the 
assembler will not ignore the already existing CFI and will only 
generate CFI for the inline asm stubs.

> Also - I have not checked this yet - if this option is enabled
> and no CFI statements are generated (or maybe if an incomplete
> sequence is generated) - presumably because of badly written
> code - will there be an error message generated ?
> 

Yes, I intend to cover all cases where SCFI fails to synthesize CFI and 
issue an appropriate warning/error to user for each function.

Thanks
Indu
diff mbox series

Patch

diff --git a/gas/as.c b/gas/as.c
index 6839c841588..523169e66e3 100644
--- a/gas/as.c
+++ b/gas/as.c
@@ -372,6 +372,11 @@  Options:\n\
   -R                      fold data section into text section\n"));
   fprintf (stream, _("\
   --reduce-memory-overheads ignored\n"));
+# ifdef TARGET_USE_SCFI
+  fprintf (stream, _("\
+  --scfi=[all,none]	  synthesize DWARF CFI for hand-written asm (not inline)\n\
+			  (default --scfi=all)\n"));
+# endif
   fprintf (stream, _("\
   --statistics            print various measured statistics from execution\n"));
   fprintf (stream, _("\
@@ -511,7 +516,8 @@  parse_args (int * pargc, char *** pargv)
       OPTION_NOCOMPRESS_DEBUG,
       OPTION_NO_PAD_SECTIONS,
       OPTION_MULTIBYTE_HANDLING,  /* = STD_BASE + 40 */
-      OPTION_SFRAME
+      OPTION_SFRAME,
+      OPTION_SCFI
     /* When you add options here, check that they do
        not collide with OPTION_MD_BASE.  See as.h.  */
     };
@@ -586,6 +592,9 @@  parse_args (int * pargc, char *** pargv)
     ,{"no-pad-sections", no_argument, NULL, OPTION_NO_PAD_SECTIONS}
     ,{"no-warn", no_argument, NULL, 'W'}
     ,{"reduce-memory-overheads", no_argument, NULL, OPTION_REDUCE_MEMORY_OVERHEADS}
+#ifdef TARGET_USE_SCFI
+    ,{"scfi", no_argument, NULL, OPTION_SCFI}
+#endif
     ,{"statistics", no_argument, NULL, OPTION_STATISTICS}
     ,{"strip-local-absolute", no_argument, NULL, OPTION_STRIP_LOCAL_ABSOLUTE}
     ,{"version", no_argument, NULL, OPTION_VERSION}
@@ -982,6 +991,15 @@  This program has absolutely no warranty.\n"));
 	  flag_execstack = 0;
 	  break;
 
+	case OPTION_SCFI:
+	  if (!optarg || strcasecmp (optarg, "all") == 0)
+	    flag_synth_cfi = SYNTH_CFI_ALL;
+	  else if (strcasecmp (optarg, "none") == 0)
+	    flag_synth_cfi = SYNTH_CFI_NONE;
+	  else
+	    as_fatal (_("Invalid --scfi= option: `%s'"), optarg);
+	  break;
+
 	case OPTION_SIZE_CHECK:
 	  if (strcasecmp (optarg, "error") == 0)
 	    flag_allow_nonconst_size = false;
diff --git a/gas/as.h b/gas/as.h
index 99ffe77afd2..42fc44bd494 100644
--- a/gas/as.h
+++ b/gas/as.h
@@ -322,6 +322,14 @@  COMMON int flag_fatal_warnings; /* --fatal-warnings */
    are detected.  */
 COMMON unsigned char flag_always_generate_output; /* -Z */
 
+enum synth_cfi_type
+{
+  SYNTH_CFI_NONE = 0,
+  SYNTH_CFI_ALL,
+};
+
+COMMON enum synth_cfi_type flag_synth_cfi;
+
 /* This is true if the assembler should output time and space usage.  */
 COMMON unsigned char flag_print_statistics;
 
diff --git a/gas/doc/as.texi b/gas/doc/as.texi
index 6a3e5eed39f..c4755721407 100644
--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -255,6 +255,7 @@  gcc(1), ld(1), and the Info entries for @file{binutils} and @file{ld}.
  [@b{--multibyte-handling=[allow|warn|warn-sym-only]}]
  [@b{--no-pad-sections}]
  [@b{-o} @var{objfile}] [@b{-R}]
+ [@b{--scfi=[all,none]}]
  [@b{--sectname-subst}]
  [@b{--size-check=[error|warning]}]
  [@b{--statistics}]
@@ -932,6 +933,10 @@  Ignored.  Supported for compatibility with tools that apss the same option to
 both the assembler and the linker.
 
 @ifset ELF
+@item --scfi=all
+@itemx --scfi=none
+Synthesize CFI for hand-written asm.
+
 @item --sectname-subst
 Honor substitution sequences in section names.
 @ifclear man