Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Maciej W. Rozycki" <macro@codesourcery.com>
To: <gdb-patches@sourceware.org>
Cc: Richard Sandiford <rdsandiford@googlemail.com>,
	Catherine Moore	<clm@codesourcery.com>, <binutils@sourceware.org>
Subject: [PATCH] in_plt_section: support alternate stub section names (was: [PATCH 1/2] MIPS: Compressed PLT/stubs support)
Date: Fri, 07 Jun 2013 13:25:00 -0000	[thread overview]
Message-ID: <alpine.DEB.1.10.1306071405170.16287@tp.orcam.me.uk> (raw)
In-Reply-To: <alpine.DEB.1.10.1302072108300.6762@tp.orcam.me.uk>

Hi,

 I have realised the change to support alternate stub section names in 
in_plt_section is really self-contained and while a prerequisite for 
microMIPS/MIPS16 PLT support it can be applied separately.  I have 
therefore split it off from the PLT change, hopefully making a review 
easier.

 For a reference, here are the relevant original observations I made when 
posting the combined change:

>  As to the semantics change of the in_plt_section GDB helper -- the `name' 
> argument is unused and all the callers pass it as NULL.  I've tracked down 
> the history of this function, and it was introduced with GDB 4.13:
> 
> Fri Apr  1 00:44:00 1994  Peter Schauer  (pes@regent.e-technik.tu-muenchen.de)
> 
> 	* sparc-tdep.c (in_solib_trampoline):  Renamed to in_plt_section
> 	and moved to objfiles.c.
> 	* objfiles.c (in_plt_section):  Moved to here from sparc-tdep.
> 	* config/tm-sysv4.h (IN_SOLIB_TRAMPOLINE):  Use new in_plt_section.
> 	* config/sparc/tm-sun4sol2.h (IN_SOLIB_TRAMPOLINE):  Removed,
> 	the new generic definition from tm-sysv4.h works for Solaris.
> 
> -- with this argument already unused.  Furthermore, `in_solib_trampoline' 
> was introduced in GDB 4.9:
> 
> Tue Mar 30 15:46:14 1993  K. Richard Pixley  (rich@rtl.cygnus.com)
> 
> 	* sparc-tdep.c (in_solib_trampoline): new function.
> 	* config/sparc/tm-sun4sol2.h (IN_SOLIB_TRAMPOLINE): redefine to
> 	  in_solib_trampoline.
> 
> with this argument also unused.  I was unable to track down the pre-4.9
> tm-sun4sol2.h version of IN_SOLIB_TRAMPOLINE as GDB 4.8 didn't have the 
> macro there yet, so no GDB version was ever released that provided it.  
> 
>  However, the tm-sysv4.h version was defined like this:
> 
> #define IN_SOLIB_TRAMPOLINE(pc,name) ((name) && (STREQ ("_init", name)))
> 
> -- and then redefined in terms of in_plt_section as recorded in the 
> ChangeLog entry quoted above like this:
> 
> #define IN_SOLIB_TRAMPOLINE(pc, name) in_plt_section((pc), (name))
> 
> at which point the `name' argument became unused as well.
> 
>  HP-PA had its own version:
> 
> #define IN_SOLIB_TRAMPOLINE(pc, name) skip_trampoline_code (pc, name)
> 
> -- but skip_trampoline_code didn't make any use of its `name' argument 
> either -- just as does't current code in hppa_in_solib_call_trampoline the 
> former has evolved to (and neither does code in 
> hppa32_hpux_in_solib_call_trampoline, hppa64_hpux_in_solib_call_trampoline 
> or hppa_hpux_in_solib_return_trampoline).
> 
>  With the above consideration in mind, I think it is safe to redefine 
> in_plt_section's API as proposed in this change -- remembering that MIPS 
> stubs are the functional equivalent of PLT entries -- for the sake of code 
> duplication avoidance.

 With in_plt_section such redefined, all the handcoded conditions 
throughout the MIPS backend can be unified, and also the helper can now be 
used in mips_linux_in_dynsym_stub to avoid the heuristic there if 
possible.

 This change was regression-tested for the mips-sde-elf and mips-linux-gnu 
targets using the following configurations (multilibs), both endiannesses 
each:

* o32 (-mabi=32),

* n64 (-mabi=64) (mips-linux-gnu only),

* n32 (-mabi=n32) (mips-linux-gnu only),

* MIPS16 o32 (-mips16 -mabi=32),

* microMIPS o32 (-mmicromips -mabi=32).

with no regressions (as previously, with the outstanding ISA bit fix 
applied).  OK to apply?

2013-06-07  Maciej W. Rozycki  <macro@codesourcery.com>

	gdb/
	* mips-linux-tdep.c (mips_linux_in_dynsym_stub): Handle
	.MIPS.stubs section like .plt.  Remove unused `name' argument.
	Return 1 rather than the low 16-bit halfword of any instruction
	examined.
	(mips_linux_in_dynsym_resolve_code): Update accordingly.
        * mips-tdep.c (mips_stub_frame_sniffer): Call in_plt_section in 
	place of an equivalent hand-coded sequence.
	* objfiles.c (in_plt_section): Reuse the `name' argument as an
	trampoline section name override.
  Maciej

gdb-mips-in-stubs-section.diff
Index: gdb-fsf-trunk-quilt/gdb/mips-linux-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-linux-tdep.c	2013-06-06 20:48:30.243223201 +0100
+++ gdb-fsf-trunk-quilt/gdb/mips-linux-tdep.c	2013-06-06 20:52:00.273227140 +0100
@@ -30,6 +30,7 @@
 #include "trad-frame.h"
 #include "tramp-frame.h"
 #include "gdbtypes.h"
+#include "objfiles.h"
 #include "solib.h"
 #include "solib-svr4.h"
 #include "solist.h"
@@ -666,25 +667,34 @@ mips_linux_core_read_description (struct
 
 
 /* Check the code at PC for a dynamic linker lazy resolution stub.
-   Because they aren't in the .plt section, we pattern-match on the
-   code generated by GNU ld.  They look like this:
+   GNU ld for MIPS has put lazy resolution stubs into a ".MIPS.stubs"
+   section uniformly since version 2.15.  If the pc is in that section,
+   then we are in such a stub.  Before that ".stub" was used in 32-bit
+   ELF binaries, however we do not bother checking for that since we
+   have never had and that case should be extremely rare these days.
+   Instead we pattern-match on the code generated by GNU ld.  They look
+   like this:
 
    lw t9,0x8010(gp)
    addu t7,ra
    jalr t9,ra
    addiu t8,zero,INDEX
 
-   (with the appropriate doubleword instructions for N64).  Also
-   return the dynamic symbol index used in the last instruction.  */
+   (with the appropriate doubleword instructions for N64).  As any lazy
+   resolution stubs in microMIPS binaries will always be in a
+   ".MIPS.stubs" section we only ever verify standard MIPS patterns. */
 
 static int
-mips_linux_in_dynsym_stub (CORE_ADDR pc, char *name)
+mips_linux_in_dynsym_stub (CORE_ADDR pc)
 {
   gdb_byte buf[28], *p;
   ULONGEST insn, insn1;
   int n64 = (mips_abi (target_gdbarch ()) == MIPS_ABI_N64);
   enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch ());
 
+  if (in_plt_section (pc, ".MIPS.stubs"))
+    return 1;
+
   read_memory (pc - 12, buf, 28);
 
   if (n64)
@@ -742,7 +752,7 @@ mips_linux_in_dynsym_stub (CORE_ADDR pc,
 	return 0;
     }
 
-  return (insn & 0xffff);
+  return 1;
 }
 
 /* Return non-zero iff PC belongs to the dynamic linker resolution
@@ -756,9 +766,10 @@ mips_linux_in_dynsym_resolve_code (CORE_
   if (svr4_in_dynsym_resolve_code (pc))
     return 1;
 
-  /* Pattern match for the stub.  It would be nice if there were a
-     more efficient way to avoid this check.  */
-  if (mips_linux_in_dynsym_stub (pc, NULL))
+  /* Likewise for the stubs.  They live in the .MIPS.stubs section these
+     days, so we check if the PC is within, than fall back to a pattern
+     match.  */
+  if (mips_linux_in_dynsym_stub (pc))
     return 1;
 
   return 0;
Index: gdb-fsf-trunk-quilt/gdb/mips-tdep.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/mips-tdep.c	2013-06-06 20:48:30.243223201 +0100
+++ gdb-fsf-trunk-quilt/gdb/mips-tdep.c	2013-06-06 20:52:00.273227140 +0100
@@ -3591,12 +3591,7 @@ mips_stub_frame_sniffer (const struct fr
   if (in_plt_section (pc, NULL))
     return 1;
 
-  /* Binutils for MIPS puts lazy resolution stubs into .MIPS.stubs.  */
-  s = find_pc_section (pc);
-
-  if (s != NULL
-      && strcmp (bfd_get_section_name (s->objfile->obfd, s->the_bfd_section),
-		 ".MIPS.stubs") == 0)
+  if (in_plt_section (pc, ".MIPS.stubs"))
     return 1;
 
   /* Calling a PIC function from a non-PIC function passes through a
Index: gdb-fsf-trunk-quilt/gdb/objfiles.c
===================================================================
--- gdb-fsf-trunk-quilt.orig/gdb/objfiles.c	2013-06-06 20:48:30.243223201 +0100
+++ gdb-fsf-trunk-quilt/gdb/objfiles.c	2013-06-06 20:52:00.273227140 +0100
@@ -1410,9 +1410,11 @@ find_pc_section (CORE_ADDR pc)
 }
 
 
-/* In SVR4, we recognize a trampoline by it's section name. 
-   That is, if the pc is in a section named ".plt" then we are in
-   a trampoline.  */
+/* In SVR4, we recognize a trampoline by it's section name.  That is,
+   if the pc is in a section named ".plt" then we are in a trampoline.
+   We let targets request an alternative name, this is currently used
+   by the MIPS backend to handle the SVR4 lazy resolution stubs that
+   binutils put into ".MIPS.stubs" instead.  */
 
 int
 in_plt_section (CORE_ADDR pc, char *name)
@@ -1424,7 +1426,7 @@ in_plt_section (CORE_ADDR pc, char *name
 
   retval = (s != NULL
 	    && s->the_bfd_section->name != NULL
-	    && strcmp (s->the_bfd_section->name, ".plt") == 0);
+	    && strcmp (s->the_bfd_section->name, name ? name : ".plt") == 0);
   return (retval);
 }
 \f


  parent reply	other threads:[~2013-06-07 13:25 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-19 20:44 [PATCH 1/2] MIPS: Compressed PLT/stubs support Maciej W. Rozycki
2013-02-19 20:45 ` [PATCH 2/2] MIPS: Compressed PLT/stubs support test cases Maciej W. Rozycki
2013-02-20 21:53 ` [PATCH 1/2] MIPS: Compressed PLT/stubs support Richard Sandiford
2013-03-09  4:04   ` Maciej W. Rozycki
2013-03-09  9:58     ` Richard Sandiford
2013-06-08  0:22       ` Maciej W. Rozycki
2013-06-08 16:04         ` Richard Sandiford
2013-06-10 17:13           ` Maciej W. Rozycki
2013-06-10 18:08             ` Richard Sandiford
2013-06-10 19:34               ` Maciej W. Rozycki
2013-06-25  0:40                 ` Maciej W. Rozycki
2013-03-11 13:53     ` Joel Brobecker
2013-06-26 15:02       ` Maciej W. Rozycki
2013-02-21 21:06 ` Tom Tromey
2013-02-22  0:58   ` Alan Modra
2013-02-22  6:06     ` Alan Modra
2013-02-22 20:09       ` Tom Tromey
2013-03-09  4:06         ` Maciej W. Rozycki
2013-06-20 16:20   ` [PING^2][PATCH] in_plt_section: support alternate stub section names (was: [PATCH 1/2] MIPS: Compressed PLT/stubs support) Maciej W. Rozycki
2013-06-20 16:50     ` [PING^2][PATCH] in_plt_section: support alternate stub section names Pedro Alves
2013-06-21 11:43       ` Maciej W. Rozycki
2013-06-21 15:34         ` Pedro Alves
2013-06-22  2:24           ` Maciej W. Rozycki
2013-06-24 12:40             ` Pedro Alves
2013-06-24 23:34               ` Maciej W. Rozycki
2013-06-25  9:57                 ` Pedro Alves
2013-06-07 13:25 ` Maciej W. Rozycki [this message]
2013-06-13 12:43   ` [PING][PATCH] in_plt_section: support alternate stub section names (was: [PATCH 1/2] MIPS: Compressed PLT/stubs support) Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.10.1306071405170.16287@tp.orcam.me.uk \
    --to=macro@codesourcery.com \
    --cc=binutils@sourceware.org \
    --cc=clm@codesourcery.com \
    --cc=gdb-patches@sourceware.org \
    --cc=rdsandiford@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox