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

Sergio Durigan Junior
2017-11-21T16:53:18Z

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