Better support for Ada renamings in "info locals" output

Better support for Ada renamings in "info locals" output

[Joel Brobecker]

Hello,

The meat of this patch series is in the first patch, but I needed an
unrelated patch to go in at the same time (patch #2). And patch #3 is
the testcase, once patch #1 is in.

Note: Patch #2, being independent, has been checked in on its own.

Thanks,
-- 
Joel

[PATCH 3/3] Testcase: "info locals" with Ada renamings.

[Joel Brobecker]

gdb/testsuite/ChangeLog:

        * gdb.ada/info_locals_renaming: New testcase.

Tested on x86_64-linux. Will commit if patch #1 or an equilvalent
makes it.

---
 gdb/testsuite/gdb.ada/info_locals_renaming.exp     |   36 ++++++++++++++++++++
 gdb/testsuite/gdb.ada/info_locals_renaming/foo.adb |   22 ++++++++++++
 gdb/testsuite/gdb.ada/info_locals_renaming/pck.adb |   21 +++++++++++
 gdb/testsuite/gdb.ada/info_locals_renaming/pck.ads |   20 +++++++++++
 4 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 gdb/testsuite/gdb.ada/info_locals_renaming.exp
 create mode 100644 gdb/testsuite/gdb.ada/info_locals_renaming/foo.adb
 create mode 100644 gdb/testsuite/gdb.ada/info_locals_renaming/pck.adb
 create mode 100644 gdb/testsuite/gdb.ada/info_locals_renaming/pck.ads

diff --git a/gdb/testsuite/gdb.ada/info_locals_renaming.exp b/gdb/testsuite/gdb.ada/info_locals_renaming.exp
new file mode 100644
index 0000000..a8c6469
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/info_locals_renaming.exp
@@ -0,0 +1,36 @@
+# Copyright 2012 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib "ada.exp"
+
+if { [skip_ada_tests] } { return -1 }
+
+set testdir "info_locals_renaming"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable {debug}] != ""} {
+    return -1
+}
+
+clean_restart ${testfile}
+
+set bp_location [gdb_get_line_number "STOP" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+gdb_test "info locals" "gv = 1"
+
diff --git a/gdb/testsuite/gdb.ada/info_locals_renaming/foo.adb b/gdb/testsuite/gdb.ada/info_locals_renaming/foo.adb
new file mode 100644
index 0000000..8f29709
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/info_locals_renaming/foo.adb
@@ -0,0 +1,22 @@
+--  Copyright 2012 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+with Pck; use Pck;
+
+procedure Foo is
+   GV : Integer renames Pck.Global_Variable;
+begin
+   Increment (GV); -- STOP
+end Foo;
diff --git a/gdb/testsuite/gdb.ada/info_locals_renaming/pck.adb b/gdb/testsuite/gdb.ada/info_locals_renaming/pck.adb
new file mode 100644
index 0000000..e69386d
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/info_locals_renaming/pck.adb
@@ -0,0 +1,21 @@
+--  Copyright 2012 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package body Pck is
+   procedure Increment (I : in out Integer) is
+   begin
+      I := I + 1;
+   end Increment;
+end Pck;
diff --git a/gdb/testsuite/gdb.ada/info_locals_renaming/pck.ads b/gdb/testsuite/gdb.ada/info_locals_renaming/pck.ads
new file mode 100644
index 0000000..c795c4b
--- /dev/null
+++ b/gdb/testsuite/gdb.ada/info_locals_renaming/pck.ads
@@ -0,0 +1,20 @@
+--  Copyright 2012 Free Software Foundation, Inc.
+--
+--  This program is free software; you can redistribute it and/or modify
+--  it under the terms of the GNU General Public License as published by
+--  the Free Software Foundation; either version 3 of the License, or
+--  (at your option) any later version.
+--
+--  This program is distributed in the hope that it will be useful,
+--  but WITHOUT ANY WARRANTY; without even the implied warranty of
+--  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+--  GNU General Public License for more details.
+--
+--  You should have received a copy of the GNU General Public License
+--  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package Pck is
+   Global_Variable : Integer := 1;
+
+   procedure Increment (I : in out Integer);
+end Pck;
-- 
1.7.1

Re: [PATCH 3/3] Testcase: "info locals" with Ada renamings.

[Joel Brobecker]

> gdb/testsuite/ChangeLog:
> 
>         * gdb.ada/info_locals_renaming: New testcase.

And checked in as well...

-- 
Joel

[RFA 1/3] language-specific read_var_value for Ada renamings

[Joel Brobecker]

The purpose of this patch is to better support renamings in the
"info locals" command. Consider ...

    procedure Foo is
       GV : Integer renames Pck.Global_Variable;
    begin
       Increment (GV); -- STOP
    end Foo;

... Pck.Global_Variable is just an integer. After having stopped at
the "STOP" line, "info locals" yields:

    (gdb) info locals
    gv = <error reading variable gv (Cannot access memory at address 0xffffffffffffffff)>

In reality, two things are happening:

   (1) Variable "GV" does not exist, which is normal, since there is
       "GV" the renaming of another variable;

   (2) But to allow the user access to that renaming the same way
       the code has, the compiler produces an artificial variable
       whose name encodes the renaming:

        gv___XR_pck__global_variable___XE

       For practical reasons, the artificial variable itself is given
       irrelevant types and addresses.

If the user did "print gv", then GDB knows to find
gv___XR_pck__global_variable___XE, how to decode it and print its value.

But The "info locals" command does not act as if it was a short-cut
of "foreach VAR in locals, print VAR". Instead it gets the value of
each VAR directly, which does not work in this case, since the variable
is artificial and needs to be decoded first.

This patch makes the "read_var_value" routine language-specific.
The old implementation of "read_var_value" gets renamed to
"default_read_var_value" and all languages now use it (unchanged
behavior), except for Ada. In Ada, the new function ada_read_var_value
checks if we have a renaming, and if so, evaluates its value, or else
defers to default_read_var_value.

gdb/ChangeLog:

        * language.h (struct language_defn): New "method" la_read_var_value.
        * findvar.c: #include "language.h".
        (default_read_var_value): Renames read_var_value.  Rewrite
        function description.
        (read_var_value): New function.
        * value.h (default_read_var_value): Add prototype.
        * ada-lang.c (ada_read_renaming_var_value, ada_read_var_value):
        New functions.
        (ada_language_defn): Add entry for la_read_var_value.
        * c-lang.c, d-lang.c, f-lang.c, jv-lang.c, language.c,
        * m2-lang.c, objc-lang.c, opencl-lang.c, p-lang.c: Update
        language_defn structures to add entry for new la_read_var_value
        field.

Tested on x86_64-linux.  OK to commit?

---
 gdb/ada-lang.c    |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 gdb/c-lang.c      |    4 ++++
 gdb/d-lang.c      |    1 +
 gdb/f-lang.c      |    1 +
 gdb/findvar.c     |   22 +++++++++++++++++-----
 gdb/jv-lang.c     |    1 +
 gdb/language.c    |    3 +++
 gdb/language.h    |    9 +++++++++
 gdb/m2-lang.c     |    1 +
 gdb/objc-lang.c   |    1 +
 gdb/opencl-lang.c |    1 +
 gdb/p-lang.c      |    1 +
 gdb/value.h       |    3 +++
 13 files changed, 89 insertions(+), 6 deletions(-)

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index f6f51ec..98833ba 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4044,8 +4044,30 @@ parse_old_style_renaming (struct type *type,
   if (len != NULL)
     *len = suffix - info;
   return kind;
-}  
+}
+
+/* Compute the value of the given RENAMING_SYM, which is expected to
+   be a symbol encoding a renaming expression.  BLOCK is the block
+   used to evaluate the renaming.  */
 
+static struct value *
+ada_read_renaming_var_value (struct symbol *renaming_sym,
+			     struct block *block)
+{
+  char *sym_name;
+  struct expression *expr;
+  struct value *value;
+  struct cleanup *old_chain = NULL;
+
+  sym_name = xstrdup (SYMBOL_LINKAGE_NAME (renaming_sym));
+  old_chain = make_cleanup (xfree, sym_name);
+  expr = parse_exp_1 (&sym_name, block, 0);
+  make_cleanup (free_current_contents, &expr);
+  value = evaluate_expression (expr);
+
+  do_cleanups (old_chain);
+  return value;
+}
 \f
 
                                 /* Evaluation: Function Calls */
@@ -12471,6 +12493,28 @@ ada_get_symbol_name_cmp (const char *lookup_name)
     return compare_names;
 }
 
+/* Implement the "la_read_var_value" language_defn method for Ada.  */
+
+static struct value *
+ada_read_var_value (struct symbol *var, struct frame_info *frame)
+{
+  struct block *frame_block = NULL;
+  struct symbol *renaming_sym = NULL;
+
+  /* The only case where default_read_var_value is not sufficient
+     is when VAR is a renaming...  */
+  if (frame)
+    frame_block = get_frame_block (frame, NULL);
+  if (frame_block)
+    renaming_sym = ada_find_renaming_symbol (var, frame_block);
+  if (renaming_sym != NULL)
+    return ada_read_renaming_var_value (renaming_sym, frame_block);
+
+  /* This is a typical case where we expect the default_read_var_value
+     function to work.  */
+  return default_read_var_value (var, frame);
+}
+
 const struct language_defn ada_language_defn = {
   "ada",                        /* Language name */
   language_ada,
@@ -12491,6 +12535,7 @@ const struct language_defn ada_language_defn = {
   ada_print_typedef,            /* Print a typedef using appropriate syntax */
   ada_val_print,                /* Print a value using appropriate syntax */
   ada_value_print,              /* Print a top-level value */
+  ada_read_var_value,		/* la_read_var_value */
   NULL,                         /* Language specific skip_trampoline */
   NULL,                         /* name_of_this */
   ada_lookup_symbol_nonlocal,   /* Looking up non-local symbols.  */
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 767d13a..28dce8d 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -847,6 +847,7 @@ const struct language_defn c_language_defn =
   c_print_typedef,		/* Print a typedef using appropriate syntax */
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   NULL,				/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
@@ -970,6 +971,7 @@ const struct language_defn cplus_language_defn =
   c_print_typedef,		/* Print a typedef using appropriate syntax */
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   cplus_skip_trampoline,	/* Language specific skip_trampoline */
   "this",                       /* name_of_this */
   cp_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
@@ -1011,6 +1013,7 @@ const struct language_defn asm_language_defn =
   c_print_typedef,		/* Print a typedef using appropriate syntax */
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   NULL,				/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
@@ -1057,6 +1060,7 @@ const struct language_defn minimal_language_defn =
   c_print_typedef,		/* Print a typedef using appropriate syntax */
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   NULL,				/* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index 80504b4..527801b 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -256,6 +256,7 @@ static const struct language_defn d_language_defn =
 				   syntax.  */
   d_val_print,			/* Print a value using appropriate syntax.  */
   c_value_print,		/* Print a top-level value.  */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline.  */
   "this",
   basic_lookup_symbol_nonlocal, 
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 805b143..5f324a8 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -293,6 +293,7 @@ const struct language_defn f_language_defn =
   default_print_typedef,	/* Print a typedef using appropriate syntax */
   f_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* FIXME */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   NULL,                    	/* name_of_this */
   cp_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 79c4221..9009e6f 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -34,6 +34,7 @@
 #include "user-regs.h"
 #include "block.h"
 #include "objfiles.h"
+#include "language.h"
 
 /* Basic byte-swapping routines.  All 'extract' functions return a
    host-format integer from a target-format integer at ADDR which is
@@ -405,13 +406,11 @@ symbol_read_needs_frame (struct symbol *sym)
   return 1;
 }
 
-/* Given a struct symbol for a variable,
-   and a stack frame id, read the value of the variable
-   and return a (pointer to a) struct value containing the value.
-   If the variable cannot be found, throw error.  */
+/* A default implementation for the "la_read_var_value" hook in
+   the language vector which should work in most situations.  */
 
 struct value *
-read_var_value (struct symbol *var, struct frame_info *frame)
+default_read_var_value (struct symbol *var, struct frame_info *frame)
 {
   struct value *v;
   struct type *type = SYMBOL_TYPE (var);
@@ -594,6 +593,19 @@ read_var_value (struct symbol *var, struct frame_info *frame)
   return v;
 }
 
+/* Calls VAR's language la_read_var_value hook with the given arguments.  */
+
+struct value *
+read_var_value (struct symbol *var, struct frame_info *frame)
+{
+  const struct language_defn *lang = language_def (SYMBOL_LANGUAGE (var));
+
+  gdb_assert (lang != NULL);
+  gdb_assert (lang->la_read_var_value != NULL);
+
+  return lang->la_read_var_value (var, frame);
+}
+
 /* Install default attributes for register values.  */
 
 struct value *
diff --git a/gdb/jv-lang.c b/gdb/jv-lang.c
index 18a7c19..08ecf5f 100644
--- a/gdb/jv-lang.c
+++ b/gdb/jv-lang.c
@@ -1181,6 +1181,7 @@ const struct language_defn java_language_defn =
   default_print_typedef,	/* Print a typedef using appropriate syntax */
   java_val_print,		/* Print a value using appropriate syntax */
   java_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   "this",	                /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/language.c b/gdb/language.c
index 49ba21a..22ce547 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -917,6 +917,7 @@ const struct language_defn unknown_language_defn =
   default_print_typedef,	/* Print a typedef using appropriate syntax */
   unk_lang_val_print,		/* Print a value using appropriate syntax */
   unk_lang_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   unk_lang_trampoline,		/* Language specific skip_trampoline */
   "this",        	    	/* name_of_this */
   basic_lookup_symbol_nonlocal, /* lookup_symbol_nonlocal */
@@ -960,6 +961,7 @@ const struct language_defn auto_language_defn =
   default_print_typedef,	/* Print a typedef using appropriate syntax */
   unk_lang_val_print,		/* Print a value using appropriate syntax */
   unk_lang_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   unk_lang_trampoline,		/* Language specific skip_trampoline */
   "this",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
@@ -1001,6 +1003,7 @@ const struct language_defn local_language_defn =
   default_print_typedef,	/* Print a typedef using appropriate syntax */
   unk_lang_val_print,		/* Print a value using appropriate syntax */
   unk_lang_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   unk_lang_trampoline,		/* Language specific skip_trampoline */
   "this", 		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/language.h b/gdb/language.h
index a47a44d..1b445b2 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -252,6 +252,15 @@ struct language_defn
     int (*la_value_print) (struct value *, struct ui_file *,
 			   const struct value_print_options *);
 
+    /* Given a symbol VAR, and a stack frame id FRAME, read the value
+       of the variable an return (pointer to a) struct value containing
+       the value.
+
+       Throw an error if the variable cannot be found.  */
+
+    struct value *(*la_read_var_value) (struct symbol *var,
+					struct frame_info *frame);
+
     /* PC is possibly an unknown languages trampoline.
        If that PC falls in a trampoline belonging to this language,
        return the address of the first pc in the real function, or 0
diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
index 9cd27c9..8faa6c1 100644
--- a/gdb/m2-lang.c
+++ b/gdb/m2-lang.c
@@ -385,6 +385,7 @@ const struct language_defn m2_language_defn =
   m2_print_typedef,		/* Print a typedef using appropriate syntax */
   m2_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   NULL,		                /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index 76b97ef..c4df6ca 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -524,6 +524,7 @@ const struct language_defn objc_language_defn = {
   c_print_typedef,		/* Print a typedef using appropriate syntax */
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   objc_skip_trampoline, 	/* Language specific skip_trampoline */
   "self",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 80f978c..47ba03a 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -1008,6 +1008,7 @@ const struct language_defn opencl_language_defn =
   c_print_typedef,		/* Print a typedef using appropriate syntax */
   c_val_print,			/* Print a value using appropriate syntax */
   c_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   NULL,                         /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/p-lang.c b/gdb/p-lang.c
index 826d24f..c59ba0b 100644
--- a/gdb/p-lang.c
+++ b/gdb/p-lang.c
@@ -444,6 +444,7 @@ const struct language_defn pascal_language_defn =
   pascal_print_typedef,		/* Print a typedef using appropriate syntax */
   pascal_val_print,		/* Print a value using appropriate syntax */
   pascal_value_print,		/* Print a top-level value */
+  default_read_var_value,	/* la_read_var_value */
   NULL,				/* Language specific skip_trampoline */
   "this",		        /* name_of_this */
   basic_lookup_symbol_nonlocal,	/* lookup_symbol_nonlocal */
diff --git a/gdb/value.h b/gdb/value.h
index c173b0e..d77769f 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -537,6 +537,9 @@ extern int symbol_read_needs_frame (struct symbol *);
 extern struct value *read_var_value (struct symbol *var,
 				     struct frame_info *frame);
 
+extern struct value *default_read_var_value (struct symbol *var,
+					     struct frame_info *frame);
+
 extern struct value *allocate_value (struct type *type);
 extern struct value *allocate_value_lazy (struct type *type);
 extern void allocate_value_contents (struct value *value);
-- 
1.7.1

Re: [RFA 1/3] language-specific read_var_value for Ada renamings

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel>     procedure Foo is
Joel>        GV : Integer renames Pck.Global_Variable;
Joel>     begin
Joel>        Increment (GV); -- STOP
Joel>     end Foo;

Joel>    (1) Variable "GV" does not exist, which is normal, since there is
Joel>        "GV" the renaming of another variable;

Joel>    (2) But to allow the user access to that renaming the same way
Joel>        the code has, the compiler produces an artificial variable
Joel>        whose name encodes the renaming:

Joel>         gv___XR_pck__global_variable___XE

Joel>        For practical reasons, the artificial variable itself is given
Joel>        irrelevant types and addresses.

I think the patch is fine, given that we've already gone pretty far down
this road.

I wonder whether it would be possible to deal with the renaming when
reading symbols.  If it is possible then I think it would be a better
design -- it would let us pretend that all these magic symbols and
whatnot are really just a facet of debuginfo (which, I think we agree,
they really should have been) -- and it would eliminate a number of
virtual methods throughout the core of gdb.  This in turn is a plus
because each virtual method adds to the conceptual load of gdb
maintenance.

Tom

Re: [RFA 1/3] language-specific read_var_value for Ada renamings

[Joel Brobecker]

> I think the patch is fine, given that we've already gone pretty far down
> this road.

Awesome, thanks for looking at the patch.

> I wonder whether it would be possible to deal with the renaming when
> reading symbols. [...]

I have thought about this many times in the past. But I am not sure how
to do this. At least some of them seem really hard to do. For instance,
this issue with renamings: It would seem to me that the way to go would
be to translate the textual expression from the symbol name into some
kind of location expression. A little like the DWARF locations, but with
possible symbol lookups, array subscripting, taking record components,
etc.

In this case, I'd rather continue pushing for standard DWARF, and just
get rid of the encoding altogether...

-- 
Joel

Re: [RFA 1/3] language-specific read_var_value for Ada renamings

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> In this case, I'd rather continue pushing for standard DWARF, and just
Joel> get rid of the encoding altogether...

Yeah, that would be clearly better.
And at least in the case arising in this thread, simple to do as well.

Tom

Re: [RFA 1/3] language-specific read_var_value for Ada renamings

[Joel Brobecker]

> gdb/ChangeLog:
> 
>         * language.h (struct language_defn): New "method" la_read_var_value.
>         * findvar.c: #include "language.h".
>         (default_read_var_value): Renames read_var_value.  Rewrite
>         function description.
>         (read_var_value): New function.
>         * value.h (default_read_var_value): Add prototype.
>         * ada-lang.c (ada_read_renaming_var_value, ada_read_var_value):
>         New functions.
>         (ada_language_defn): Add entry for la_read_var_value.
>         * c-lang.c, d-lang.c, f-lang.c, jv-lang.c, language.c,
>         * m2-lang.c, objc-lang.c, opencl-lang.c, p-lang.c: Update
>         language_defn structures to add entry for new la_read_var_value
>         field.

Now checked in.


-- 
Joel

[commit/Ada 2/3] processId: Do not modify already encoded IDs

[Joel Brobecker]

The processID function is supposed to take a symbol name, and process it
in a way that allows us to look that symbol up.  This patch is adding
a guard to make sure that we do not apply any transformation if we detect
that we are given an already-encoded symbol name.  For instance:

    gv___XR_pck__global_variable___XE

This happens in the case where we are trying to print the value of
a renaming. To do this, we simply parse and evaluate the XR symbol
name as an expression. Without this change, the expression parser
transforms gv___XR_pck__global_variable___XE into somethink like
gv___xr_pck__global_variable___xe, which then screws up the rest
of the renaming evaluation.

gdb/ChangeLog:

        * ada-lex.p (processId): Do not modify already encoded IDs.
        Update function documentation.

Tested on x86_64-linux, checked in.

---
 gdb/ChangeLog |    5 +++++
 gdb/ada-lex.l |   15 +++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c6a6d8f..621da5b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2012-02-29  Joel Brobecker  <brobecker@adacore.com>
 
+	* ada-lex.p (processId): Do not modify already encoded IDs.
+	Update function documentation.
+
+2012-02-29  Joel Brobecker  <brobecker@adacore.com>
+
 	* ada-lang.h (ada_find_renaming_symbol): Replace parameter
 	"name" with "struct symbol *name_sym".
 	* ada-exp.y (write_var_or_type): Update call to
diff --git a/gdb/ada-lex.l b/gdb/ada-lex.l
index 48667d0..5102ff4 100644
--- a/gdb/ada-lex.l
+++ b/gdb/ada-lex.l
@@ -410,7 +410,9 @@ processReal (const char *num0)
 
 
 /* Store a canonicalized version of NAME0[0..LEN-1] in yylval.ssym.  The
-   resulting string is valid until the next call to ada_parse.  It differs
+   resulting string is valid until the next call to ada_parse.  If
+   NAME0 contains the substring "___", it is assumed to be already
+   encoded and the resulting name is equal to it.  Otherwise, it differs
    from NAME0 in that:
     + Characters between '...' or <...> are transfered verbatim to 
       yylval.ssym.
@@ -430,8 +432,18 @@ processId (const char *name0, int len)
   int i0, i;
   struct stoken result;
 
+  result.ptr = name;
   while (len > 0 && isspace (name0[len-1]))
     len -= 1;
+
+  if (strstr (name0, "___") != NULL)
+    {
+      strncpy (name, name0, len);
+      name[len] = '\000';
+      result.length = len;
+      return result;
+    }
+
   i = i0 = 0;
   while (i0 < len)
     {
@@ -471,7 +483,6 @@ processId (const char *name0, int len)
     }
   name[i] = '\000';
 
-  result.ptr = name;
   result.length = i;
   return result;
 }
-- 
1.7.1