[V2,06/10] gas: scfidw2gen: new functionality to prepapre for SCFI

  [Changes from the RFC patch set]
  - scfidw2gen.c now defines its own set of handlers.  For many of those
    CFI directives, we simply warn ("Warning: --scfi=all ignores some
    user-specified CFI directives") and return.
  - The above-mentioned warning is issued only once per file.
  - The advantage of adding new set of handlers in scfidwgen.c is that
    dw2gencfi now does not need to add a condition for each handler to
    simply return if --scfi is in effect (the latter was the approach in
    the RFC patch set.)
[End of changes]

Define a new set of handlers for CFI directives for the purpose of SCFI.
The SCFI machinery ignores many of the user-specified CFI direcives when
--scfi=all is in effect.  The following CFI directives, however, will not
ignored:
      - .cfi_sections
      - .cfi_label
      - .cfi_signal_frame

The handling of .cfi_label, and .cfi_signal_frame will be added in a
follow up patch, as it needs the ginsn implementation.

Since the option --scfi=inline still needs to be worked out, the
implementation in scfidw2gen may need to be adjusted later.

gas/
	* Makefile.am: Add new files to GAS_CFILES and HFILES.
	* Makefile.in: Likewise.
	* gas/read.c (scfi_pop_insert): New define.
	(pobegin): Use the SCFI handlers.
	* scfidw2gen.c: New file.
	* scfidw2gen.h: New file.
---
 gas/Makefile.am  |   2 +
 gas/Makefile.in  |   9 +-
 gas/read.c       |  17 ++-
 gas/scfidw2gen.c | 305 +++++++++++++++++++++++++++++++++++++++++++++++
 gas/scfidw2gen.h |  37 ++++++
 5 files changed, 365 insertions(+), 5 deletions(-)
 create mode 100644 gas/scfidw2gen.c
 create mode 100644 gas/scfidw2gen.h
  

On 30.10.2023 17:51, Indu Bhagat wrote:
> --- /dev/null
> +++ b/gas/scfidw2gen.c
> @@ -0,0 +1,305 @@
> +/* scfidw2gen.c - Support for emission of synthesized Dwarf2 CFI.
> +   Copyright (C) 2003-2023 Free Software Foundation, Inc.

Is this year range really applicable to this new file?

> +   This file is part of GAS, the GNU Assembler.
> +
> +   GAS is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GAS is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GAS; see the file COPYING.  If not, write to the Free
> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
> +   02110-1301, USA.  */
> +
> +#include "as.h"
> +#include "dw2gencfi.h"
> +#include "subsegs.h"
> +#include "scfidw2gen.h"
> +
> +#if defined (TARGET_USE_SCFI) && defined (TARGET_USE_GINSN)
> +
> +static int scfi_ignore_warn_once = 0;

Nit: bool please (and no real need for an initializer).

> +static void dot_scfi_sections (int);
> +static void dot_scfi_ignore (int);
> +static void dot_scfi (int);

May I suggest to avoid such forward declarations by moving ...

> +const pseudo_typeS scfi_pseudo_table[] =

... this table towards the bottom of the file?

> +  {
> +    { "cfi_sections", dot_scfi_sections, 0 }, /* No ignore.  */

Instead of three such individual comments, how about putting the three
relevant ones first, followed by a comment (serving as a separator) and
then all dot_scfi_ignore entries?

> +    { "cfi_startproc", dot_scfi_ignore, 0 },
> +    { "cfi_endproc", dot_scfi_ignore, 0 },
> +    { "cfi_fde_data", dot_scfi_ignore, 0 },
> +    { "cfi_def_cfa", dot_scfi_ignore, 0 },
> +    { "cfi_def_cfa_register", dot_scfi_ignore, 0 },
> +    { "cfi_def_cfa_offset", dot_scfi_ignore, 0 },
> +    { "cfi_adjust_cfa_offset", dot_scfi_ignore, 0 },
> +    { "cfi_offset", dot_scfi_ignore, 0 },
> +    { "cfi_rel_offset", dot_scfi_ignore, 0 },
> +    { "cfi_register", dot_scfi_ignore, 0 },
> +    { "cfi_return_column", dot_scfi_ignore, 0 },
> +    { "cfi_restore", dot_scfi_ignore, 0 },
> +    { "cfi_undefined", dot_scfi_ignore, 0 },
> +    { "cfi_same_value", dot_scfi_ignore, 0 },
> +    { "cfi_remember_state", dot_scfi_ignore, 0 },
> +    { "cfi_restore_state", dot_scfi_ignore, 0 },
> +    { "cfi_window_save", dot_scfi_ignore, 0 },
> +    { "cfi_negate_ra_state", dot_scfi_ignore, 0 },
> +    { "cfi_escape", dot_scfi_ignore, 0 },
> +    { "cfi_signal_frame", dot_scfi, CFI_signal_frame }, /* No ignore.  */
> +    { "cfi_personality", dot_scfi_ignore, 0 },
> +    { "cfi_personality_id", dot_scfi_ignore, 0 },
> +    { "cfi_lsda", dot_scfi_ignore, 0 },
> +    { "cfi_val_encoded_addr", dot_scfi_ignore, 0 },
> +    { "cfi_inline_lsda", dot_scfi_ignore, 0 },
> +    { "cfi_label", dot_scfi, CFI_label }, /* No ignore.  */
> +    { "cfi_val_offset", dot_scfi_ignore, 0 },
> +    { NULL, NULL, 0 }
> +  };
> +
> +static void
> +dot_scfi_ignore (int ignored ATTRIBUTE_UNUSED)
> +{
> +  gas_assert (flag_synth_cfi);
> +
> +  if (scfi_ignore_warn_once == 0)
> +    {
> +      as_warn (_("--scfi=all ignores some user-specified CFI directives"));

s/some/most/ ?

> +void
> +scfi_dot_cfi_startproc (symbolS *start_sym)

This and the following two functions presently have no caller, and I
also can't spot equivalents in dw2gencfi.c. How are they (going) to be
used? This ...

> +{
> +  if (frchain_now->frch_cfi_data != NULL)
> +    {
> +      as_bad (_("previous CFI entry not closed (missing .cfi_endproc)"));

... for example suggests that the function here might really be the
handler for .cfi_startproc, yet the table above says .cfi_startproc is
ignored.

> +#else
> +
> +static void
> +dot_scfi_dummy (int ignored ATTRIBUTE_UNUSED)
> +{
> +  as_bad (_("SCFI is not supported for this target"));
> +  ignore_rest_of_line ();
> +}
> +
> +const pseudo_typeS scfi_pseudo_table[] =
> +  {
> +    { "cfi_sections", dot_scfi_dummy, 0 },
> +    { "cfi_startproc", dot_scfi_dummy, 0 },
> +    { "cfi_endproc", dot_scfi_dummy, 0 },
> +    { "cfi_fde_data", dot_scfi_dummy, 0 },
> +    { "cfi_def_cfa", dot_scfi_dummy, 0 },
> +    { "cfi_def_cfa_register", dot_scfi_dummy, 0 },
> +    { "cfi_def_cfa_offset", dot_scfi_dummy, 0 },
> +    { "cfi_adjust_cfa_offset", dot_scfi_dummy, 0 },
> +    { "cfi_offset", dot_scfi_dummy, 0 },
> +    { "cfi_rel_offset", dot_scfi_dummy, 0 },
> +    { "cfi_register", dot_scfi_dummy, 0 },
> +    { "cfi_return_column", dot_scfi_dummy, 0 },
> +    { "cfi_restore", dot_scfi_dummy, 0 },
> +    { "cfi_undefined", dot_scfi_dummy, 0 },
> +    { "cfi_same_value", dot_scfi_dummy, 0 },
> +    { "cfi_remember_state", dot_scfi_dummy, 0 },
> +    { "cfi_restore_state", dot_scfi_dummy, 0 },
> +    { "cfi_window_save", dot_scfi_dummy, 0 },
> +    { "cfi_negate_ra_state", dot_scfi_dummy, 0 },
> +    { "cfi_escape", dot_scfi_dummy, 0 },
> +    { "cfi_signal_frame", dot_scfi_dummy, 0 },
> +    { "cfi_personality", dot_scfi_dummy, 0 },
> +    { "cfi_personality_id", dot_scfi_dummy, 0 },
> +    { "cfi_lsda", dot_scfi_dummy, 0 },
> +    { "cfi_val_encoded_addr", dot_scfi_dummy, 0 },
> +    { "cfi_inline_lsda", dot_scfi_dummy, 0 },
> +    { "cfi_label", dot_scfi_dummy, 0 },
> +    { "cfi_val_offset", dot_scfi_dummy, 0 },
> +    { NULL, NULL, 0 }
> +  };
> +
> +#endif

Is this really needed? Can't you simply error on use of the command
line option, without the need for the extra table and dummy handler?

> --- /dev/null
> +++ b/gas/scfidw2gen.h
> @@ -0,0 +1,37 @@
> +/* scfidw2gen.h - Support for emitting synthesized Dwarf2 CFI.
> +   Copyright (C) 2003-2023 Free Software Foundation, Inc.
> +
> +   This file is part of GAS, the GNU Assembler.
> +
> +   GAS is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3, or (at your option)
> +   any later version.
> +
> +   GAS is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with GAS; see the file COPYING.  If not, write to the Free
> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
> +   02110-1301, USA.  */
> +
> +#ifndef SCFIDW2GEN_H
> +#define SCFIDW2GEN_H
> +
> +#include "as.h"
> +#include "dwarf2.h"
> +
> +extern int all_cfi_sections;

This needs to go into dw2gencfi.h, such that dw2gencfi.c will also see
the declaration (and the compiler be able to check that declaration and
definition are in sync).

Jan
  

Hi Jan,

Thanks for reviewing.

On 10/31/23 04:28, Jan Beulich wrote:
> On 30.10.2023 17:51, Indu Bhagat wrote:
>> --- /dev/null
>> +++ b/gas/scfidw2gen.c
>> @@ -0,0 +1,305 @@
>> +/* scfidw2gen.c - Support for emission of synthesized Dwarf2 CFI.
>> +   Copyright (C) 2003-2023 Free Software Foundation, Inc.
> 
> Is this year range really applicable to this new file?
> 

No. I will fix it here and other files.

>> +   This file is part of GAS, the GNU Assembler.
>> +
>> +   GAS is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3, or (at your option)
>> +   any later version.
>> +
>> +   GAS is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with GAS; see the file COPYING.  If not, write to the Free
>> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
>> +   02110-1301, USA.  */
>> +
>> +#include "as.h"
>> +#include "dw2gencfi.h"
>> +#include "subsegs.h"
>> +#include "scfidw2gen.h"
>> +
>> +#if defined (TARGET_USE_SCFI) && defined (TARGET_USE_GINSN)
>> +
>> +static int scfi_ignore_warn_once = 0;
> 
> Nit: bool please (and no real need for an initializer).
> 

OK.

>> +static void dot_scfi_sections (int);
>> +static void dot_scfi_ignore (int);
>> +static void dot_scfi (int);
> 
> May I suggest to avoid such forward declarations by moving ...
> 
>> +const pseudo_typeS scfi_pseudo_table[] =
> 
> ... this table towards the bottom of the file?
> 

OK.

>> +  {
>> +    { "cfi_sections", dot_scfi_sections, 0 }, /* No ignore.  */
> 
> Instead of three such individual comments, how about putting the three
> relevant ones first, followed by a comment (serving as a separator) and
> then all dot_scfi_ignore entries?
> 

OK.

>> +    { "cfi_startproc", dot_scfi_ignore, 0 },
>> +    { "cfi_endproc", dot_scfi_ignore, 0 },
>> +    { "cfi_fde_data", dot_scfi_ignore, 0 },
>> +    { "cfi_def_cfa", dot_scfi_ignore, 0 },
>> +    { "cfi_def_cfa_register", dot_scfi_ignore, 0 },
>> +    { "cfi_def_cfa_offset", dot_scfi_ignore, 0 },
>> +    { "cfi_adjust_cfa_offset", dot_scfi_ignore, 0 },
>> +    { "cfi_offset", dot_scfi_ignore, 0 },
>> +    { "cfi_rel_offset", dot_scfi_ignore, 0 },
>> +    { "cfi_register", dot_scfi_ignore, 0 },
>> +    { "cfi_return_column", dot_scfi_ignore, 0 },
>> +    { "cfi_restore", dot_scfi_ignore, 0 },
>> +    { "cfi_undefined", dot_scfi_ignore, 0 },
>> +    { "cfi_same_value", dot_scfi_ignore, 0 },
>> +    { "cfi_remember_state", dot_scfi_ignore, 0 },
>> +    { "cfi_restore_state", dot_scfi_ignore, 0 },
>> +    { "cfi_window_save", dot_scfi_ignore, 0 },
>> +    { "cfi_negate_ra_state", dot_scfi_ignore, 0 },
>> +    { "cfi_escape", dot_scfi_ignore, 0 },
>> +    { "cfi_signal_frame", dot_scfi, CFI_signal_frame }, /* No ignore.  */
>> +    { "cfi_personality", dot_scfi_ignore, 0 },
>> +    { "cfi_personality_id", dot_scfi_ignore, 0 },
>> +    { "cfi_lsda", dot_scfi_ignore, 0 },
>> +    { "cfi_val_encoded_addr", dot_scfi_ignore, 0 },
>> +    { "cfi_inline_lsda", dot_scfi_ignore, 0 },
>> +    { "cfi_label", dot_scfi, CFI_label }, /* No ignore.  */
>> +    { "cfi_val_offset", dot_scfi_ignore, 0 },
>> +    { NULL, NULL, 0 }
>> +  };
>> +
>> +static void
>> +dot_scfi_ignore (int ignored ATTRIBUTE_UNUSED)
>> +{
>> +  gas_assert (flag_synth_cfi);
>> +
>> +  if (scfi_ignore_warn_once == 0)
>> +    {
>> +      as_warn (_("--scfi=all ignores some user-specified CFI directives"));
> 
> s/some/most/ ?
> 

Hmm. Technically, "most" is the correct choice. So, OK.

>> +void
>> +scfi_dot_cfi_startproc (symbolS *start_sym)
> 
> This and the following two functions presently have no caller, and I
> also can't spot equivalents in dw2gencfi.c. How are they (going) to be
> used? This ...
> 
>> +{
>> +  if (frchain_now->frch_cfi_data != NULL)
>> +    {
>> +      as_bad (_("previous CFI entry not closed (missing .cfi_endproc)"));
> 
> ... for example suggests that the function here might really be the
> handler for .cfi_startproc, yet the table above says .cfi_startproc is
> ignored.
> 

The callers of scfi_dot_cfi_startproc (), scfi_dot_cfi_endproc () and 
scfi_dot_cfi () are in scfi.c, when its time to emit DWARF CFI after 
SCFI machinery has generated the SCFI Ops (See scfi_dot_cfi () and 
scfi_emit_dw2cfi ()).  The callers are added in the next patch in the 
series, "[PATCH, V2 07/10] gas: synthesize CFI for hand-written asm".

>> +#else
>> +
>> +static void
>> +dot_scfi_dummy (int ignored ATTRIBUTE_UNUSED)
>> +{
>> +  as_bad (_("SCFI is not supported for this target"));
>> +  ignore_rest_of_line ();
>> +}
>> +
>> +const pseudo_typeS scfi_pseudo_table[] =
>> +  {
>> +    { "cfi_sections", dot_scfi_dummy, 0 },
>> +    { "cfi_startproc", dot_scfi_dummy, 0 },
>> +    { "cfi_endproc", dot_scfi_dummy, 0 },
>> +    { "cfi_fde_data", dot_scfi_dummy, 0 },
>> +    { "cfi_def_cfa", dot_scfi_dummy, 0 },
>> +    { "cfi_def_cfa_register", dot_scfi_dummy, 0 },
>> +    { "cfi_def_cfa_offset", dot_scfi_dummy, 0 },
>> +    { "cfi_adjust_cfa_offset", dot_scfi_dummy, 0 },
>> +    { "cfi_offset", dot_scfi_dummy, 0 },
>> +    { "cfi_rel_offset", dot_scfi_dummy, 0 },
>> +    { "cfi_register", dot_scfi_dummy, 0 },
>> +    { "cfi_return_column", dot_scfi_dummy, 0 },
>> +    { "cfi_restore", dot_scfi_dummy, 0 },
>> +    { "cfi_undefined", dot_scfi_dummy, 0 },
>> +    { "cfi_same_value", dot_scfi_dummy, 0 },
>> +    { "cfi_remember_state", dot_scfi_dummy, 0 },
>> +    { "cfi_restore_state", dot_scfi_dummy, 0 },
>> +    { "cfi_window_save", dot_scfi_dummy, 0 },
>> +    { "cfi_negate_ra_state", dot_scfi_dummy, 0 },
>> +    { "cfi_escape", dot_scfi_dummy, 0 },
>> +    { "cfi_signal_frame", dot_scfi_dummy, 0 },
>> +    { "cfi_personality", dot_scfi_dummy, 0 },
>> +    { "cfi_personality_id", dot_scfi_dummy, 0 },
>> +    { "cfi_lsda", dot_scfi_dummy, 0 },
>> +    { "cfi_val_encoded_addr", dot_scfi_dummy, 0 },
>> +    { "cfi_inline_lsda", dot_scfi_dummy, 0 },
>> +    { "cfi_label", dot_scfi_dummy, 0 },
>> +    { "cfi_val_offset", dot_scfi_dummy, 0 },
>> +    { NULL, NULL, 0 }
>> +  };
>> +
>> +#endif
> 
> Is this really needed? Can't you simply error on use of the command
> line option, without the need for the extra table and dummy handler?
> 

Actually, gas does not even present the option --scfi when the target 
does not have the two required defines TARGET_USE_SCFI  and 
TARGET_USE_GINSN.  So I could guard the code in pobegin () in read.c 
(and remove the dummy handlers in scfidw2genc.c):

   /* Now CFI ones.  */
#if defined (TARGET_USE_SCFI) && defined (TARGET_USE_GINSN)
   if (flag_synth_cfi)
     {
       pop_table_name = "scfi";
       scfi_pop_insert ();
     }
   else
     {
       pop_table_name = "cfi";
       cfi_pop_insert ();
     }
#else
   pop_table_name = "cfi";
   cfi_pop_insert ();
#endif


>> --- /dev/null
>> +++ b/gas/scfidw2gen.h
>> @@ -0,0 +1,37 @@
>> +/* scfidw2gen.h - Support for emitting synthesized Dwarf2 CFI.
>> +   Copyright (C) 2003-2023 Free Software Foundation, Inc.
>> +
>> +   This file is part of GAS, the GNU Assembler.
>> +
>> +   GAS is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3, or (at your option)
>> +   any later version.
>> +
>> +   GAS is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with GAS; see the file COPYING.  If not, write to the Free
>> +   Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA
>> +   02110-1301, USA.  */
>> +
>> +#ifndef SCFIDW2GEN_H
>> +#define SCFIDW2GEN_H
>> +
>> +#include "as.h"
>> +#include "dwarf2.h"
>> +
>> +extern int all_cfi_sections;
> 
> This needs to go into dw2gencfi.h, such that dw2gencfi.c will also see
> the declaration (and the compiler be able to check that declaration and
> definition are in sync).
> 

OK.
  

On 31.10.2023 23:06, Indu Bhagat wrote:
> On 10/31/23 04:28, Jan Beulich wrote:
>> On 30.10.2023 17:51, Indu Bhagat wrote:
>>> +void
>>> +scfi_dot_cfi_startproc (symbolS *start_sym)
>>
>> This and the following two functions presently have no caller, and I
>> also can't spot equivalents in dw2gencfi.c. How are they (going) to be
>> used? This ...
>>
>>> +{
>>> +  if (frchain_now->frch_cfi_data != NULL)
>>> +    {
>>> +      as_bad (_("previous CFI entry not closed (missing .cfi_endproc)"));
>>
>> ... for example suggests that the function here might really be the
>> handler for .cfi_startproc, yet the table above says .cfi_startproc is
>> ignored.
>>
> 
> The callers of scfi_dot_cfi_startproc (), scfi_dot_cfi_endproc () and 
> scfi_dot_cfi () are in scfi.c, when its time to emit DWARF CFI after 
> SCFI machinery has generated the SCFI Ops (See scfi_dot_cfi () and 
> scfi_emit_dw2cfi ()).  The callers are added in the next patch in the 
> series, "[PATCH, V2 07/10] gas: synthesize CFI for hand-written asm".

But that's not in the context of processing a .cfi_* directive. IOW the
message is properly misleading.

>>> +#else
>>> +
>>> +static void
>>> +dot_scfi_dummy (int ignored ATTRIBUTE_UNUSED)
>>> +{
>>> +  as_bad (_("SCFI is not supported for this target"));
>>> +  ignore_rest_of_line ();
>>> +}
>>> +
>>> +const pseudo_typeS scfi_pseudo_table[] =
>>> +  {
>>> +    { "cfi_sections", dot_scfi_dummy, 0 },
>>> +    { "cfi_startproc", dot_scfi_dummy, 0 },
>>> +    { "cfi_endproc", dot_scfi_dummy, 0 },
>>> +    { "cfi_fde_data", dot_scfi_dummy, 0 },
>>> +    { "cfi_def_cfa", dot_scfi_dummy, 0 },
>>> +    { "cfi_def_cfa_register", dot_scfi_dummy, 0 },
>>> +    { "cfi_def_cfa_offset", dot_scfi_dummy, 0 },
>>> +    { "cfi_adjust_cfa_offset", dot_scfi_dummy, 0 },
>>> +    { "cfi_offset", dot_scfi_dummy, 0 },
>>> +    { "cfi_rel_offset", dot_scfi_dummy, 0 },
>>> +    { "cfi_register", dot_scfi_dummy, 0 },
>>> +    { "cfi_return_column", dot_scfi_dummy, 0 },
>>> +    { "cfi_restore", dot_scfi_dummy, 0 },
>>> +    { "cfi_undefined", dot_scfi_dummy, 0 },
>>> +    { "cfi_same_value", dot_scfi_dummy, 0 },
>>> +    { "cfi_remember_state", dot_scfi_dummy, 0 },
>>> +    { "cfi_restore_state", dot_scfi_dummy, 0 },
>>> +    { "cfi_window_save", dot_scfi_dummy, 0 },
>>> +    { "cfi_negate_ra_state", dot_scfi_dummy, 0 },
>>> +    { "cfi_escape", dot_scfi_dummy, 0 },
>>> +    { "cfi_signal_frame", dot_scfi_dummy, 0 },
>>> +    { "cfi_personality", dot_scfi_dummy, 0 },
>>> +    { "cfi_personality_id", dot_scfi_dummy, 0 },
>>> +    { "cfi_lsda", dot_scfi_dummy, 0 },
>>> +    { "cfi_val_encoded_addr", dot_scfi_dummy, 0 },
>>> +    { "cfi_inline_lsda", dot_scfi_dummy, 0 },
>>> +    { "cfi_label", dot_scfi_dummy, 0 },
>>> +    { "cfi_val_offset", dot_scfi_dummy, 0 },
>>> +    { NULL, NULL, 0 }
>>> +  };
>>> +
>>> +#endif
>>
>> Is this really needed? Can't you simply error on use of the command
>> line option, without the need for the extra table and dummy handler?
>>
> 
> Actually, gas does not even present the option --scfi when the target 
> does not have the two required defines TARGET_USE_SCFI  and 
> TARGET_USE_GINSN.  So I could guard the code in pobegin () in read.c 
> (and remove the dummy handlers in scfidw2genc.c):
> 
>    /* Now CFI ones.  */
> #if defined (TARGET_USE_SCFI) && defined (TARGET_USE_GINSN)
>    if (flag_synth_cfi)
>      {
>        pop_table_name = "scfi";
>        scfi_pop_insert ();
>      }
>    else
>      {
>        pop_table_name = "cfi";
>        cfi_pop_insert ();
>      }
> #else
>    pop_table_name = "cfi";
>    cfi_pop_insert ();
> #endif

Except imo preferably without redundancy:

   /* Now CFI ones.  */
#if defined (TARGET_USE_SCFI) && defined (TARGET_USE_GINSN)
   if (flag_synth_cfi)
     {
       pop_table_name = "scfi";
       scfi_pop_insert ();
     }
   else
#endif
     {
       pop_table_name = "cfi";
       cfi_pop_insert ();
     }

Jan
  

On 11/2/23 03:35, Jan Beulich wrote:
> On 31.10.2023 23:06, Indu Bhagat wrote:
>> On 10/31/23 04:28, Jan Beulich wrote:
>>> On 30.10.2023 17:51, Indu Bhagat wrote:
>>>> +void
>>>> +scfi_dot_cfi_startproc (symbolS *start_sym)
>>>
>>> This and the following two functions presently have no caller, and I
>>> also can't spot equivalents in dw2gencfi.c. How are they (going) to be
>>> used? This ...
>>>
>>>> +{
>>>> +  if (frchain_now->frch_cfi_data != NULL)
>>>> +    {
>>>> +      as_bad (_("previous CFI entry not closed (missing .cfi_endproc)"));
>>>
>>> ... for example suggests that the function here might really be the
>>> handler for .cfi_startproc, yet the table above says .cfi_startproc is
>>> ignored.
>>>
>>
>> The callers of scfi_dot_cfi_startproc (), scfi_dot_cfi_endproc () and
>> scfi_dot_cfi () are in scfi.c, when its time to emit DWARF CFI after
>> SCFI machinery has generated the SCFI Ops (See scfi_dot_cfi () and
>> scfi_emit_dw2cfi ()).  The callers are added in the next patch in the
>> series, "[PATCH, V2 07/10] gas: synthesize CFI for hand-written asm".
> 
> But that's not in the context of processing a .cfi_* directive. IOW the
> message is properly misleading.
> 

Makes sense. This will usually arise when something goes amiss with the 
SCFI machinery.

I will reword the message.

Indu