Don't include *sol2-tdep.o on Linux/sparc*

Message ID ydda70ro8pe.fsf_-_@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • Don't include *sol2-tdep.o on Linux/sparc*
Related show

Commit Message

Rainer Orth June 25, 2020, 8:23 a.m.
Hi Simon,

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


the following patch does just that: tested on sparc64-unknown-linux-gnu
(build only due to PR tdep/26170) and sparcv9-sun-solaris2.11.

Ok for master?

	Rainer

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


2020-06-24  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* configure.tgt <sparc-*-linux*> (gdb_target_obs): Remove
	sparc-sol2-tdep.o, sol2-tdep.o, sparc64-sol2-tdep.o.
	<sparc64-*-linux*> (gdb_target_obs): Remove sparc64-sol2-tdep.o,
	sol2-tdep.o, sparc-sol2-tdep.o.
	* sparc-sol2-tdep.c (sparc32_sol2_init_abi): Make static.
	* sparc-tdep.h (sparc32_sol2_init_abi): Remove.
	* sparc64-sol2-tdep.c (sparc64_sol2_init_abi): Make static.
	* sparc64-tdep.h (sparc64_sol2_init_abi): Remove.

Comments

Shahab Vahedi via Gdb-patches June 25, 2020, 9:57 a.m. | #1
On 6/25/20 9:23 AM, Rainer Orth wrote:
>> 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.

> the following patch does just that: tested on sparc64-unknown-linux-gnu

> (build only due to PR tdep/26170) and sparcv9-sun-solaris2.11.


I got curious and looked into git history to find why that was done
in the first place.

'git blame configure.tgt' points at a4ce5b0d0204 as the commit that
converted the old .mt files into configure.tgt.

And then:

 $ git blame a4ce5b0d0204^ -- ./config/sparc/linux64.mt

points at:

 commit 691342f948aa4f8c4f4e039669276faf753a36b4
 Author:     David S. Miller <davem@redhat.com>
 AuthorDate: Mon Feb 27 06:14:51 2006 +0000

    2006-02-26  David S. Miller  <davem@sunset.davemloft.net>
    
            * config/sparc/linux.mt (TDEPFILES): Add sol2-tdep.o.
            * config/sparc/linux64.mt (TDEPFILES): Likewise.

Which is... drumroll... a big nothingburger:

 https://sourceware.org/legacy-ml/gdb-patches/2006-02/msg00492.html

So, nothing to see here, move along.  :-)

Thanks,
Pedro Alves
Rainer Orth June 25, 2020, 12:03 p.m. | #2
Hi Pedro,

> On 6/25/20 9:23 AM, Rainer Orth wrote:

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

>> the following patch does just that: tested on sparc64-unknown-linux-gnu

>> (build only due to PR tdep/26170) and sparcv9-sun-solaris2.11.

>

> I got curious and looked into git history to find why that was done

> in the first place.

>

> 'git blame configure.tgt' points at a4ce5b0d0204 as the commit that

> converted the old .mt files into configure.tgt.

>

> And then:

>

>  $ git blame a4ce5b0d0204^ -- ./config/sparc/linux64.mt

>

> points at:

>

>  commit 691342f948aa4f8c4f4e039669276faf753a36b4

>  Author:     David S. Miller <davem@redhat.com>

>  AuthorDate: Mon Feb 27 06:14:51 2006 +0000

>

>     2006-02-26  David S. Miller  <davem@sunset.davemloft.net>

>     

>             * config/sparc/linux.mt (TDEPFILES): Add sol2-tdep.o.

>             * config/sparc/linux64.mt (TDEPFILES): Likewise.

>

> Which is... drumroll... a big nothingburger:

>

>  https://sourceware.org/legacy-ml/gdb-patches/2006-02/msg00492.html

>

> So, nothing to see here, move along.  :-)


however, this only accounts for the presence of sol2-tdep.o, not the
rest (sparc*-sol2-tdep.o).  I looked further and they were present from
the beginning when sparc/linux64.mt was introduced.  There may have been
a reason to include them once but it's certainly gone today.

	Rainer

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

Patch

# HG changeset patch
# Parent  3a3a48c067f99b6fc95d7b297570b179fd575e9c
Don't include *sol2-tdep.o on Linux/sparc*

diff --git a/gdb/configure.tgt b/gdb/configure.tgt
--- a/gdb/configure.tgt
+++ b/gdb/configure.tgt
@@ -585,20 +585,20 @@  sh*)
 
 sparc-*-linux*)
 	# Target: GNU/Linux SPARC
-	gdb_target_obs="sparc-tdep.o sparc-sol2-tdep.o sol2-tdep.o \
+	gdb_target_obs="sparc-tdep.o \
 			sparc-linux-tdep.o solib-svr4.o symfile-mem.o \
 			linux-tdep.o \
 			ravenscar-thread.o sparc-ravenscar-thread.o"
 	if test "x$enable_64_bit_bfd" = "xyes"; then
 	    # Target: GNU/Linux UltraSPARC
-	    gdb_target_obs="sparc64-tdep.o sparc64-sol2-tdep.o \
+	    gdb_target_obs="sparc64-tdep.o \
 			    sparc64-linux-tdep.o ${gdb_target_obs}"
 	fi
 	;;
 sparc64-*-linux*)
 	# Target: GNU/Linux UltraSPARC
-	gdb_target_obs="sparc64-tdep.o sparc64-sol2-tdep.o sol2-tdep.o \
-			sparc64-linux-tdep.o sparc-tdep.o sparc-sol2-tdep.o \
+	gdb_target_obs="sparc64-tdep.o \
+			sparc64-linux-tdep.o sparc-tdep.o \
 			sparc-linux-tdep.o solib-svr4.o linux-tdep.o \
 			ravenscar-thread.o sparc-ravenscar-thread.o"
 	;;
diff --git a/gdb/sparc-sol2-tdep.c b/gdb/sparc-sol2-tdep.c
--- a/gdb/sparc-sol2-tdep.c
+++ b/gdb/sparc-sol2-tdep.c
@@ -192,7 +192,7 @@  static const struct frame_unwind sparc32
 
 
 
-void
+static void
 sparc32_sol2_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/sparc-tdep.h b/gdb/sparc-tdep.h
--- a/gdb/sparc-tdep.h
+++ b/gdb/sparc-tdep.h
@@ -245,9 +245,6 @@  extern int sparc_is_annulled_branch_insn
 extern const struct sparc_gregmap sparc32_sol2_gregmap;
 extern const struct sparc_fpregmap sparc32_sol2_fpregmap;
 
-extern void sparc32_sol2_init_abi (struct gdbarch_info info,
-				   struct gdbarch *gdbarch);
-
 /* Functions and variables exported from sparcnbsd-tdep.c.  */
 
 /* Register offsets for NetBSD.  */
diff --git a/gdb/sparc64-sol2-tdep.c b/gdb/sparc64-sol2-tdep.c
--- a/gdb/sparc64-sol2-tdep.c
+++ b/gdb/sparc64-sol2-tdep.c
@@ -195,7 +195,7 @@  static const struct frame_unwind sparc64
 
 
 
-void
+static void
 sparc64_sol2_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
diff --git a/gdb/sparc64-tdep.h b/gdb/sparc64-tdep.h
--- a/gdb/sparc64-tdep.h
+++ b/gdb/sparc64-tdep.h
@@ -119,9 +119,6 @@  extern void sparc64_collect_fpregset (co
 extern const struct sparc_gregmap sparc64_sol2_gregmap;
 extern const struct sparc_fpregmap sparc64_sol2_fpregmap;
 
-extern void sparc64_sol2_init_abi (struct gdbarch_info info,
-				   struct gdbarch *gdbarch);
-
 /* Variables exported from sparc64-fbsd-tdep.c.  */
 
 /* Register offsets for FreeBSD/sparc64.  */