From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 8360E398603A for ; Wed, 24 Jun 2020 14:56:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8360E398603A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id CF7091E111; Wed, 24 Jun 2020 10:56:45 -0400 (EDT) Subject: Re: [RFC][PATCH] Move common handlers to sol2_init_abi To: Rainer Orth , gdb-patches@sourceware.org References: From: Simon Marchi Message-ID: <00d11a67-fb37-52de-859d-f8055c841668@simark.ca> Date: Wed, 24 Jun 2020 10:56:45 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.3 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_SHORT, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jun 2020 14:56:48 -0000 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