Re: [RFC][PATCH] Move common handlers to sol2_init_abi

[Simon Marchi <simark@simark.ca>]

On 2020-06-23 9:15 a.m., Rainer Orth wrote:
> There's some overlap and duplication between 32 and 64-bit Solaris/SPARC
> and x86 tdep files, in particular
> 
>         sol2_core_pid_to_str
> 	*_sol2_sigtramp_p
>         sol2_skip_solib_resolver
>         *_sol2_static_transform_name (forgotten on amd64)
>         set_gdbarch_sofun_address_maybe_missing (likewise)
> 
> This patch avoids this by centralizing common code in sol2-tdep.c.
> While sparc_sol2_pc_in_sigtramp and sparc_sol2_static_transform_name
> were declared in the shared sparc-tdep.h, they were only used in Solaris
> files.
> 
> However, I just discovered that there are two targets that would break
> with this patch: both sparc-*-linux* and sparc64-*-linux* include
> sparc-sol2-tdep.o and sparc64-sol2-tdep.o in configure.tgt.  With the
> new call to sol2_init_abi which only lives in sol2-tdep.o, gdb would
> fail to link.  I have no idea what business they have with
> Solaris-specific files: I suspect that's to allow debugging of
> Solaris/SPARC binaries (i.e. GDB_OSABI_SOLARIS).  What should I do about
> this?  Maybe I also could include sol2-tdep.o on Linux/SPARC, but is
> this TRT?  AFAICS those files received only mechanical changes over the
> last two years (haven't looked further), and I have no way of testing
> changes.

Hmm, sparc-*-linux* and sparc64-*-linux* have no business including some
Solaris-specific files in the build.

When building a GDB on sparc64/Linux, it shouldn't have support for debugging
sparc64/Solaris binaries.  If you want that, you need to pass
--enable-targets=sparc64-solaris-something (I don't know what the actual triplet
would be).

So it seems like there is some untangling here, putting the functions on the
files that they really belong to, until you can successfully build a sparc64/Linux
GDB without including the sol2 tdep files.  I haven't looked much at the patch
(don't have time right now), but tt would be easier to review if you could go
incrementally, one function at a time.

> 
> Tested on amd64-pc-solaris2.11, i386-pc-solaris2.11,
> sparcv9-sun-solaris2.11, and sparc-sun-solaris2.11.
> 
> While those patches only/mostly affect Solaris-specific code, I have
> some questions:
> 
> * Two settings above (static_transform_name,
>   sofun_address_maybe_missing) only apply to quirks of stabs.
> 
>   The first is completely Solaris-specific (or rather specific to the
>   Studio compilers) and I didn't do any real testing here.  Studio cc
>   has deprecated stabs support in the 12.4 release back in 2015, gcc has
>   defaulted to DWARF-2 on Solaris 7+ since 2004, so maybe the whole
>   static_transform_name code can go?

I would be completely fine with it.  The Solaris port pretty much depends on
the time you have to give, so if you don't have time to deal with ancient stuff,
it's fine to drop support for it.

> 
>   The second is also called in i386-linux-tdep.c and rs6000-tdep.c and I
>   don't know enough about this to say what to do here.
> 
> * Beyond that, maybe it's time to think about deprecating Stabs support
>   in general.  There has been some discussion on the GCC side
> 
>   https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01297.html
> 
>   but nothing happened yet.

I would also be fine with it.  For years, all we have done to the stabs code
(and other old debug info reader) is adjust it to interface changes of the
core code.  And actually, since that goes pretty much untested, it is likely
that an old GDB is less buggy than today's GDB w.r.t. stabs debug info reading.

Simon