Re: [PATCH 2/2] Don't lose language determined from the "main" name (fix gdb.ada/minsyms.exp)

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

On Tuesday, November 21 2017, Pedro Alves wrote:

> On 11/21/2017 04:23 PM, Sergio Durigan Junior wrote:
>> On Tuesday, November 21 2017, Pedro Alves wrote:
>> 
>>> gdb.ada/minsyms.exp fails like this here:
>>>
>>>  FAIL: gdb.ada/minsyms.exp: print integer(some_minsym)
>>>  FAIL: gdb.ada/minsyms.exp: print /x integer(&some_minsym)
>>>
>>> The problem is that if you have debug info for glibc, GDB switches the
>>> current language to C before it reaches the program's entry point, and
>>> then Ada's cast syntax doesn't work when the current language is C:
>>>
>>>   print integer(some_minsym)
>>>   A syntax error in expression, near `some_minsym)'.
>>>   (gdb) FAIL: gdb.ada/minsyms.exp: print integer(some_minsym)
>>>
>>> I first thought of doing "set language ada" in the testcase, but
>>> looking deeper, I realized that before running to main, GDB knows the
>>> program is Ada, determined by reading __gnat_ada_main_program_name,
>>> via set_initial_language->main_language->find_main_name->
>>> ada_main_name, and loses that when it is handling a shared library
>>> event.  That looks like a bug to me.
>>>
>>> gdb/testsuite/ChangeLog:
>>> 2017-11-21  Pedro Alves  <palves@redhat.com>
>>>
>>> 	* solib-svr4.c: Save/restore language around evaluating a probe
>>> 	argument.
>>> ---
>>>  gdb/solib-svr4.c | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
>>> index 5ec606d..e6f818a 100644
>>> --- a/gdb/solib-svr4.c
>>> +++ b/gdb/solib-svr4.c
>>> @@ -1936,6 +1936,21 @@ svr4_handle_solib_event (void)
>>>    usm_chain = make_cleanup (resume_section_map_updates_cleanup,
>>>  			    current_program_space);
>>>  
>>> +  /* Make sure evaluating probe arguments doesn't cause us to switch
>>> +     the user's current language to the runtime's language.
>>> +     Evaluating probe arguments relies on reading registers off the
>>> +     selected frame.  When we're handling a shared library event, this
>>> +     is going to be the first time we fetch the selected frame (as
>>> +     opposed to the current frame), and if there's debug info for the
>>> +     loader (e.g., glibc), this switches to its language (usually C).
>>> +     Usually that ends up masked because we will usually next stop in
>>> +     the main program (e.g., user did "start"), and switch to the
>>> +     right language again then, if the program has debug info.
>>> +     However, if the program does not have debug info, then GDB won't
>>> +     switch, and we'd lose the language that was determined earlier by
>>> +     sniffing the program's main name.  */
>>> +  scoped_restore_current_language save_language;
>>> +
>>>    TRY
>>>      {
>>>        val = evaluate_probe_argument (pa->probe, 1, frame);
>> 
>> Hey, thanks for the patch.
>> 
>> Since this is guaranteed to be an stap probe, WDYT about moving this
>> scoped_restore_current_language to
>> stap-probe.c:stap_evaluate_probe_argument?  This way we won't be bit by
>> this problem in other parts that also evaluate arguments of probes.
>> 
>> Arguably, this should be set for every probe type IMHO, but it's fine if
>> we just do it for stap probes for now.
>
> That sounds like a good idea.  But we could do it in 
> evaluate_probe_argument then, which handles all probe types?

Yes, I was going to suggest that, but I thought about my C++-ification
patch, as you imagined.

> [In your probe C++ification, that translates to evaluate_probe_argument
> becoming a  non-virtual method of probe, which then calls into a
> protected virtual method that is overridden by the actual probe
> implementation (see e.g., the do_xxx methods of class ui_out).]

Great, I will do that in my local tree.

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/