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

[Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>]

Hi Simon,

> 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).

sparc{v9,64}-sun-solaris2.11

> 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

From a quick check, this should just work today: the functions declared
in sparc*-sol2-tdep.c are only called in Solaris-specific files already.
I'll give it a try separately.  sparc{32,64}_sol2_init_abi are currently
declared in common files (sparc*-tdep.h), but they can just be made
static and the declarations removed.

> (don't have time right now), but tt would be easier to review if you could go
> incrementally, one function at a time.

I'd thought about that.  However, given that many of the features moved
to common code are rather obscure, I formally don't need approval for
Solaris-specific changes, and a make -j24 check takes between 30 and 60
minutes each time (there are some tests that regularly time out, letting
the test time grow out of bounds), I decided to keep it in one patch.

I'm primarily looking for answers to the meta-questions below here.

>> 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.

I guess I'll go for it, time permitting: while it doesn't create a
burden, it's just dead code these days.

>> * 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.

Probably someone just has to take the lead, both in GDB and GCC.
Especially in the latter, it's going to take a considerable amount of
work.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University