[binutils-gdb] Catching errors on probes-based dynamic linker interface

[Sergio Durigan Junior <sergiodj at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=3bd7e5b7ee5ea0b3bbb4030ca841f66faad74f0f

commit 3bd7e5b7ee5ea0b3bbb4030ca841f66faad74f0f
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date:   Fri Aug 21 18:28:07 2015 -0400

    Catching errors on probes-based dynamic linker interface
    
    This patch is intended to make the interaction between the
    probes-based dynamic linker interface and the SystemTap SDT probe code
    on GDB more robust.  It does that by wrapping the calls to the probe
    API with TRY...CATCH'es, so that any exception thrown will be caught
    and handled properly.
    
    The idea for this patch came from
    <https://bugzilla.redhat.com/show_bug.cgi?id=1196181>, which is a bug
    initially filed against Fedora GDB (but now under Fedora GLIBC).  This
    bug happens on armhfp (although it could happen on other targets as
    well), and is triggered because GCC generates a strange argument for
    one of the probes used by GDB in the dynamic linker interface.  As can
    be seen in the bug, this argument is "-4@.L1052".
    
    I don't want to discuss the reasons for this argument to be there
    (this discussion belongs to the bug, or to another thread), but GDB
    could definitely do a better error handling here.  Currently, one sees
    the following message when there is an error in the probes-based
    dynamic linker interface:
    
      (gdb) run
      Starting program: /bin/inferior
      warning: Probes-based dynamic linker interface failed.
      Reverting to original interface.
    
      Cannot parse expression `.L976 4@r4'.
      (gdb)
    
    Which means that one needs to explicitly issue a "continue" command to
    make GDB continue running the inferior, even though this error is not
    fatal and GDB will fallback to the old interface automatically.
    
    This is where this patch helps: it makes GDB still print the necessary
    warnings or error messages, but it *also* does not stop the inferior
    unnecessarily.
    
    I have tested this patch on the systems where this error happens, but
    I could not come up with a way to create a testcase for it.
    Nevertheless, it should be straightforward to see that this patch does
    improve the current situation.
    
    gdb/ChangeLog:
    2015-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
    
    	* solib-svr4.c (solib_event_probe_action): Call
    	get_probe_argument_count using TRY...CATCH.
    	(svr4_handle_solib_event): Likewise, for evaluate_probe_argument.

Diff:
---
 gdb/ChangeLog    |  6 ++++++
 gdb/solib-svr4.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index cd3eaa1..8ed469f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
 
+	* solib-svr4.c (solib_event_probe_action): Call
+	get_probe_argument_count using TRY...CATCH.
+	(svr4_handle_solib_event): Likewise, for evaluate_probe_argument.
+
+2015-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
+
 	* probe.h (struct probe_ops) <get_probe_argument_count,
 	evaluate_probe_argument, enable_probe, disable_probe>: Mention in
 	the comment that the function can throw an exception.
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 36b6c59..5d2b9dd 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1786,7 +1786,23 @@ solib_event_probe_action (struct probe_and_action *pa)
        arg0: Lmid_t lmid (mandatory)
        arg1: struct r_debug *debug_base (mandatory)
        arg2: struct link_map *new (optional, for incremental updates)  */
-  probe_argc = get_probe_argument_count (pa->probe, frame);
+  TRY
+    {
+      probe_argc = get_probe_argument_count (pa->probe, frame);
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      exception_print (gdb_stderr, ex);
+      probe_argc = 0;
+    }
+  END_CATCH
+
+  /* If get_probe_argument_count throws an exception, probe_argc will
+     be set to zero.  However, if pa->probe does not have arguments,
+     then get_probe_argument_count will succeed but probe_argc will
+     also be zero.  Both cases happen because of different things, but
+     they are treated equally here: action will be set to
+     PROBES_INTERFACE_FAILED.  */
   if (probe_argc == 2)
     action = FULL_RELOAD;
   else if (probe_argc < 2)
@@ -1940,7 +1956,17 @@ svr4_handle_solib_event (void)
   usm_chain = make_cleanup (resume_section_map_updates_cleanup,
 			    current_program_space);
 
-  val = evaluate_probe_argument (pa->probe, 1, frame);
+  TRY
+    {
+      val = evaluate_probe_argument (pa->probe, 1, frame);
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      exception_print (gdb_stderr, ex);
+      val = NULL;
+    }
+  END_CATCH
+
   if (val == NULL)
     {
       do_cleanups (old_chain);
@@ -1971,7 +1997,18 @@ svr4_handle_solib_event (void)
 
   if (action == UPDATE_OR_RELOAD)
     {
-      val = evaluate_probe_argument (pa->probe, 2, frame);
+      TRY
+	{
+	  val = evaluate_probe_argument (pa->probe, 2, frame);
+	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  exception_print (gdb_stderr, ex);
+	  do_cleanups (old_chain);
+	  return;
+	}
+      END_CATCH
+
       if (val != NULL)
 	lm = value_as_address (val);