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

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

[Pedro Alves]

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);
-- 
2.5.5

[PATCH 0/2] Fix a couple gdb.ada/minsyms.exp problems

[Pedro Alves]

Hi Joel, all,

The new gdb.ada/minsyms.exp testcase fails for me like this:

 FAIL: gdb.ada/minsyms.exp: print integer(some_minsym)
 FAIL: gdb.ada/minsyms.exp: print &some_minsym
 FAIL: gdb.ada/minsyms.exp: print /x integer(&some_minsym)

There are two problems here.

One's obvious: the testcase hardcodes an expected address for the
"some_minsym" variable, which obviously can't work everywhere.

The other problem is that if you have debug info for glibc, GDB
loses track of the fact that the program is a Ada program, and
switches the current language to C, where Ada's cast syntax doesn't
work:

    print integer(some_minsym)
    A syntax error in expression, near `some_minsym)'.
    (gdb) FAIL: gdb.ada/minsyms.exp: print integer(some_minsym)
    print &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, and is losing that when handling a shared library
event.  That looks like a GDB bug, so I'm proposing fixing GDB
instead.

WDYT?

Pedro Alves (2):
  gdb.ada/minsyms.exp: Don't hardcode the variable's address
  Don't lose language determined from the "main" name (fix
    gdb.ada/minsyms.exp)

 gdb/solib-svr4.c                  | 15 +++++++++++++++
 gdb/testsuite/gdb.ada/minsyms.exp |  2 +-
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.5.5

[PATCH 1/2] gdb.ada/minsyms.exp: Don't hardcode the variable's address

[Pedro Alves]

This new testcase has a test that fails like this here:

  $1 = (<data variable, no debug info> *) 0x60208c <some_minsym>
  (gdb) FAIL: gdb.ada/minsyms.exp: print &some_minsym

The problem is that the testcase hardcodes an expected address for the
"some_minsym" variable, which obviously isn't stable.

Fix that by expecting $hex instead.

gdb/testsuite/ChangeLog:
2017-11-21  Pedro Alves  <palves@redhat.com>

	* gdb.ada/minsyms.exp: Accept any address for 'some_minsym'.
---
 gdb/testsuite/gdb.ada/minsyms.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.ada/minsyms.exp b/gdb/testsuite/gdb.ada/minsyms.exp
index 2c91125..9878f9f 100644
--- a/gdb/testsuite/gdb.ada/minsyms.exp
+++ b/gdb/testsuite/gdb.ada/minsyms.exp
@@ -35,7 +35,7 @@ gdb_test "print integer(some_minsym)" \
          " = 1234"
 
 gdb_test "print &some_minsym" \
-         " = \\(access <data variable, no debug info>\\) 0x62c2f8 <some_minsym>"
+         " = \\(access <data variable, no debug info>\\) $hex <some_minsym>"
 
 gdb_test "print /x integer(&some_minsym)" \
          " = $hex"
-- 
2.5.5

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

[Sergio Durigan Junior]

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,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

Re: [PATCH 1/2] gdb.ada/minsyms.exp: Don't hardcode the variable's address

[Sergio Durigan Junior]

On Tuesday, November 21 2017, Pedro Alves wrote:

> This new testcase has a test that fails like this here:
>
>   $1 = (<data variable, no debug info> *) 0x60208c <some_minsym>
>   (gdb) FAIL: gdb.ada/minsyms.exp: print &some_minsym
>
> The problem is that the testcase hardcodes an expected address for the
> "some_minsym" variable, which obviously isn't stable.
>
> Fix that by expecting $hex instead.

No Ada expert here, but I'd say this is borderline obvious and should go
in :-).

Thanks,

> gdb/testsuite/ChangeLog:
> 2017-11-21  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.ada/minsyms.exp: Accept any address for 'some_minsym'.
> ---
>  gdb/testsuite/gdb.ada/minsyms.exp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/testsuite/gdb.ada/minsyms.exp b/gdb/testsuite/gdb.ada/minsyms.exp
> index 2c91125..9878f9f 100644
> --- a/gdb/testsuite/gdb.ada/minsyms.exp
> +++ b/gdb/testsuite/gdb.ada/minsyms.exp
> @@ -35,7 +35,7 @@ gdb_test "print integer(some_minsym)" \
>           " = 1234"
>  
>  gdb_test "print &some_minsym" \
> -         " = \\(access <data variable, no debug info>\\) 0x62c2f8 <some_minsym>"
> +         " = \\(access <data variable, no debug info>\\) $hex <some_minsym>"
>  
>  gdb_test "print /x integer(&some_minsym)" \
>           " = $hex"
> -- 
> 2.5.5

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

[Pedro Alves]

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

Re: [PATCH 1/2] gdb.ada/minsyms.exp: Don't hardcode the variable's address

[Joel Brobecker]

> This new testcase has a test that fails like this here:
> 
>   $1 = (<data variable, no debug info> *) 0x60208c <some_minsym>
>   (gdb) FAIL: gdb.ada/minsyms.exp: print &some_minsym
> 
> The problem is that the testcase hardcodes an expected address for the
> "some_minsym" variable, which obviously isn't stable.
> 
> Fix that by expecting $hex instead.
> 
> gdb/testsuite/ChangeLog:
> 2017-11-21  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.ada/minsyms.exp: Accept any address for 'some_minsym'.

Gaaah. Head-slapping myself on that one.

Thanks for fixing. Obviously OK :).

-- 
Joel

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

[Sergio Durigan Junior]

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/

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

[Pedro Alves]

On 11/21/2017 04:42 PM, Pedro Alves wrote:
> On 11/21/2017 04:23 PM, Sergio Durigan Junior wrote:

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

Hmm, maybe what we need instead is to make expression evaluation
never set the selected frame (and thus language as side effect)
if it wasn't selected/set already.  Like below.  This fixes
the testcase too.  I'll run the full testsuite now.  WDYT?

From c5fd954787e058abcc72aca329414b0edb40ac1b Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 21 Nov 2017 16:50:32 +0000
Subject: [PATCH] alternative

---
 gdb/eval.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/eval.c b/gdb/eval.c
index 14a3e05..086ac59 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1319,7 +1319,6 @@ evaluate_subexp_standard (struct type *expect_type,
 
       {
 	struct symbol *sym = exp->elts[pc + 1].symbol;
-	struct frame_info *frame;
 
 	if (noside == EVAL_AVOID_SIDE_EFFECTS)
 	  return value_zero (SYMBOL_TYPE (sym), not_lval);
@@ -1329,7 +1328,9 @@ evaluate_subexp_standard (struct type *expect_type,
 	  error (_("Symbol \"%s\" does not have any specific entry value"),
 		 SYMBOL_PRINT_NAME (sym));
 
-	frame = get_selected_frame (NULL);
+	frame_info *frame = get_selected_frame_if_set ();
+	if (frame == NULL)
+	  frame = get_current_frame ();
 	return SYMBOL_COMPUTED_OPS (sym)->read_variable_at_entry (sym, frame);
       }
 
@@ -1381,7 +1382,12 @@ evaluate_subexp_standard (struct type *expect_type,
 			+ gdbarch_num_pseudo_regs (exp->gdbarch))
 	  val = value_zero (register_type (exp->gdbarch, regno), not_lval);
 	else
-	  val = value_of_register (regno, get_selected_frame (NULL));
+	  {
+	    frame_info *frame = get_selected_frame_if_set ();
+	    if (frame == NULL)
+	      frame = get_current_frame ();
+	    val = value_of_register (regno, frame);
+	  }
 	if (val == NULL)
 	  error (_("Value of register %s not available."), name);
 	else
-- 
2.5.5

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

[Pedro Alves]

On 11/21/2017 04:56 PM, Pedro Alves wrote:
> On 11/21/2017 04:42 PM, Pedro Alves wrote:
>> On 11/21/2017 04:23 PM, Sergio Durigan Junior wrote:
> 
>>> 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).]
> 
> Hmm, maybe what we need instead is to make expression evaluation
> never set the selected frame (and thus language as side effect)
> if it wasn't selected/set already.  Like below.  This fixes
> the testcase too.  I'll run the full testsuite now.  WDYT?

Full testsuite run complete; no regressions.

Thanks,
Pedro Alves

Re: [PATCH 1/2] gdb.ada/minsyms.exp: Don't hardcode the variable's address

[Pedro Alves]

On 11/21/2017 04:50 PM, Joel Brobecker wrote:
>> This new testcase has a test that fails like this here:
>>
>>   $1 = (<data variable, no debug info> *) 0x60208c <some_minsym>
>>   (gdb) FAIL: gdb.ada/minsyms.exp: print &some_minsym
>>
>> The problem is that the testcase hardcodes an expected address for the
>> "some_minsym" variable, which obviously isn't stable.
>>
>> Fix that by expecting $hex instead.
>>
>> gdb/testsuite/ChangeLog:
>> 2017-11-21  Pedro Alves  <palves@redhat.com>
>>
>> 	* gdb.ada/minsyms.exp: Accept any address for 'some_minsym'.
> 
> Gaaah. Head-slapping myself on that one.
> 
> Thanks for fixing. Obviously OK :).

:-)

I pushed this one in, to get it out of the way.

Thanks,
Pedro Alves

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

[Sergio Durigan Junior]

On Tuesday, November 21 2017, Pedro Alves wrote:

> On 11/21/2017 04:42 PM, Pedro Alves wrote:
>> On 11/21/2017 04:23 PM, Sergio Durigan Junior wrote:
>
>>> 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).]
>
> Hmm, maybe what we need instead is to make expression evaluation
> never set the selected frame (and thus language as side effect)
> if it wasn't selected/set already.  Like below.  This fixes
> the testcase too.  I'll run the full testsuite now.  WDYT?

That does look better, indeed.  I was trying to think if we'd encounter
any situation where setting the language is on of the desired effects,
but couldn't think of any.

Thanks,

> From c5fd954787e058abcc72aca329414b0edb40ac1b Mon Sep 17 00:00:00 2001
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 21 Nov 2017 16:50:32 +0000
> Subject: [PATCH] alternative
>
> ---
>  gdb/eval.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 14a3e05..086ac59 100644
> --- a/gdb/eval.c
> +++ b/gdb/eval.c
> @@ -1319,7 +1319,6 @@ evaluate_subexp_standard (struct type *expect_type,
>  
>        {
>  	struct symbol *sym = exp->elts[pc + 1].symbol;
> -	struct frame_info *frame;
>  
>  	if (noside == EVAL_AVOID_SIDE_EFFECTS)
>  	  return value_zero (SYMBOL_TYPE (sym), not_lval);
> @@ -1329,7 +1328,9 @@ evaluate_subexp_standard (struct type *expect_type,
>  	  error (_("Symbol \"%s\" does not have any specific entry value"),
>  		 SYMBOL_PRINT_NAME (sym));
>  
> -	frame = get_selected_frame (NULL);
> +	frame_info *frame = get_selected_frame_if_set ();
> +	if (frame == NULL)
> +	  frame = get_current_frame ();
>  	return SYMBOL_COMPUTED_OPS (sym)->read_variable_at_entry (sym, frame);
>        }
>  
> @@ -1381,7 +1382,12 @@ evaluate_subexp_standard (struct type *expect_type,
>  			+ gdbarch_num_pseudo_regs (exp->gdbarch))
>  	  val = value_zero (register_type (exp->gdbarch, regno), not_lval);
>  	else
> -	  val = value_of_register (regno, get_selected_frame (NULL));
> +	  {
> +	    frame_info *frame = get_selected_frame_if_set ();
> +	    if (frame == NULL)
> +	      frame = get_current_frame ();
> +	    val = value_of_register (regno, frame);
> +	  }
>  	if (val == NULL)
>  	  error (_("Value of register %s not available."), name);
>  	else
> -- 
> 2.5.5

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

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

[Pedro Alves]

n 11/21/2017 05:15 PM, Sergio Durigan Junior wrote:
> On Tuesday, November 21 2017, Pedro Alves wrote:
> 
>> On 11/21/2017 04:42 PM, Pedro Alves wrote:
>>> On 11/21/2017 04:23 PM, Sergio Durigan Junior wrote:
>>
>>>> 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).]
>>
>> Hmm, maybe what we need instead is to make expression evaluation
>> never set the selected frame (and thus language as side effect)
>> if it wasn't selected/set already.  Like below.  This fixes
>> the testcase too.  I'll run the full testsuite now.  WDYT?
> 
> That does look better, indeed.  I was trying to think if we'd encounter
> any situation where setting the language is on of the desired effects,
> but couldn't think of any.

Alright, I've sent a v2 now.

Thanks,
Pedro Alves