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

  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(+)
  

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.

Cheers,
  

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
  

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,