[2/2] Catching errors on probes-based dynamic linker interface

Message ID 8737yx78u5.fsf@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Sept. 2, 2015, 4:18 a.m. UTC
  On Tuesday, September 01 2015, I wrote:

>> I am ok with doing this:
>>
>>   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 you put a big fat comment above the following block, e.g.:
>>
>>   /* Note that failure of get_probe_argument_count will
>>      set probe_argc == 0.  This must result in returning
>>      action = PROBES_INTERFACE_FAILED.  */
>>   if (probe_argc == 2)
>>     action = FULL_RELOAD;
>>   else if (probe_argc < 2)
>>     action = PROBES_INTERFACE_FAILED;
>
> Great, that works for me as well.  I will update the patch here to
> address this.

I took the liberty to modify and expand the comment; I hope you still
find it OK.  Here's what I pushed.

Thanks,
  

Comments

Sergio Durigan Junior Sept. 2, 2015, 4:21 a.m. UTC | #1
On Wednesday, September 02 2015, I wrote:

> On Tuesday, September 01 2015, I wrote:
>
>>> I am ok with doing this:
>>>
>>>   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 you put a big fat comment above the following block, e.g.:
>>>
>>>   /* Note that failure of get_probe_argument_count will
>>>      set probe_argc == 0.  This must result in returning
>>>      action = PROBES_INTERFACE_FAILED.  */
>>>   if (probe_argc == 2)
>>>     action = FULL_RELOAD;
>>>   else if (probe_argc < 2)
>>>     action = PROBES_INTERFACE_FAILED;
>>
>> Great, that works for me as well.  I will update the patch here to
>> address this.
>
> I took the liberty to modify and expand the comment; I hope you still
> find it OK.  Here's what I pushed.

Pushed.

  https://sourceware.org/ml/gdb-cvs/2015-09/msg00002.html
  

Patch

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);