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

[Pedro Alves <palves@redhat.com>]

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