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

Message ID 1511284814-25508-1-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Nov. 21, 2017, 5:20 p.m. UTC
  New in v2:
  - completely different solution/implementation.  avoids selecting a
    frame in the first place.

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 in turn evaluates probe arguments.  That looks like a bug
to me.

Fix that by avoiding selecting a frame if one isn't set yet in
expression evaluation, which is what switches to the frame's language
as side effect.

(There are a couple calls to get_selected_block in the file that
should probably get a similar treatment, but I'm leaving those as is
for now.)

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

	* eval.c (evaluate_subexp_standard): Use
	get_selected_frame_if_set+get_current_frame instead of
	get_selected_frame.
---
 gdb/eval.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)
  

Comments

Sergio Durigan Junior Nov. 21, 2017, 5:29 p.m. UTC | #1
On Tuesday, November 21 2017, Pedro Alves wrote:

> New in v2:
>   - completely different solution/implementation.  avoids selecting a
>     frame in the first place.

For the record: this LGTM.

Thanks,

> 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 in turn evaluates probe arguments.  That looks like a bug
> to me.
>
> Fix that by avoiding selecting a frame if one isn't set yet in
> expression evaluation, which is what switches to the frame's language
> as side effect.
>
> (There are a couple calls to get_selected_block in the file that
> should probably get a similar treatment, but I'm leaving those as is
> for now.)
>
> gdb/testsuite/ChangeLog:
> 2017-11-21  Pedro Alves  <palves@redhat.com>
>
> 	* eval.c (evaluate_subexp_standard): Use
> 	get_selected_frame_if_set+get_current_frame instead of
> 	get_selected_frame.
> ---
>  gdb/eval.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/gdb/eval.c b/gdb/eval.c
> index 14a3e05..b789982 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,11 @@ 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);
> +	/* Avoid get_selected_frame because that selects the frame's
> +	   language as side effect.  */
> +	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 +1384,14 @@ 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));
> +	  {
> +	    /* Avoid get_selected_frame because that selects the
> +	       frame's language as side effect.  */
> +	    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
  

Patch

diff --git a/gdb/eval.c b/gdb/eval.c
index 14a3e05..b789982 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,11 @@  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);
+	/* Avoid get_selected_frame because that selects the frame's
+	   language as side effect.  */
+	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 +1384,14 @@  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));
+	  {
+	    /* Avoid get_selected_frame because that selects the
+	       frame's language as side effect.  */
+	    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