[PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

John Baldwin
The ELF runtime linker on all FreeBSD architectures uses the
"_rtld_bind" entry point for unresolved PTL entries.  FreeBSD/mips has
an additional entry point called "_mips_rtld_bind".

gdb/ChangeLog:

        * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
        (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
        * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
        * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
        (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
        method.
---
 gdb/ChangeLog        |  9 +++++++++
 gdb/fbsd-tdep.c      | 15 +++++++++++++++
 gdb/fbsd-tdep.h      |  3 +++
 gdb/mips-fbsd-tdep.c | 16 ++++++++++++++++
 4 files changed, 43 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c292927e49..219c2b110b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-07-10  John Baldwin  <[hidden email]>
+
+ * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
+ (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
+ * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
+ * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
+ (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
+ method.
+
 2020-07-10  John Baldwin  <[hidden email]>
 
  * fbsd-nat.h (fbsd_nat_target::supports_multi_process): New
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 557c5d3d73..02d1b1fd40 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -21,6 +21,7 @@
 #include "auxv.h"
 #include "gdbcore.h"
 #include "inferior.h"
+#include "objfiles.h"
 #include "regcache.h"
 #include "regset.h"
 #include "gdbthread.h"
@@ -2071,6 +2072,19 @@ fbsd_get_thread_local_address (struct gdbarch *gdbarch, CORE_ADDR dtv_addr,
   return addr + offset;
 }
 
+/* Implement the "skip_solib_resolver" gdbarch method.  */
+
+CORE_ADDR
+fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  struct bound_minimal_symbol msym;
+
+  msym = lookup_minimal_symbol("_rtld_bind", NULL, NULL);
+  if (msym.minsym && BMSYMBOL_VALUE_ADDRESS (msym) == pc)
+    return frame_unwind_caller_pc (get_current_frame ());
+  return 0;
+}
+
 /* To be called from GDB_OSABI_FREEBSD handlers. */
 
 void
@@ -2085,6 +2099,7 @@ fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_get_siginfo_type (gdbarch, fbsd_get_siginfo_type);
   set_gdbarch_gdb_signal_from_target (gdbarch, fbsd_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch, fbsd_gdb_signal_to_target);
+  set_gdbarch_skip_solib_resolver (gdbarch, fbsd_skip_solib_resolver);
 
   /* `catch syscall' */
   set_xml_syscall_file_name (gdbarch, "syscalls/freebsd.xml");
diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
index 5dd8203299..2e3f46186c 100644
--- a/gdb/fbsd-tdep.h
+++ b/gdb/fbsd-tdep.h
@@ -71,4 +71,7 @@ extern CORE_ADDR fbsd_get_thread_local_address (struct gdbarch *gdbarch,
  CORE_ADDR lm_addr,
  CORE_ADDR offset);
 
+extern CORE_ADDR fbsd_skip_solib_resolver (struct gdbarch *gdbarch,
+   CORE_ADDR pc);
+
 #endif /* fbsd-tdep.h */
diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
index abaf7f2474..90ef466137 100644
--- a/gdb/mips-fbsd-tdep.c
+++ b/gdb/mips-fbsd-tdep.c
@@ -462,6 +462,20 @@ static const struct tramp_frame mips64_fbsd_sigframe =
 
 /* Shared library support.  */
 
+/* FreeBSD/mips can use an alternate routine in the runtime linker to
+   resolve functions.  */
+
+CORE_ADDR
+mips_fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  struct bound_minimal_symbol msym;
+
+  msym = lookup_minimal_symbol("_mips_rtld_bind", NULL, NULL);
+  if (msym.minsym && BMSYMBOL_VALUE_ADDRESS (msym) == pc)
+    return frame_unwind_caller_pc (get_current_frame ());
+  return fbsd_skip_solib_resolver (gdbarch, pc);
+}
+
 /* FreeBSD/mips uses a slightly different `struct link_map' than the
    other FreeBSD platforms as it includes an additional `l_off'
    member.  */
@@ -546,6 +560,8 @@ mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, mips_fbsd_iterate_over_regset_sections);
 
+  set_gdbarch_skip_solib_resolver (gdbarch, mips_fbsd_skip_solib_resolver);
+
   /* FreeBSD/mips has SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
     (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
--
2.25.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

Simon Marchi-4
On 2020-07-11 12:14 p.m., John Baldwin wrote:

> The ELF runtime linker on all FreeBSD architectures uses the
> "_rtld_bind" entry point for unresolved PTL entries.  FreeBSD/mips has
> an additional entry point called "_mips_rtld_bind".
>
> gdb/ChangeLog:
>
> * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
> (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
> * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
> * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
> (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
> method.
> ---
>  gdb/ChangeLog        |  9 +++++++++
>  gdb/fbsd-tdep.c      | 15 +++++++++++++++
>  gdb/fbsd-tdep.h      |  3 +++
>  gdb/mips-fbsd-tdep.c | 16 ++++++++++++++++
>  4 files changed, 43 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index c292927e49..219c2b110b 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,12 @@
> +2020-07-10  John Baldwin  <[hidden email]>
> +
> + * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
> + (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
> + * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
> + * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
> + (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
> + method.
> +
>  2020-07-10  John Baldwin  <[hidden email]>
>  
>   * fbsd-nat.h (fbsd_nat_target::supports_multi_process): New
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 557c5d3d73..02d1b1fd40 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -21,6 +21,7 @@
>  #include "auxv.h"
>  #include "gdbcore.h"
>  #include "inferior.h"
> +#include "objfiles.h"
>  #include "regcache.h"
>  #include "regset.h"
>  #include "gdbthread.h"
> @@ -2071,6 +2072,19 @@ fbsd_get_thread_local_address (struct gdbarch *gdbarch, CORE_ADDR dtv_addr,
>    return addr + offset;
>  }
>  
> +/* Implement the "skip_solib_resolver" gdbarch method.  */

`/* See fbsd-tdep.h.  */` here, and move the actual comment to the .h.

> +
> +CORE_ADDR
> +fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  struct bound_minimal_symbol msym;
> +
> +  msym = lookup_minimal_symbol("_rtld_bind", NULL, NULL);

Space before parenthesis.  For new code, we prefer nullptr rather than NULL (unless
maybe if there are already some NULL in the existing surrounding code in the same
function).

Declare on first use:

  bound_minimal_symbol msym = ...

> +  if (msym.minsym && BMSYMBOL_VALUE_ADDRESS (msym) == pc)

msym.minsym != nullptr

Just curious, why do you have to validate that `BMSYMBOL_VALUE_ADDRESS (msym) == pc`,
in which case would it not be true?

> +    return frame_unwind_caller_pc (get_current_frame ());
> +  return 0;

Just a personal preference, but I think an empty line after an `if` helps readability.

> @@ -462,6 +462,20 @@ static const struct tramp_frame mips64_fbsd_sigframe =
>  
>  /* Shared library support.  */
>  
> +/* FreeBSD/mips can use an alternate routine in the runtime linker to
> +   resolve functions.  */
> +
> +CORE_ADDR
> +mips_fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
> +{
> +  struct bound_minimal_symbol msym;
> +
> +  msym = lookup_minimal_symbol("_mips_rtld_bind", NULL, NULL);
> +  if (msym.minsym && BMSYMBOL_VALUE_ADDRESS (msym) == pc)
> +    return frame_unwind_caller_pc (get_current_frame ());
> +  return fbsd_skip_solib_resolver (gdbarch, pc);

Same comments as above.

Simon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

Tom Tromey-2
In reply to this post by John Baldwin
>>>>> "John" == John Baldwin <[hidden email]> writes:

John> +CORE_ADDR
John> +mips_fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
John> +{

I think this should be "static".

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

Simon Marchi-4
On 2020-07-11 3:17 p.m., Tom Tromey wrote:

>>>>>> "John" == John Baldwin <[hidden email]> writes:
>
> John> +CORE_ADDR
> John> +mips_fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
> John> +{
>
> I think this should be "static".
>
> Tom
>

Oh right, and that should generate a warning, doesn't it?
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

Tom Tromey-2
>> I think this should be "static".

Simon> Oh right, and that should generate a warning, doesn't it?

Yeah.  I assume John's compiler is one that doesn't emit it.

Tom
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

John Baldwin
On 7/11/20 2:56 PM, Tom Tromey wrote:
>>> I think this should be "static".
>
> Simon> Oh right, and that should generate a warning, doesn't it?
>
> Yeah.  I assume John's compiler is one that doesn't emit it.

It does, I just missed it. :-/

--
John Baldwin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

John Baldwin
In reply to this post by Simon Marchi-4
On 7/11/20 11:54 AM, Simon Marchi wrote:
> On 2020-07-11 12:14 p.m., John Baldwin wrote:
> Just curious, why do you have to validate that `BMSYMBOL_VALUE_ADDRESS (msym) == pc`,
> in which case would it not be true?

I will fix all the style things.  Apart from the missing space before paren they
are all just copy-pasta from the sol2, nbsd, and obsd versions.  (The glibc version
is quite a bit different).

All of the other versions do the == pc check.  I'm not quite sure.  Perhaps it is
so that if you are intentionally single stepping through the PLT stub to debug it
(as I have sometimes had to do) you can still do that without it jumping to the end,
but that if you are doing stepping at the caller's frame it will skip over the
stub.

--
John Baldwin
Reply | Threaded
Open this post in threaded view
|

[PATCH v2] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

John Baldwin
The ELF runtime linker on all FreeBSD architectures uses the
"_rtld_bind" entry point for unresolved PTL entries.  FreeBSD/mips has
an additional entry point called "_mips_rtld_bind".

gdb/ChangeLog:

        * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
        (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
        * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
        * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
        (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
        method.
---
 gdb/ChangeLog        |  9 +++++++++
 gdb/fbsd-tdep.c      | 14 ++++++++++++++
 gdb/fbsd-tdep.h      |  5 +++++
 gdb/mips-fbsd-tdep.c | 16 ++++++++++++++++
 4 files changed, 44 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1b5bc8b458..ceff8178d7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2020-07-17  John Baldwin  <[hidden email]>
+
+ * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
+ (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
+ * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
+ * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
+ (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
+ method.
+
 2020-07-17  Tom Tromey  <[hidden email]>
 
  * linux-nat.c (linux_nat_target::supports_non_stop)
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 557c5d3d73..ca397fa8e0 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -21,6 +21,7 @@
 #include "auxv.h"
 #include "gdbcore.h"
 #include "inferior.h"
+#include "objfiles.h"
 #include "regcache.h"
 #include "regset.h"
 #include "gdbthread.h"
@@ -2071,6 +2072,18 @@ fbsd_get_thread_local_address (struct gdbarch *gdbarch, CORE_ADDR dtv_addr,
   return addr + offset;
 }
 
+/* See fbsd-tdep.h.  */
+
+CORE_ADDR
+fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  struct bound_minimal_symbol msym = lookup_bound_minimal_symbol ("_rtld_bind");
+  if (msym.minsym != nullptr && BMSYMBOL_VALUE_ADDRESS (msym) == pc)
+    return frame_unwind_caller_pc (get_current_frame ());
+
+  return 0;
+}
+
 /* To be called from GDB_OSABI_FREEBSD handlers. */
 
 void
@@ -2085,6 +2098,7 @@ fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_get_siginfo_type (gdbarch, fbsd_get_siginfo_type);
   set_gdbarch_gdb_signal_from_target (gdbarch, fbsd_gdb_signal_from_target);
   set_gdbarch_gdb_signal_to_target (gdbarch, fbsd_gdb_signal_to_target);
+  set_gdbarch_skip_solib_resolver (gdbarch, fbsd_skip_solib_resolver);
 
   /* `catch syscall' */
   set_xml_syscall_file_name (gdbarch, "syscalls/freebsd.xml");
diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
index 5dd8203299..e8e49dc25f 100644
--- a/gdb/fbsd-tdep.h
+++ b/gdb/fbsd-tdep.h
@@ -71,4 +71,9 @@ extern CORE_ADDR fbsd_get_thread_local_address (struct gdbarch *gdbarch,
  CORE_ADDR lm_addr,
  CORE_ADDR offset);
 
+/* Implement the "skip_solib_resolver" gdbarch method.  */
+
+extern CORE_ADDR fbsd_skip_solib_resolver (struct gdbarch *gdbarch,
+   CORE_ADDR pc);
+
 #endif /* fbsd-tdep.h */
diff --git a/gdb/mips-fbsd-tdep.c b/gdb/mips-fbsd-tdep.c
index abaf7f2474..ba666f1dc5 100644
--- a/gdb/mips-fbsd-tdep.c
+++ b/gdb/mips-fbsd-tdep.c
@@ -462,6 +462,20 @@ static const struct tramp_frame mips64_fbsd_sigframe =
 
 /* Shared library support.  */
 
+/* FreeBSD/mips can use an alternate routine in the runtime linker to
+   resolve functions.  */
+
+static CORE_ADDR
+mips_fbsd_skip_solib_resolver (struct gdbarch *gdbarch, CORE_ADDR pc)
+{
+  struct bound_minimal_symbol msym
+    = lookup_bound_minimal_symbol ("_mips_rtld_bind");
+  if (msym.minsym != nullptr && BMSYMBOL_VALUE_ADDRESS (msym) == pc)
+    return frame_unwind_caller_pc (get_current_frame ());
+
+  return fbsd_skip_solib_resolver (gdbarch, pc);
+}
+
 /* FreeBSD/mips uses a slightly different `struct link_map' than the
    other FreeBSD platforms as it includes an additional `l_off'
    member.  */
@@ -546,6 +560,8 @@ mips_fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_iterate_over_regset_sections
     (gdbarch, mips_fbsd_iterate_over_regset_sections);
 
+  set_gdbarch_skip_solib_resolver (gdbarch, mips_fbsd_skip_solib_resolver);
+
   /* FreeBSD/mips has SVR4-style shared libraries.  */
   set_solib_svr4_fetch_link_map_offsets
     (gdbarch, (gdbarch_ptr_bit (gdbarch) == 32 ?
--
2.25.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2] Implement the skip_solib_resolver gdbarch hook for FreeBSD architectures.

Simon Marchi-4
On 2020-07-17 6:59 p.m., John Baldwin wrote:

> The ELF runtime linker on all FreeBSD architectures uses the
> "_rtld_bind" entry point for unresolved PTL entries.  FreeBSD/mips has
> an additional entry point called "_mips_rtld_bind".
>
> gdb/ChangeLog:
>
> * fbsd-tdep.c (fbsd_skip_solib_resolver): New function.
> (fbsd_init_abi): Install gdbarch "skip_solib_resolver" method.
> * fbsd-tdep.h (fbsd_skip_solib_resolver): New prototype.
> * mips-fbsd-tdep.c (mips_fbsd_skip_solib_resolver): New function.
> (mips_fbsd_init_abi): Install gdbarch "skip_solib_resolver"
> method.

Thanks, this LGTM!

Simon