Re: [PATCH v2] Sanitize access to gdbarch on the probe API (and fix ARM bug)

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Tuesday, December 10 2013, Pedro Alves wrote:

> On 12/10/2013 04:04 AM, Sergio Durigan Junior wrote:
>> --- a/gdb/break-catch-throw.c
>> +++ b/gdb/break-catch-throw.c
>> @@ -104,7 +104,7 @@ struct exception_catchpoint
>>  static void
>>  fetch_probe_arguments (struct value **arg0, struct value **arg1)
>>  {
>> -  struct frame_info *frame = get_selected_frame (_("No frame selected"));
>> +  struct frame_info *frame = get_selected_frame (NULL);
>>    CORE_ADDR pc = get_frame_pc (frame);
>
> The bits that do this change are OK, though I'd have preferred they
> were split to a separate, preparatory patch, and checked in as
> a separate commit, with its own rationale.

Right, I've removed those bits from the patch.

I will probably open another thread or ping you on IRC to discuss the
cleanup of get_selected_frame.

> On 12/10/2013 04:04 AM, Sergio Durigan Junior wrote:
>> --- a/gdb/solib-svr4.c
>> +++ b/gdb/solib-svr4.c
>> @@ -1651,6 +1651,7 @@ solib_event_probe_action (struct probe_and_action *pa)
>>  {
>>    enum probe_action action;
>>    unsigned probe_argc;
>> +  struct frame_info *frame = get_selected_frame (NULL);
>
> Missed a little detail here before.  Strictly speaking, event handling
> functions always work with the current, innermost frame, as that
> always corresponds to where the program actually stopped.  In
> that sense, writing that out explicitly is more self-documenting:
>
>   struct frame_info *frame = get_current_frame ();

Fixed.

>>  
>>    action = pa->action;
>>    if (action == DO_NOTHING || action == PROBES_INTERFACE_FAILED)
>> @@ -1663,7 +1664,7 @@ 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);
>> +  probe_argc = get_probe_argument_count (pa->probe, frame);
>>    if (probe_argc == 2)
>>      action = FULL_RELOAD;
>>    else if (probe_argc < 2)
>> @@ -1772,6 +1773,7 @@ svr4_handle_solib_event (void)
>>    struct value *val;
>>    CORE_ADDR pc, debug_base, lm = 0;
>>    int is_initial_ns;
>> +  struct frame_info *frame = get_selected_frame (NULL);
>
> Here too.

Fixed.

> Otherwise looks good to me.

Thanks.  Patch checked-in:

         https://sourceware.org/ml/gdb-cvs/2013-12/msg00065.html

-- 
Sergio