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

[Jan Beulich]

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