Re: [GCC][PATCH 13/15, v5] arm: Add support for dwarf debug directives and pseudo hard-register for PAC feature.

[Richard Earnshaw <Richard.Earnshaw@foss.arm.com>]

On 13/01/2023 22:25, Richard Earnshaw (lists) via Gcc-patches wrote:
> On 13/01/2023 22:12, Jakub Jelinek wrote:
>> On Fri, Jan 13, 2023 at 09:58:26PM +0000, Richard Earnshaw (lists) wrote:
>>> > I'm afraid increasing number of DWARF registers is ABI incompatible 
>>> change.
>>> > E.g. libgcc __frame_state_for function fills in:
>>> > typedef struct frame_state
>>> > {
>>> >    void *cfa;
>>> >    void *eh_ptr;
>>> >    long cfa_offset;
>>> >    long args_size;
>>> >    long reg_or_offset[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
>>> >    unsigned short cfa_reg;
>>> >    unsigned short retaddr_column;
>>> >    char saved[PRE_GCC3_DWARF_FRAME_REGISTERS+1];
>>> > } frame_state;
>>> > > structure, where PRE_GCC3_DWARF_FRAME_REGISTERS defaults to
>>> > __LIBGCC_DWARF_FRAME_REGISTERS__, which is defined to
>>> > DWARF_FRAME_REGISTERS, which defaults to FIRST_PSEUDO_REGISTER.
>>> > So, changing FIRST_PSEUDO_REGISTER is an ABI change unless you 
>>> arrange for
>>> > PRE_GCC3_DWARF_FRAME_REGISTERS to be defined to the old value.
>>> > >      Jakub
>>> >
>>> So where's the red flag that warns about this?
>>>
>>> I also note that Richard Sandiford made a similar type of change for 
>>> AArch64
>>> in r10-4195 (183bfdafc6f1f98711c5400498a7268cc1441096) and nothing 
>>> was said
>>> about that at the time.
>>>
>>> It seems incredibly fragile to me to have some ABI based off the 
>>> number of
>>> machine registers.
>>
>> It is.  The new unwinder fortunately doesn't suffer from this (at least I
>> think it doesn't), but in older gccs the unwinder could be split 
>> across different
>> objects, having e.g. parts of the unwinder in one shared library and 
>> another
>> part in another one, each built by different GCC version.
>>
>> Guess targets which weren't supported in GCC 2.x are ok, while
>> __frame_state_for is in libgcc, nothing calls it, so while such changes
>> change the ABI, nothing likely cares.
>> But for older targets it is a problem.
>>
>> And it is hard to catch this in the testsuite, one would either need to
>> hardcode the count for each target in the test, or test with mixing 
>> GCC 2.x
>> compiled code with current trunk.
>>
>> Before the introduction of libgcc_eh.a etc., parts of the unwinder was 
>> e.g.
>> exported from glibc.
>> See e.g. 
>> https://gcc.gnu.org/legacy-ml/gcc-patches/2001-07/threads.html#00472 
>> <https://gcc.gnu.org/legacy-ml/gcc-patches/2001-07/threads.html#00472>
>> for some details.
>>
>>          Jakub
>>
> 
> So:
> 1) GCC-2.* didn't support the EABI, which is all we support these days.
> 2) the Arm port updated FIRST_PSEUDO_REGISTER in 2019 in r10-4441 
> (16155ccf588a403c033ccd7743329671bcfb27d5) and I didn't see any fallout 
> from that.
In fact it's been changed in

  16155ccf588a
  cf16f980e527
  0be8bd1a1c89
  f1adb0a9f4d7
  9b66ebb1460d
  5a9335ef017c

All since 2003 (ie since gcc-3.0 was released).

> 3) The Arm port uses the unwinding mechanism defined by the ABI, not the 
> dwarf2 based tables.
> 
> So I'm inclined to think this probably isn't going to be a problem in 
> reality.
> 
> R.