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

Pedro Alves
2017-11-21T16:42:33Z

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?

[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).]

Thanks,
Pedro Alves