public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Fix stop-on-solib event failures
@ 2019-08-21 15:58 Alan Hayward
  2019-08-21 15:58 ` [PATCH v2 2/3] Use gdbarch for probe::get_argument_count Alan Hayward
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Alan Hayward @ 2019-08-21 15:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

On some Arm targets (namely the buildbot Arm Docker setup) placing breakpoints
on just the solib dynamic probes will cause the target process to not stop.
This is due to the probes being invalid - see link in 3/3 for more details.

Fix is to fully validate the probes before using the,.

Patches 1 and 2 are code refactors. The actual fix is in patch 3.


Alan Hayward (3):
  Refactor svr4_create_solib_event_breakpoints
  Use gdbarch for probe::get_argument_count
  Check arguments for all probes before using them

 gdb/break-catch-throw.c |   2 +-
 gdb/dtrace-probe.c      |   4 +-
 gdb/probe.c             |   7 +-
 gdb/probe.h             |   2 +-
 gdb/solib-svr4.c        | 139 ++++++++++++++++++++--------------------
 gdb/stap-probe.c        |   6 +-
 6 files changed, 80 insertions(+), 80 deletions(-)

-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] Use gdbarch for probe::get_argument_count
  2019-08-21 15:58 [PATCH v2 0/3] Fix stop-on-solib event failures Alan Hayward
@ 2019-08-21 15:58 ` Alan Hayward
  2019-09-04 17:58   ` Sergio Durigan Junior
  2019-08-21 15:58 ` [PATCH v2 1/3] Refactor svr4_create_solib_event_breakpoints Alan Hayward
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Alan Hayward @ 2019-08-21 15:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

The probe function get_argument_count does not need a frame, only
the current gdbarch.  Switch the code to pass gdbarch instead.
No functional changes.

gdb/ChangeLog:

2019-08-21  Alan Hayward  <alan.hayward@arm.com>

	* break-catch-throw.c (fetch_probe_arguments): Use gdbarch.
	* dtrace-probe.c (dtrace_probe::get_argument_count): Likewise.
	* probe.c (probe_safe_evaluate_at_pc) (compute_probe_arg)
	(compile_probe_arg): Likewise.
	* probe.h (get_argument_count): Likewise.
	* solib-svr4.c (solib_event_probe_action): Likewise.
	* stap-probe.c (stap_probe::get_argument_count): Likewise.
---
 gdb/break-catch-throw.c | 2 +-
 gdb/dtrace-probe.c      | 4 ++--
 gdb/probe.c             | 7 +++----
 gdb/probe.h             | 2 +-
 gdb/solib-svr4.c        | 2 +-
 gdb/stap-probe.c        | 6 ++----
 6 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 0677a55ee5..2c2a3b7d72 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -113,7 +113,7 @@ fetch_probe_arguments (struct value **arg0, struct value **arg1)
 	  && pc_probe.prob->get_name () != "rethrow"))
     error (_("not stopped at a C++ exception catchpoint"));
 
-  n_args = pc_probe.prob->get_argument_count (frame);
+  n_args = pc_probe.prob->get_argument_count (get_frame_arch (frame));
   if (n_args < 2)
     error (_("C++ exception catchpoint has too few arguments"));
 
diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index 2d92edb11c..e9e71fd4c9 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -122,7 +122,7 @@ public:
   CORE_ADDR get_relocated_address (struct objfile *objfile) override;
 
   /* See probe.h.  */
-  unsigned get_argument_count (struct frame_info *frame) override;
+  unsigned get_argument_count (struct gdbarch *gdbarch) override;
 
   /* See probe.h.  */
   bool can_evaluate_arguments () const override;
@@ -693,7 +693,7 @@ dtrace_probe::get_relocated_address (struct objfile *objfile)
 /* Implementation of the get_argument_count method.  */
 
 unsigned
-dtrace_probe::get_argument_count (struct frame_info *frame)
+dtrace_probe::get_argument_count (struct gdbarch *gdbarch)
 {
   return m_args.size ();
 }
diff --git a/gdb/probe.c b/gdb/probe.c
index cdc6e021d2..8b108d6b02 100644
--- a/gdb/probe.c
+++ b/gdb/probe.c
@@ -695,7 +695,7 @@ probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
   if (!probe.prob)
     return NULL;
 
-  n_args = probe.prob->get_argument_count (frame);
+  n_args = probe.prob->get_argument_count (get_frame_arch (frame));
   if (n >= n_args)
     return NULL;
 
@@ -818,7 +818,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
   if (pc_probe.prob == NULL)
     error (_("No probe at PC %s"), core_addr_to_string (pc));
 
-  n_args = pc_probe.prob->get_argument_count (frame);
+  n_args = pc_probe.prob->get_argument_count (arch);
   if (sel == -1)
     return value_from_longest (builtin_type (arch)->builtin_int, n_args);
 
@@ -840,7 +840,6 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
   int sel = (int) (uintptr_t) data;
   struct bound_probe pc_probe;
   int n_args;
-  struct frame_info *frame = get_selected_frame (NULL);
 
   /* SEL == -1 means "_probe_argc".  */
   gdb_assert (sel >= -1);
@@ -849,7 +848,7 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
   if (pc_probe.prob == NULL)
     error (_("No probe at PC %s"), core_addr_to_string (pc));
 
-  n_args = pc_probe.prob->get_argument_count (frame);
+  n_args = pc_probe.prob->get_argument_count (expr->gdbarch);
 
   if (sel == -1)
     {
diff --git a/gdb/probe.h b/gdb/probe.h
index 8abf69e354..7410c5aadf 100644
--- a/gdb/probe.h
+++ b/gdb/probe.h
@@ -131,7 +131,7 @@ public:
 
   /* Return the number of arguments of the probe.  This function can
      throw an exception.  */
-  virtual unsigned get_argument_count (struct frame_info *frame) = 0;
+  virtual unsigned get_argument_count (struct gdbarch *gdbarch) = 0;
 
   /* Return 1 if the probe interface can evaluate the arguments of
      probe, zero otherwise.  See the comments on
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index b21eacb68f..2a44dd6985 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1756,7 +1756,7 @@ solib_event_probe_action (struct probe_and_action *pa)
        arg2: struct link_map *new (optional, for incremental updates)  */
   try
     {
-      probe_argc = pa->prob->get_argument_count (frame);
+      probe_argc = pa->prob->get_argument_count (get_frame_arch (frame));
     }
   catch (const gdb_exception_error &ex)
     {
diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index b6de873d2c..700b657967 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -136,7 +136,7 @@ public:
   CORE_ADDR get_relocated_address (struct objfile *objfile) override;
 
   /* See probe.h.  */
-  unsigned get_argument_count (struct frame_info *frame) override;
+  unsigned get_argument_count (struct gdbarch *gdbarch) override;
 
   /* See probe.h.  */
   bool can_evaluate_arguments () const override;
@@ -1301,10 +1301,8 @@ stap_probe::get_relocated_address (struct objfile *objfile)
    argument string.  */
 
 unsigned
-stap_probe::get_argument_count (struct frame_info *frame)
+stap_probe::get_argument_count (struct gdbarch *gdbarch)
 {
-  struct gdbarch *gdbarch = get_frame_arch (frame);
-
   if (!m_have_parsed_args)
     {
       if (this->can_evaluate_arguments ())
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] Refactor svr4_create_solib_event_breakpoints
  2019-08-21 15:58 [PATCH v2 0/3] Fix stop-on-solib event failures Alan Hayward
  2019-08-21 15:58 ` [PATCH v2 2/3] Use gdbarch for probe::get_argument_count Alan Hayward
@ 2019-08-21 15:58 ` Alan Hayward
  2019-08-21 15:58 ` [PATCH v2 3/3] Check arguments for all probes before using them Alan Hayward
  2019-08-30 15:51 ` [PATCH v2 0/3] Fix stop-on-solib event failures Gary Benson
  3 siblings, 0 replies; 8+ messages in thread
From: Alan Hayward @ 2019-08-21 15:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

Move the bulk of svr4_create_solib_event_breakpoints into a new
function to simplify the logic. No functional changes.

gdb/ChangeLog:

2019-08-21  Alan Hayward  <alan.hayward@arm.com>

	* solib-svr4.c (svr4_find_and_create_probe_breakpoints): Move
	code to here...
	(svr4_create_solib_event_breakpoints): ...from here.
---
 gdb/solib-svr4.c | 127 ++++++++++++++++++++++-------------------------
 1 file changed, 60 insertions(+), 67 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index c0c505acaa..b21eacb68f 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2061,6 +2061,61 @@ svr4_create_probe_breakpoints (svr4_info *info, struct gdbarch *gdbarch,
   svr4_update_solib_event_breakpoints ();
 }
 
+/* Find all the glibc named probes.  Only if all of the probes are found, then
+   create them and return true.  Otherwise return false.  If WITH_PREFIX is set
+   then add "rtld" to the front of the probe names.  */
+static bool
+svr4_find_and_create_probe_breakpoints (svr4_info *info,
+					struct gdbarch *gdbarch,
+					struct obj_section *os,
+					bool with_prefix)
+{
+  std::vector<probe *> probes[NUM_PROBES];
+  bool checked_can_use_probe_arguments = false;
+
+  for (int i = 0; i < NUM_PROBES; i++)
+    {
+      const char *name = probe_info[i].name;
+      char buf[32];
+
+      /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4 shipped with an early
+	 version of the probes code in which the probes' names were prefixed
+	 with "rtld_" and the "map_failed" probe did not exist.  The locations
+	 of the probes are otherwise the same, so we check for probes with
+	 prefixed names if probes with unprefixed names are not present.  */
+      if (with_prefix)
+	{
+	  xsnprintf (buf, sizeof (buf), "rtld_%s", name);
+	  name = buf;
+	}
+
+      probes[i] = find_probes_in_objfile (os->objfile, "rtld", name);
+
+      /* The "map_failed" probe did not exist in early
+	 versions of the probes code in which the probes'
+	 names were prefixed with "rtld_".  */
+      if (with_prefix && streq (name, "rtld_map_failed"))
+	continue;
+
+      /* Ensure at least one probe for the current name was found.  */
+      if (probes[i].empty ())
+	return false;
+
+      /* Ensure probe arguments can be evaluated.  */
+      if (!checked_can_use_probe_arguments)
+	{
+	  probe *p = probes[i][0];
+	  if (!p->can_evaluate_arguments ())
+	    return false;
+	  checked_can_use_probe_arguments = true;
+	}
+    }
+
+  /* All probes found.  Now create them.  */
+  svr4_create_probe_breakpoints (info, gdbarch, probes, os->objfile);
+  return true;
+}
+
 /* Both the SunOS and the SVR4 dynamic linkers call a marker function
    before and after mapping and unmapping shared libraries.  The sole
    purpose of this method is to allow debuggers to set a breakpoint so
@@ -2077,74 +2132,12 @@ static void
 svr4_create_solib_event_breakpoints (svr4_info *info, struct gdbarch *gdbarch,
 				     CORE_ADDR address)
 {
-  struct obj_section *os;
-
-  os = find_pc_section (address);
-  if (os != NULL)
-    {
-      int with_prefix;
-
-      for (with_prefix = 0; with_prefix <= 1; with_prefix++)
-	{
-	  std::vector<probe *> probes[NUM_PROBES];
-	  int all_probes_found = 1;
-	  int checked_can_use_probe_arguments = 0;
-
-	  for (int i = 0; i < NUM_PROBES; i++)
-	    {
-	      const char *name = probe_info[i].name;
-	      probe *p;
-	      char buf[32];
-
-	      /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4
-		 shipped with an early version of the probes code in
-		 which the probes' names were prefixed with "rtld_"
-		 and the "map_failed" probe did not exist.  The
-		 locations of the probes are otherwise the same, so
-		 we check for probes with prefixed names if probes
-		 with unprefixed names are not present.  */
-	      if (with_prefix)
-		{
-		  xsnprintf (buf, sizeof (buf), "rtld_%s", name);
-		  name = buf;
-		}
-
-	      probes[i] = find_probes_in_objfile (os->objfile, "rtld", name);
-
-	      /* The "map_failed" probe did not exist in early
-		 versions of the probes code in which the probes'
-		 names were prefixed with "rtld_".  */
-	      if (strcmp (name, "rtld_map_failed") == 0)
-		continue;
-
-	      if (probes[i].empty ())
-		{
-		  all_probes_found = 0;
-		  break;
-		}
-
-	      /* Ensure probe arguments can be evaluated.  */
-	      if (!checked_can_use_probe_arguments)
-		{
-		  p = probes[i][0];
-		  if (!p->can_evaluate_arguments ())
-		    {
-		      all_probes_found = 0;
-		      break;
-		    }
-		  checked_can_use_probe_arguments = 1;
-		}
-	    }
-
-	  if (all_probes_found)
-	    svr4_create_probe_breakpoints (info, gdbarch, probes, os->objfile);
-
-	  if (all_probes_found)
-	    return;
-	}
-    }
+  struct obj_section *os = find_pc_section (address);
 
-  create_solib_event_breakpoint (gdbarch, address);
+  if (os == nullptr
+      || (!svr4_find_and_create_probe_breakpoints (info, gdbarch, os, false)
+	  && !svr4_find_and_create_probe_breakpoints (info, gdbarch, os, true)))
+    create_solib_event_breakpoint (gdbarch, address);
 }
 
 /* Helper function for gdb_bfd_lookup_symbol.  */
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] Check arguments for all probes before using them
  2019-08-21 15:58 [PATCH v2 0/3] Fix stop-on-solib event failures Alan Hayward
  2019-08-21 15:58 ` [PATCH v2 2/3] Use gdbarch for probe::get_argument_count Alan Hayward
  2019-08-21 15:58 ` [PATCH v2 1/3] Refactor svr4_create_solib_event_breakpoints Alan Hayward
@ 2019-08-21 15:58 ` Alan Hayward
  2019-08-30 15:51 ` [PATCH v2 0/3] Fix stop-on-solib event failures Gary Benson
  3 siblings, 0 replies; 8+ messages in thread
From: Alan Hayward @ 2019-08-21 15:58 UTC (permalink / raw)
  To: gdb-patches; +Cc: nd, Alan Hayward

There is a long standing bug in the Arm toolchain where invalid
stap probes get created due to the probes referring to symbols which
have been resolved away.

More details are here:
https://bugzilla.redhat.com/show_bug.cgi?id=1196181

When these invalid probes are present, GDB will create the breakpoints
and then fail to stop. The errors are only spotted the first time
GDB stops, which is too late.

The solution is to ensure the arguments for all the probes are
resolved before using them.

This fixes >100 timeouts when running break-interp.exp when using
bad probes.

gdb/ChangeLog:

2019-08-21  Alan Hayward  <alan.hayward@arm.com>

	* solib-svr4.c (svr4_find_and_create_probe_breakpoints): Check all
	probe arguments.
---
 gdb/solib-svr4.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 2a44dd6985..ffae26bfc5 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2071,7 +2071,6 @@ svr4_find_and_create_probe_breakpoints (svr4_info *info,
 					bool with_prefix)
 {
   std::vector<probe *> probes[NUM_PROBES];
-  bool checked_can_use_probe_arguments = false;
 
   for (int i = 0; i < NUM_PROBES; i++)
     {
@@ -2102,12 +2101,23 @@ svr4_find_and_create_probe_breakpoints (svr4_info *info,
 	return false;
 
       /* Ensure probe arguments can be evaluated.  */
-      if (!checked_can_use_probe_arguments)
+      for (probe *p : probes[i])
 	{
-	  probe *p = probes[i][0];
 	  if (!p->can_evaluate_arguments ())
 	    return false;
-	  checked_can_use_probe_arguments = true;
+	  /* This will fail if the probe is invalid.  This has been seen on Arm
+	     due to references to symbols that have been resolved away.  */
+	  try
+	    {
+	      p->get_argument_count (gdbarch);
+	    }
+	  catch (const gdb_exception_error &ex)
+	    {
+	      exception_print (gdb_stderr, ex);
+	      warning (_("Initializing probes-based dynamic linker interface "
+			 "failed.\nReverting to original interface."));
+	      return false;
+	    }
 	}
     }
 
-- 
2.20.1 (Apple Git-117)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] Fix stop-on-solib event failures
  2019-08-21 15:58 [PATCH v2 0/3] Fix stop-on-solib event failures Alan Hayward
                   ` (2 preceding siblings ...)
  2019-08-21 15:58 ` [PATCH v2 3/3] Check arguments for all probes before using them Alan Hayward
@ 2019-08-30 15:51 ` Gary Benson
  2019-09-02 13:20   ` Alan Hayward
  3 siblings, 1 reply; 8+ messages in thread
From: Gary Benson @ 2019-08-30 15:51 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

Hi Alan,

Alan Hayward wrote:
> On some Arm targets (namely the buildbot Arm Docker setup) placing
> breakpoints on just the solib dynamic probes will cause the target
> process to not stop.  This is due to the probes being invalid - see
> link in 3/3 for more details.
> 
> Fix is to fully validate the probes before using the,.
> 
> Patches 1 and 2 are code refactors. The actual fix is in patch 3.

The code looks good to me, my only caveat being that I think we're
supposed to wrap lines at 72 columns (unless that changed) and some
lines in your patches seem too long.  I noticed it in comments, but
possibly it's in code too.  With that fixed (or, not fixed if we
don't wrap at 72 columns any more) I'd say this is good to commit.
Thank you for doing the work!

Cheers,
Gary

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 0/3] Fix stop-on-solib event failures
  2019-08-30 15:51 ` [PATCH v2 0/3] Fix stop-on-solib event failures Gary Benson
@ 2019-09-02 13:20   ` Alan Hayward
  0 siblings, 0 replies; 8+ messages in thread
From: Alan Hayward @ 2019-09-02 13:20 UTC (permalink / raw)
  To: Gary Benson; +Cc: gdb-patches, nd



> On 30 Aug 2019, at 16:51, Gary Benson <gbenson@redhat.com> wrote:
> 
> Hi Alan,
> 
> Alan Hayward wrote:
>> On some Arm targets (namely the buildbot Arm Docker setup) placing
>> breakpoints on just the solib dynamic probes will cause the target
>> process to not stop.  This is due to the probes being invalid - see
>> link in 3/3 for more details.
>> 
>> Fix is to fully validate the probes before using the,.
>> 
>> Patches 1 and 2 are code refactors. The actual fix is in patch 3.
> 
> The code looks good to me, my only caveat being that I think we're
> supposed to wrap lines at 72 columns (unless that changed) and some
> lines in your patches seem too long.  I noticed it in comments, but
> possibly it's in code too.  With that fixed (or, not fixed if we
> don't wrap at 72 columns any more) I'd say this is good to commit.
> Thank you for doing the work!
> 

I’ve always kept within 80. Double checking the coding standards, it has:
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Column_limits
I’ve always wondered why there’s a lot of code that wraps earlier and
that explains why. (personally I’d prefer even longer, but that’s just me).

Anyway, pushed patch to head.
Thanks!

Alan.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] Use gdbarch for probe::get_argument_count
  2019-08-21 15:58 ` [PATCH v2 2/3] Use gdbarch for probe::get_argument_count Alan Hayward
@ 2019-09-04 17:58   ` Sergio Durigan Junior
  2019-09-04 18:00     ` Sergio Durigan Junior
  0 siblings, 1 reply; 8+ messages in thread
From: Sergio Durigan Junior @ 2019-09-04 17:58 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

On Wednesday, August 21 2019, Alan Hayward wrote:

> The probe function get_argument_count does not need a frame, only
> the current gdbarch.  Switch the code to pass gdbarch instead.
> No functional changes.

The patch looks good at first glance, but I'd like to apply it before
approving.  Unfortunately, it seems like your MUA has mangled the
message (which is strange, because apparently you're using
git-send-email):

  <https://sourceware.org/cgi-bin/get-raw-msg?listname=gdb-patches&date=2019-08&msgid=20190821155816.45504-3-alan.hayward%40arm.com>

Can you resend it, please?

> gdb/ChangeLog:
>
> 2019-08-21  Alan Hayward  <alan.hayward@arm.com>
>
> 	* break-catch-throw.c (fetch_probe_arguments): Use gdbarch.
> 	* dtrace-probe.c (dtrace_probe::get_argument_count): Likewise.
> 	* probe.c (probe_safe_evaluate_at_pc) (compute_probe_arg)
> 	(compile_probe_arg): Likewise.
> 	* probe.h (get_argument_count): Likewise.
> 	* solib-svr4.c (solib_event_probe_action): Likewise.
> 	* stap-probe.c (stap_probe::get_argument_count): Likewise.
> ---
>  gdb/break-catch-throw.c | 2 +-
>  gdb/dtrace-probe.c      | 4 ++--
>  gdb/probe.c             | 7 +++----
>  gdb/probe.h             | 2 +-
>  gdb/solib-svr4.c        | 2 +-
>  gdb/stap-probe.c        | 6 ++----
>  6 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
> index 0677a55ee5..2c2a3b7d72 100644
> --- a/gdb/break-catch-throw.c
> +++ b/gdb/break-catch-throw.c
> @@ -113,7 +113,7 @@ fetch_probe_arguments (struct value **arg0, struct value **arg1)
>  	  && pc_probe.prob->get_name () != "rethrow"))
>      error (_("not stopped at a C++ exception catchpoint"));
>  
> -  n_args = pc_probe.prob->get_argument_count (frame);
> +  n_args = pc_probe.prob->get_argument_count (get_frame_arch (frame));
>    if (n_args < 2)
>      error (_("C++ exception catchpoint has too few arguments"));
>  
> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
> index 2d92edb11c..e9e71fd4c9 100644
> --- a/gdb/dtrace-probe.c
> +++ b/gdb/dtrace-probe.c
> @@ -122,7 +122,7 @@ public:
>    CORE_ADDR get_relocated_address (struct objfile *objfile) override;
>  
>    /* See probe.h.  */
> -  unsigned get_argument_count (struct frame_info *frame) override;
> +  unsigned get_argument_count (struct gdbarch *gdbarch) override;
>  
>    /* See probe.h.  */
>    bool can_evaluate_arguments () const override;
> @@ -693,7 +693,7 @@ dtrace_probe::get_relocated_address (struct objfile *objfile)
>  /* Implementation of the get_argument_count method.  */
>  
>  unsigned
> -dtrace_probe::get_argument_count (struct frame_info *frame)
> +dtrace_probe::get_argument_count (struct gdbarch *gdbarch)
>  {
>    return m_args.size ();
>  }
> diff --git a/gdb/probe.c b/gdb/probe.c
> index cdc6e021d2..8b108d6b02 100644
> --- a/gdb/probe.c
> +++ b/gdb/probe.c
> @@ -695,7 +695,7 @@ probe_safe_evaluate_at_pc (struct frame_info *frame, unsigned n)
>    if (!probe.prob)
>      return NULL;
>  
> -  n_args = probe.prob->get_argument_count (frame);
> +  n_args = probe.prob->get_argument_count (get_frame_arch (frame));
>    if (n >= n_args)
>      return NULL;
>  
> @@ -818,7 +818,7 @@ compute_probe_arg (struct gdbarch *arch, struct internalvar *ivar,
>    if (pc_probe.prob == NULL)
>      error (_("No probe at PC %s"), core_addr_to_string (pc));
>  
> -  n_args = pc_probe.prob->get_argument_count (frame);
> +  n_args = pc_probe.prob->get_argument_count (arch);
>    if (sel == -1)
>      return value_from_longest (builtin_type (arch)->builtin_int, n_args);
>  
> @@ -840,7 +840,6 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
>    int sel = (int) (uintptr_t) data;
>    struct bound_probe pc_probe;
>    int n_args;
> -  struct frame_info *frame = get_selected_frame (NULL);
>  
>    /* SEL == -1 means "_probe_argc".  */
>    gdb_assert (sel >= -1);
> @@ -849,7 +848,7 @@ compile_probe_arg (struct internalvar *ivar, struct agent_expr *expr,
>    if (pc_probe.prob == NULL)
>      error (_("No probe at PC %s"), core_addr_to_string (pc));
>  
> -  n_args = pc_probe.prob->get_argument_count (frame);
> +  n_args = pc_probe.prob->get_argument_count (expr->gdbarch);
>  
>    if (sel == -1)
>      {
> diff --git a/gdb/probe.h b/gdb/probe.h
> index 8abf69e354..7410c5aadf 100644
> --- a/gdb/probe.h
> +++ b/gdb/probe.h
> @@ -131,7 +131,7 @@ public:
>  
>    /* Return the number of arguments of the probe.  This function can
>       throw an exception.  */
> -  virtual unsigned get_argument_count (struct frame_info *frame) = 0;
> +  virtual unsigned get_argument_count (struct gdbarch *gdbarch) = 0;
>  
>    /* Return 1 if the probe interface can evaluate the arguments of
>       probe, zero otherwise.  See the comments on
> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
> index b21eacb68f..2a44dd6985 100644
> --- a/gdb/solib-svr4.c
> +++ b/gdb/solib-svr4.c
> @@ -1756,7 +1756,7 @@ solib_event_probe_action (struct probe_and_action *pa)
>         arg2: struct link_map *new (optional, for incremental updates)  */
>    try
>      {
> -      probe_argc = pa->prob->get_argument_count (frame);
> +      probe_argc = pa->prob->get_argument_count (get_frame_arch (frame));
>      }
>    catch (const gdb_exception_error &ex)
>      {
> diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
> index b6de873d2c..700b657967 100644
> --- a/gdb/stap-probe.c
> +++ b/gdb/stap-probe.c
> @@ -136,7 +136,7 @@ public:
>    CORE_ADDR get_relocated_address (struct objfile *objfile) override;
>  
>    /* See probe.h.  */
> -  unsigned get_argument_count (struct frame_info *frame) override;
> +  unsigned get_argument_count (struct gdbarch *gdbarch) override;
>  
>    /* See probe.h.  */
>    bool can_evaluate_arguments () const override;
> @@ -1301,10 +1301,8 @@ stap_probe::get_relocated_address (struct objfile *objfile)
>     argument string.  */
>  
>  unsigned
> -stap_probe::get_argument_count (struct frame_info *frame)
> +stap_probe::get_argument_count (struct gdbarch *gdbarch)
>  {
> -  struct gdbarch *gdbarch = get_frame_arch (frame);
> -
>    if (!m_have_parsed_args)
>      {
>        if (this->can_evaluate_arguments ())
> -- 
> 2.20.1 (Apple Git-117)

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 2/3] Use gdbarch for probe::get_argument_count
  2019-09-04 17:58   ` Sergio Durigan Junior
@ 2019-09-04 18:00     ` Sergio Durigan Junior
  0 siblings, 0 replies; 8+ messages in thread
From: Sergio Durigan Junior @ 2019-09-04 18:00 UTC (permalink / raw)
  To: Alan Hayward; +Cc: gdb-patches, nd

On Wednesday, September 04 2019, I wrote:

> On Wednesday, August 21 2019, Alan Hayward wrote:
>
>> The probe function get_argument_count does not need a frame, only
>> the current gdbarch.  Switch the code to pass gdbarch instead.
>> No functional changes.
>
> The patch looks good at first glance, but I'd like to apply it before
> approving.  Unfortunately, it seems like your MUA has mangled the
> message (which is strange, because apparently you're using
> git-send-email):
>
>   <https://sourceware.org/cgi-bin/get-raw-msg?listname=gdb-patches&date=2019-08&msgid=20190821155816.45504-3-alan.hayward%40arm.com>
>
> Can you resend it, please?

Ah, I see that the whole patch series has already been pushed.  Sorry,
and please disregard my message.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-09-04 18:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 15:58 [PATCH v2 0/3] Fix stop-on-solib event failures Alan Hayward
2019-08-21 15:58 ` [PATCH v2 2/3] Use gdbarch for probe::get_argument_count Alan Hayward
2019-09-04 17:58   ` Sergio Durigan Junior
2019-09-04 18:00     ` Sergio Durigan Junior
2019-08-21 15:58 ` [PATCH v2 1/3] Refactor svr4_create_solib_event_breakpoints Alan Hayward
2019-08-21 15:58 ` [PATCH v2 3/3] Check arguments for all probes before using them Alan Hayward
2019-08-30 15:51 ` [PATCH v2 0/3] Fix stop-on-solib event failures Gary Benson
2019-09-02 13:20   ` Alan Hayward

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).