[rpms/gdb] gdb-17.2-rebase-f44: Fix 'Make the probes-based dynamic linker interface more robust to errors' (Sergio Durigan Junior, RH BZ 1259132).

[Sergio Durigan Junior <sergiodj@redhat.com>]

A new commit has been pushed.

Repo   : rpms/gdb
Branch : gdb-17.2-rebase-f44
Commit : 1c8228902d89a127c86da2768b7b84a3994df7c0
Author : Sergio Durigan Junior <sergiodj@redhat.com>
Date   : 2015-09-02T16:02:54-04:00
Stats  : +399/-1 in 3 file(s)
URL    : https://src.fedoraproject.org/rpms/gdb/c/1c8228902d89a127c86da2768b7b84a3994df7c0?branch=gdb-17.2-rebase-f44

Log:
Fix 'Make the probes-based dynamic linker interface more robust to errors' (Sergio Durigan Junior, RH BZ 1259132).

---
diff --git a/gdb-probes-based-interface-robust-1of2.patch b/gdb-probes-based-interface-robust-1of2.patch
new file mode 100644
index 0000000..5295e06
--- /dev/null
+++ b/gdb-probes-based-interface-robust-1of2.patch
@@ -0,0 +1,193 @@
+From f469e8ce11672e26feb5ba6f9a134275fcfd5b4f Mon Sep 17 00:00:00 2001
+From: Sergio Durigan Junior <sergiodj@redhat.com>
+Date: Fri, 21 Aug 2015 18:13:46 -0400
+Subject: [PATCH 1/4] Improve error reporting when handling SystemTap SDT
+ probes
+
+This patch improves the error reporting when handling SystemTap SDT
+probes.  "Handling", in this case, mostly means "parsing".
+
+On gdb/probe.h, only trivial changes on functions' comments in order
+to explicitly mention that some of them can throw exceptions.  This is
+just to make the API a bit more clear.
+
+On gdb/stap-probe.c, I have s/internal_error/error/ on two functions
+that are responsible for parsing specific bits of the probes'
+arguments: stap_get_opcode and stap_get_expected_argument_type.  It is
+not correct to call internal_error on such situations because it is
+not really GDB's fault if the probes have malformed arguments.  I also
+improved the error reported on stap_get_expected_argument_type by also
+including the probe name on it.
+
+Aside from that, and perhaps most importantly, I added a check on
+stap_get_arg to make sure that we don't try to extract an argument
+from a probe that has no arguments.  This check issues an
+internal_error, because it really means that GDB is doing something it
+shouldn't.
+
+Although it can be considered almost trivial, and despite the fact
+that I am the maintainer for this part of the code, I am posting this
+patch for review.  I will wait a few days, and if nobody has anything
+to say, I will go ahead and push it.
+
+gdb/ChangeLog:
+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.
+	(get_probe_argument_count): Likewise.
+	(evaluate_probe_argument): Likewise.
+	* stap-probe.c (stap_get_opcode): Call error instead of
+	internal_error.
+	(stap_get_expected_argument_type): Likewise.  Add argument
+	'probe'.  Improve error message by mentioning the probe's name.
+	(stap_parse_probe_arguments): Adjust call to
+	stap_get_expected_argument_type.
+	(stap_get_arg): Add comment.  Assert that 'probe->args_parsed' is
+	not zero.  Call internal_error if GDB requests an argument but the
+	probe has no arguments.
+---
+ gdb/ChangeLog    | 17 +++++++++++++++++
+ gdb/probe.h      | 20 ++++++++++++++------
+ gdb/stap-probe.c | 29 ++++++++++++++++++++++-------
+ 3 files changed, 53 insertions(+), 13 deletions(-)
+
+Index: gdb-7.10/gdb/probe.h
+===================================================================
+--- gdb-7.10.orig/gdb/probe.h
++++ gdb-7.10/gdb/probe.h
+@@ -70,7 +70,8 @@ struct probe_ops
+     CORE_ADDR (*get_probe_address) (struct probe *probe,
+ 				    struct objfile *objfile);
+ 
+-    /* Return the number of arguments of PROBE.  */
++    /* Return the number of arguments of PROBE.  This function can
++       throw an exception.  */
+ 
+     unsigned (*get_probe_argument_count) (struct probe *probe,
+ 					  struct frame_info *frame);
+@@ -82,7 +83,8 @@ struct probe_ops
+     int (*can_evaluate_probe_arguments) (struct probe *probe);
+ 
+     /* Evaluate the Nth argument from the PROBE, returning a value
+-       corresponding to it.  The argument number is represented N.  */
++       corresponding to it.  The argument number is represented N.
++       This function can throw an exception.  */
+ 
+     struct value *(*evaluate_probe_argument) (struct probe *probe,
+ 					      unsigned n,
+@@ -141,13 +143,15 @@ struct probe_ops
+ 
+     /* Enable a probe.  The semantics of "enabling" a probe depend on
+        the specific backend and the field can be NULL in case enabling
+-       probes is not supported.  */
++       probes is not supported.  This function can throw an
++       exception.  */
+ 
+     void (*enable_probe) (struct probe *probe);
+ 
+     /* Disable a probe.  The semantics of "disabling" a probe depend
+        on the specific backend and the field can be NULL in case
+-       disabling probes is not supported.  */
++       disabling probes is not supported.  This function can throw an
++       exception.  */
+ 
+     void (*disable_probe) (struct probe *probe);
+   };
+@@ -264,7 +268,9 @@ extern struct cmd_list_element **info_pr
+ extern CORE_ADDR get_probe_address (struct probe *probe,
+ 				    struct objfile *objfile);
+ 
+-/* Return the argument count of the specified probe.  */
++/* Return the argument count of the specified probe.
++
++   This function can throw an exception.  */
+ 
+ extern unsigned get_probe_argument_count (struct probe *probe,
+ 					  struct frame_info *frame);
+@@ -276,7 +282,9 @@ extern unsigned get_probe_argument_count
+ extern int can_evaluate_probe_arguments (struct probe *probe);
+ 
+ /* Evaluate argument N of the specified probe.  N must be between 0
+-   inclusive and get_probe_argument_count exclusive.  */
++   inclusive and get_probe_argument_count exclusive.
++
++   This function can throw an exception.  */
+ 
+ extern struct value *evaluate_probe_argument (struct probe *probe,
+ 					      unsigned n,
+Index: gdb-7.10/gdb/stap-probe.c
+===================================================================
+--- gdb-7.10.orig/gdb/stap-probe.c
++++ gdb-7.10/gdb/stap-probe.c
+@@ -313,9 +313,8 @@ stap_get_opcode (const char **s)
+       break;
+ 
+     default:
+-      internal_error (__FILE__, __LINE__,
+-		      _("Invalid opcode in expression `%s' for SystemTap"
+-			"probe"), *s);
++      error (_("Invalid opcode in expression `%s' for SystemTap"
++	       "probe"), *s);
+     }
+ 
+   return op;
+@@ -326,7 +325,8 @@ stap_get_opcode (const char **s)
+ 
+ static struct type *
+ stap_get_expected_argument_type (struct gdbarch *gdbarch,
+-				 enum stap_arg_bitness b)
++				 enum stap_arg_bitness b,
++				 const struct stap_probe *probe)
+ {
+   switch (b)
+     {
+@@ -361,8 +361,8 @@ stap_get_expected_argument_type (struct
+       return builtin_type (gdbarch)->builtin_uint64;
+ 
+     default:
+-      internal_error (__FILE__, __LINE__,
+-		      _("Undefined bitness for probe."));
++      error (_("Undefined bitness for probe '%s'."),
++	     probe->p.name);
+       break;
+     }
+ }
+@@ -1172,7 +1172,8 @@ stap_parse_probe_arguments (struct stap_
+       else
+ 	arg.bitness = STAP_ARG_BITNESS_UNDEFINED;
+ 
+-      arg.atype = stap_get_expected_argument_type (gdbarch, arg.bitness);
++      arg.atype = stap_get_expected_argument_type (gdbarch, arg.bitness,
++						   probe);
+ 
+       expr = stap_parse_argument (&cur, arg.atype, gdbarch);
+ 
+@@ -1278,12 +1279,26 @@ stap_is_operator (const char *op)
+   return ret;
+ }
+ 
++/* Return argument N of probe PROBE.
++
++   If the probe's arguments have not been parsed yet, parse them.  If
++   there are no arguments, throw an exception (error).  Otherwise,
++   return the requested argument.  */
++
+ static struct stap_probe_arg *
+ stap_get_arg (struct stap_probe *probe, unsigned n, struct gdbarch *gdbarch)
+ {
+   if (!probe->args_parsed)
+     stap_parse_probe_arguments (probe, gdbarch);
+ 
++  gdb_assert (probe->args_parsed);
++  if (probe->args_u.vec == NULL)
++    internal_error (__FILE__, __LINE__,
++		    _("Probe '%s' apparently does not have arguments, but \n"
++		      "GDB is requesting its argument number %u anyway.  "
++		      "This should not happen.  Please report this bug."),
++		    probe->p.name, n);
++
+   return VEC_index (stap_probe_arg_s, probe->args_u.vec, n);
+ }
+ 

diff --git a/gdb-probes-based-interface-robust-2of2.patch b/gdb-probes-based-interface-robust-2of2.patch
new file mode 100644
index 0000000..a9a022c
--- /dev/null
+++ b/gdb-probes-based-interface-robust-2of2.patch
@@ -0,0 +1,194 @@
+From 3bd7e5b7ee5ea0b3bbb4030ca841f66faad74f0f Mon Sep 17 00:00:00 2001
+From: Sergio Durigan Junior <sergiodj@redhat.com>
+Date: Fri, 21 Aug 2015 18:28:07 -0400
+Subject: [PATCH 2/4] 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.
+---
+ gdb/ChangeLog    |  6 ++++++
+ gdb/solib-svr4.c | 43 ++++++++++++++++++++++++++++++++++++++++---
+ 2 files changed, 46 insertions(+), 3 deletions(-)
+
+Index: gdb-7.10/gdb/solib-svr4.c
+===================================================================
+--- gdb-7.10.orig/gdb/solib-svr4.c
++++ gdb-7.10/gdb/solib-svr4.c
+@@ -1796,7 +1796,23 @@ solib_event_probe_action (struct probe_a
+        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)
+@@ -1950,7 +1966,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);
+@@ -1981,7 +2007,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);
+ 
+From ad1c917a79e8c5aa67657f148415c1bee01b240f Mon Sep 17 00:00:00 2001
+From: Sergio Durigan Junior <sergiodj@redhat.com>
+Date: Wed, 2 Sep 2015 00:34:22 -0400
+Subject: [PATCH 3/4] Initialize variable and silence GCC warning from last
+ commit
+
+BuildBot e-mailed me to let me know that my last commit broke GDB on
+RHEL-7.1 s390x.  On solib-svr4.c:svr4_handle_solib_event, 'val' now
+needs to be initialized as NULL because it is inside a TRY..CATCH
+block.  This patch does that.  Pushed as obvious.
+
+gdb/ChangeLog:
+2015-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+	* solib-svr4.c (svr4_handle_solib_event): Initialize 'val' as NULL
+---
+ gdb/ChangeLog    | 4 ++++
+ gdb/solib-svr4.c | 2 +-
+ 2 files changed, 5 insertions(+), 1 deletion(-)
+
+Index: gdb-7.10/gdb/solib-svr4.c
+===================================================================
+--- gdb-7.10.orig/gdb/solib-svr4.c
++++ gdb-7.10/gdb/solib-svr4.c
+@@ -1918,7 +1918,7 @@ svr4_handle_solib_event (void)
+   struct probe_and_action *pa;
+   enum probe_action action;
+   struct cleanup *old_chain, *usm_chain;
+-  struct value *val;
++  struct value *val = NULL;
+   CORE_ADDR pc, debug_base, lm = 0;
+   int is_initial_ns;
+   struct frame_info *frame = get_current_frame ();
+From 73c6b4756a7cee53c274ed05fddcd079b8b7e57c Mon Sep 17 00:00:00 2001
+From: Sergio Durigan Junior <sergiodj@redhat.com>
+Date: Wed, 2 Sep 2015 00:46:43 -0400
+Subject: [PATCH 4/4] Initialize yet another variable to silence GCC warning
+ from last-but-one commit
+
+Yet another BuildBot e-mail, yet another breakage on RHEL-7.1 s390x
+(which uses an older GCC).  This time,
+solib-svr4.c:solib_event_probe_action has the probe_argc variable,
+which is now inside a TRY..CATCH and therefore needs to be
+initialized.  Pushed as obvious.
+
+gdb/ChangeLog:
+2015-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+	* solib-svr4.c (solib_event_probe_action): Initialize 'probe_argc'
+	as zero.
+---
+ gdb/ChangeLog    | 5 +++++
+ gdb/solib-svr4.c | 2 +-
+ 2 files changed, 6 insertions(+), 1 deletion(-)
+
+ 2015-09-01  Sergio Durigan Junior  <sergiodj@redhat.com>
+Index: gdb-7.10/gdb/solib-svr4.c
+===================================================================
+--- gdb-7.10.orig/gdb/solib-svr4.c
++++ gdb-7.10/gdb/solib-svr4.c
+@@ -1782,7 +1782,7 @@ static enum probe_action
+ solib_event_probe_action (struct probe_and_action *pa)
+ {
+   enum probe_action action;
+-  unsigned probe_argc;
++  unsigned probe_argc = 0;
+   struct frame_info *frame = get_current_frame ();
+ 
+   action = pa->action;

diff --git a/gdb.spec b/gdb.spec
index 1bcbc63..f419374 100644
--- a/gdb.spec
+++ b/gdb.spec
@@ -26,7 +26,7 @@ Version: 7.10
 
 # The release always contains a leading reserved number, start it at 1.
 # `upstream' is not a part of `name' to stay fully rpm dependencies compatible for the testing.
-Release: 16%{?dist}
+Release: 17%{?dist}
 
 License: GPLv3+ and GPLv3+ with exceptions and GPLv2+ and GPLv2+ with exceptions and GPL+ and LGPLv2+ and BSD and Public Domain and GFDL
 Group: Development/Debuggers
@@ -535,6 +535,11 @@ Patch927: gdb-python-gil.patch
 # Fix jit-reader.h for multi-lib.
 Patch978: gdb-jit-reader-multilib.patch
 
+# Fix 'Make the probes-based dynamic linker interface more robust to
+# errors' (Sergio Durigan Junior, RH BZ 1259132).
+Patch1029: gdb-probes-based-interface-robust-1of2.patch
+Patch1030: gdb-probes-based-interface-robust-2of2.patch
+
 %if 0%{!?rhel:1} || 0%{?rhel} > 6
 # RL_STATE_FEDORA_GDB would not be found for:
 # Patch642: gdb-readline62-ask-more-rh.patch
@@ -821,6 +826,8 @@ find -name "*.info*"|xargs rm -f
 %patch925 -p1
 %patch927 -p1
 %patch978 -p1
+%patch1029 -p1
+%patch1030 -p1
 
 %patch848 -p1
 %if 0%{!?el6:1}
@@ -1325,6 +1332,10 @@ then
 fi
 
 %changelog
+* Wed Sep  2 2015 Sergio Durigan Junior <sergiodj@redhat.com> - 7.10-17.fc23
+- Fix 'Make the probes-based dynamic linker interface more robust to
+  errors' (Sergio Durigan Junior, RH BZ 1259132).
+
 * Tue Sep  1 2015 Jan Kratochvil <jan.kratochvil@redhat.com> - 7.10-16.fc23
 - [RHEL] Fix librpm Recommends compatibility.