* [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case
@ 2021-12-29 11:14 abdul.b.ijaz
2021-12-29 11:14 ` [PATCH v2 1/2] gdb: " abdul.b.ijaz
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: abdul.b.ijaz @ 2021-12-29 11:14 UTC (permalink / raw)
To: gdb-patches
Cc: abdul.b.ijaz, jonah, Tankut.Baris.Aktemur, philippe.waroquiers,
cristian.sandu
Hi All,
For variable shadowing case this V2 patches add annotation "shadowed" with location information to the shadowed variables so that it is easier for end user to identify which variable was shadowed and also rebase patches to current master.
V1 of this series can be found at:
https://sourceware.org/pipermail/gdb-patches/2021-July/181174.html
Changes since V1:
* Update shadowed annonation to print location in "info locals" and mi commands "-stack-list-locals/variables" as per Philippe feedback.
* Update tests according to the above change.
* Rebase patches to current master.
@Philippe can you please review if new patch set fulfills the location information for shadowed variables.
@jonah can you please provide your feedback for this change since second patch will change the response of '-stack-list-locals' and '-stack-list-variables' command.
GDB Testsuite is executed on Ubuntu18.04 OS and there is no regression seen with this change. mi2-amd64-entry-value, inline-locals and entry-values tests were affected with this change so they were updated accordingly.
Best Regards,
Abdul Basit
Abdul Basit Ijaz (2):
gdb: add annotation in 'info locals' command for variables shadowing
case
gdb: add shadowed_loc field in '-stack-list-locals/variables' mi
commands
gdb/doc/gdb.texinfo | 24 +++
gdb/mi/mi-cmd-stack.c | 38 ++++-
gdb/printcmd.c | 15 +-
gdb/stack.c | 41 +++++-
gdb/stack.h | 3 +-
gdb/testsuite/gdb.base/var-shadowing.c | 52 +++++++
gdb/testsuite/gdb.base/var-shadowing.exp | 85 +++++++++++
gdb/testsuite/gdb.mi/mi-var-shadowing.c | 48 ++++++
gdb/testsuite/gdb.mi/mi-var-shadowing.exp | 138 ++++++++++++++++++
gdb/testsuite/gdb.mi/mi2-amd64-entry-value.c | 6 +-
.../gdb.mi/mi2-amd64-entry-value.exp | 23 +--
gdb/testsuite/gdb.opt/inline-locals.c | 2 +-
gdb/testsuite/gdb.opt/inline-locals.exp | 8 +-
gdb/testsuite/gdb.rust/var_reuse.exp | 33 +++++
gdb/testsuite/gdb.rust/var_reuse.rs | 20 +++
gdb/testsuite/gdb.trace/entry-values.exp | 2 +-
gdb/tracepoint.c | 3 +-
gdb/value.h | 3 +-
18 files changed, 513 insertions(+), 31 deletions(-)
create mode 100755 gdb/testsuite/gdb.base/var-shadowing.c
create mode 100755 gdb/testsuite/gdb.base/var-shadowing.exp
create mode 100644 gdb/testsuite/gdb.mi/mi-var-shadowing.c
create mode 100644 gdb/testsuite/gdb.mi/mi-var-shadowing.exp
create mode 100755 gdb/testsuite/gdb.rust/var_reuse.exp
create mode 100755 gdb/testsuite/gdb.rust/var_reuse.rs
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] gdb: add annotation in 'info locals' command for variables shadowing case
2021-12-29 11:14 [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case abdul.b.ijaz
@ 2021-12-29 11:14 ` abdul.b.ijaz
2022-01-05 15:20 ` Andrew Burgess
2021-12-29 11:14 ` [PATCH v2 2/2] gdb: add shadowed_loc field in '-stack-list-locals/variables' mi commands abdul.b.ijaz
2022-01-01 20:14 ` [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case Philippe Waroquiers
2 siblings, 1 reply; 8+ messages in thread
From: abdul.b.ijaz @ 2021-12-29 11:14 UTC (permalink / raw)
To: gdb-patches
Cc: abdul.b.ijaz, jonah, Tankut.Baris.Aktemur, philippe.waroquiers,
cristian.sandu
For C/C++/Fortran languages GDB prints same name variable multiple times in
case of variable shadowing and it is confusing for user to identify which
variable belongs to the current scope. So add '<shadowed: decl at line #>'
annotation after value of super-block shadowed variable.
Suppose we have
1:int x = 42;
2: {
3: int x = 99;
4: x = 99; /* break here */
5: }
The "info locals" command at the "break here" line gives the following
output:
(gdb) info locals
x = 99
x = 42
So far this scenario is only known case for C/C++/Fortran language so
to tackle in this case, when iterating over the stack frame variables it keep
the track of inner block variables to identify shadowed variables. Iteration
of stack frame variables always starts with inner block so gdb now print
<shadowed: decl at line #> for super-block duplicate variables like this:
(gdb) info locals
x = 99
x = 42 <shadowed: decl at line 1>
In case of rust language it is possible to declare variable with same name
many times so in this case just print first instance of variaable.
gdb/ChangeLog:
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
* printcmd.c (print_variable_and_value): Print
<shadowed: decl at line #> after the value of
super-block variable in case of variable shadowing.
* stack.c (iterate_over_block_locals): Detect variable shadowing
using vector to track the inner block variables.
(iterate_over_block_local_vars): Declare unordered_set type object
and pass it to function 'iterate_over_block_locals'.
(do_print_variable_and_value): Add Boolean variable in call back
function to indicate variable shadowing.
* stack.h (iterate_over_block_arg_local_vars_cb) : Ditto.
* tracepoint.c (do_collect_symbol): Ditto.
* value.h (print_variable_and_value): Add Boolean variable in
function declaration to indicate variable shadowing.
gdb/doc/ChangeLog:
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
* gdb.texinfo: Update with shadowed variable line info.
gdb/testsuite/ChangeLog:
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
* gdb.opt/inline-locals.exp: Update regex expressions for testing
'info locals' command output to handle <shadowed: decl at line #>
annotation.
* gdb.opt/inline-locals.c: Add new comments to refer lines.
* gdb.base/var-shadowing.c: New file.
* gdb.base/var-shadowing.exp: New file.
* gdb.rust/var_reuse.exp: New file.
* gdb.rust/var_reuse.rs: New file.
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
---
gdb/doc/gdb.texinfo | 12 ++++
gdb/printcmd.c | 15 ++++-
gdb/stack.c | 41 ++++++++++--
gdb/stack.h | 3 +-
gdb/testsuite/gdb.base/var-shadowing.c | 52 +++++++++++++++
gdb/testsuite/gdb.base/var-shadowing.exp | 85 ++++++++++++++++++++++++
gdb/testsuite/gdb.opt/inline-locals.c | 2 +-
gdb/testsuite/gdb.opt/inline-locals.exp | 8 ++-
gdb/testsuite/gdb.rust/var_reuse.exp | 33 +++++++++
gdb/testsuite/gdb.rust/var_reuse.rs | 20 ++++++
gdb/tracepoint.c | 3 +-
gdb/value.h | 3 +-
12 files changed, 263 insertions(+), 14 deletions(-)
create mode 100755 gdb/testsuite/gdb.base/var-shadowing.c
create mode 100755 gdb/testsuite/gdb.base/var-shadowing.exp
create mode 100755 gdb/testsuite/gdb.rust/var_reuse.exp
create mode 100755 gdb/testsuite/gdb.rust/var_reuse.rs
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 7da6805f80..83994c3486 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -8590,6 +8590,18 @@ The optional flag @samp{-q}, which stands for @samp{quiet}, disables
printing header information and messages explaining why no local variables
have been printed.
+@smallexample
+1: int x = 3;
+2: @{
+3: int x = 4; // breakpt
+4: @}
+(gdb) info locals
+x = 4
+x = 3 <shadowed: decl at line 1>
+@end smallexample
+
+If a variable is shadowed, the declaration line is also printed.
+
@item info locals [-q] [-t @var{type_regexp}] [@var{regexp}]
Like @kbd{info locals}, but only print the local variables selected
with the provided regexp(s).
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index e408b19db6..5aea39370f 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -2387,7 +2387,8 @@ clear_dangling_display_expressions (struct objfile *objfile)
void
print_variable_and_value (const char *name, struct symbol *var,
struct frame_info *frame,
- struct ui_file *stream, int indent)
+ struct ui_file *stream, int indent,
+ bool shadowed)
{
if (!name)
@@ -2410,6 +2411,18 @@ print_variable_and_value (const char *name, struct symbol *var,
opts.deref_ref = 1;
common_val_print_checked (val, stream, indent, &opts, current_language);
+ /* Print <shadowed: decl at line %line> after the variable value only
+ when it is variable shadowing case. */
+ if (shadowed)
+ {
+ if (var->line > 0)
+ fprintf_styled (stream, metadata_style.style (),
+ _("\t<shadowed: decl at line %d>"), var->line);
+ else
+ fprintf_styled (stream, metadata_style.style (),
+ _("\t<shadowed: decl line not available>"));
+ }
+
/* common_val_print invalidates FRAME when a pretty printer calls inferior
function. */
frame = NULL;
diff --git a/gdb/stack.c b/gdb/stack.c
index 66517cf186..049a4455ed 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -55,6 +55,7 @@
#include "gdbsupport/def-vector.h"
#include "cli/cli-option.h"
#include "cli/cli-style.h"
+#include <unordered_set>
/* The possible choices of "set print frame-arguments", and the value
of this setting. */
@@ -2243,13 +2244,17 @@ backtrace_command_completer (struct cmd_list_element *ignore,
static void
iterate_over_block_locals (const struct block *b,
iterate_over_block_arg_local_vars_cb cb,
- void *cb_data)
+ void *cb_data,
+ std::unordered_set<std::string> &collected_vars)
{
struct block_iterator iter;
struct symbol *sym;
ALL_BLOCK_SYMBOLS (b, iter, sym)
{
+ const char *name = sym->print_name ();
+ bool already_collected
+ = collected_vars.find (name) != collected_vars.end ();
switch (SYMBOL_CLASS (sym))
{
case LOC_CONST:
@@ -2262,7 +2267,28 @@ iterate_over_block_locals (const struct block *b,
break;
if (SYMBOL_DOMAIN (sym) == COMMON_BLOCK_DOMAIN)
break;
- (*cb) (sym->print_name (), sym, cb_data);
+ /* Only for C/C++/Fortran languages, in case of variables shadowing
+ print <shadowed: decl at line #> annotation after the superblock
+ variable. Iteration of block starts from inner block so
+ collected_vars variable keeps track of the variables in the
+ innerblock. */
+ if ((current_language->la_language == language_c
+ || current_language->la_language == language_cplus
+ || current_language->la_language == language_fortran)
+ && already_collected)
+ (*cb) (name, sym, cb_data, true);
+ /* In case of rust language it is possible to declare variable with
+ same name multiple times and only latest declaration of variable
+ is accessible. So print only the first instance and there is no
+ need of printing duplicates. */
+ else if (current_language->la_language == language_rust
+ && already_collected)
+ break;
+ else
+ {
+ collected_vars.insert (name);
+ (*cb) (name, sym, cb_data, false);
+ }
break;
default:
@@ -2280,9 +2306,10 @@ iterate_over_block_local_vars (const struct block *block,
iterate_over_block_arg_local_vars_cb cb,
void *cb_data)
{
+ std::unordered_set<std::string> collected_vars;
while (block)
{
- iterate_over_block_locals (block, cb, cb_data);
+ iterate_over_block_locals (block, cb, cb_data, collected_vars);
/* After handling the function's top-level block, stop. Don't
continue to its superblock, the block of per-file
symbols. */
@@ -2310,7 +2337,8 @@ struct print_variable_and_value_data
static void
do_print_variable_and_value (const char *print_name,
struct symbol *sym,
- void *cb_data)
+ void *cb_data,
+ bool shadowed)
{
struct print_variable_and_value_data *p
= (struct print_variable_and_value_data *) cb_data;
@@ -2332,7 +2360,8 @@ do_print_variable_and_value (const char *print_name,
return;
}
- print_variable_and_value (print_name, sym, frame, p->stream, p->num_tabs);
+ print_variable_and_value (print_name, sym, frame, p->stream,
+ p->num_tabs, shadowed);
/* print_variable_and_value invalidates FRAME. */
frame = NULL;
@@ -2520,7 +2549,7 @@ iterate_over_block_arg_vars (const struct block *b,
sym2 = lookup_symbol_search_name (sym->search_name (),
b, VAR_DOMAIN).symbol;
- (*cb) (sym->print_name (), sym2, cb_data);
+ (*cb) (sym->print_name (), sym2, cb_data, false);
}
}
}
diff --git a/gdb/stack.h b/gdb/stack.h
index 17d4119988..b063cb6348 100644
--- a/gdb/stack.h
+++ b/gdb/stack.h
@@ -32,7 +32,8 @@ gdb::unique_xmalloc_ptr<char> find_frame_funname (struct frame_info *frame,
typedef void (*iterate_over_block_arg_local_vars_cb) (const char *print_name,
struct symbol *sym,
- void *cb_data);
+ void *cb_data,
+ bool shadowed);
void iterate_over_block_arg_vars (const struct block *block,
iterate_over_block_arg_local_vars_cb cb,
diff --git a/gdb/testsuite/gdb.base/var-shadowing.c b/gdb/testsuite/gdb.base/var-shadowing.c
new file mode 100755
index 0000000000..6363821dba
--- /dev/null
+++ b/gdb/testsuite/gdb.base/var-shadowing.c
@@ -0,0 +1,52 @@
+/* Copyright (C) 2021 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/>. */
+
+/* This is only ever run if it is compiled with a new-enough GCC, but
+ we don't want the compilation to fail if compiled by some other
+ compiler. */
+
+#include <stdlib.h>
+
+void
+shadowing ()
+{
+ int a;
+ unsigned int val1 = 1; /* val1-d1 */
+ unsigned int val2 = 2; /* val2-d1 */
+ a = 101; /* bp for locals 1 */
+ {
+ unsigned int val2 = 3; /* val2-d2 */
+ unsigned int val3 = 4; /* val3-d1 */
+ a = 102; /* bp for locals 2 */
+ {
+ unsigned int val1 = 5; /* val1-d2 */
+ a = 103; /* bp for locals 3 */
+ {
+ unsigned int val1 = 6;
+ unsigned int val2 = 7;
+ unsigned int val3 = 8;
+ a = 104; /* bp for locals 4 */
+ }
+ }
+ }
+ a = 0; /* bp for locals 5 */
+}
+
+int
+main (void)
+{
+ shadowing ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.base/var-shadowing.exp b/gdb/testsuite/gdb.base/var-shadowing.exp
new file mode 100755
index 0000000000..b43f5bfc7c
--- /dev/null
+++ b/gdb/testsuite/gdb.base/var-shadowing.exp
@@ -0,0 +1,85 @@
+# Copyright 2021 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/>.
+
+standard_testfile
+if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
+ return -1
+}
+
+if ![runto_main] {
+ untested "failed to run to main"
+ return -1
+}
+
+set bp_line1 [gdb_get_line_number "bp for locals 1" ${srcfile}]
+set bp_line2 [gdb_get_line_number "bp for locals 2" ${srcfile}]
+set bp_line3 [gdb_get_line_number "bp for locals 3" ${srcfile}]
+set bp_line4 [gdb_get_line_number "bp for locals 4" ${srcfile}]
+set bp_line5 [gdb_get_line_number "bp for locals 5" ${srcfile}]
+
+set val1_d1 [gdb_get_line_number "val1-d1" ${srcfile}]
+set val1_d2 [gdb_get_line_number "val1-d2" ${srcfile}]
+set val2_d1 [gdb_get_line_number "val2-d1" ${srcfile}]
+set val2_d2 [gdb_get_line_number "val2-d2" ${srcfile}]
+set val3_d1 [gdb_get_line_number "val3-d1" ${srcfile}]
+
+gdb_breakpoint $srcfile:$bp_line1
+gdb_test "continue" ".*bp for locals 1.*" "continue to outermost level"
+gdb_test "info locals" [multi_line \
+ "val1 = 1" \
+ "val2 = 2" \
+ ] "info locals at outermost level"
+
+gdb_breakpoint $srcfile:$bp_line2
+gdb_test "continue" ".*bp for locals 2.*" "continue to first level"
+gdb_test "info locals" [multi_line \
+ "val2 = 3" \
+ "val3 = 4" \
+ "a = 101" \
+ "val1 = 1" \
+ "val2 = 2\t<shadowed: decl at line $val2_d1>" \
+ ] "info locals first level"
+
+gdb_breakpoint $srcfile:$bp_line3
+gdb_test "continue" ".*bp for locals 3.*" "continue to second level"
+gdb_test "info locals" [multi_line \
+ "val1 = 5" \
+ "val2 = 3" \
+ "val3 = 4" \
+ "a = 102" \
+ "val1 = 1\t<shadowed: decl at line $val1_d1>" \
+ "val2 = 2\t<shadowed: decl at line $val2_d1>" \
+ ] "info locals second level"
+
+gdb_breakpoint $srcfile:$bp_line4
+gdb_test "continue" ".*bp for locals 4.*" "continue to innermost level"
+gdb_test "info locals" [multi_line \
+ "val1 = 6" \
+ "val2 = 7" \
+ "val3 = 8" \
+ "val1 = 5\t<shadowed: decl at line $val1_d2>" \
+ "val2 = 3\t<shadowed: decl at line $val2_d2>" \
+ "val3 = 4\t<shadowed: decl at line $val3_d1>" \
+ "a = 103" \
+ "val1 = 1\t<shadowed: decl at line $val1_d1>" \
+ "val2 = 2\t<shadowed: decl at line $val2_d1>" \
+ ] "info locals at innermost level"
+
+gdb_breakpoint $srcfile:$bp_line5
+gdb_test "continue" ".*bp for locals 5.*" "continue to outermost level last"
+gdb_test "info locals" [multi_line \
+ "val1 = 1" \
+ "val2 = 2" \
+ ] "info locals at outermost level last"
diff --git a/gdb/testsuite/gdb.opt/inline-locals.c b/gdb/testsuite/gdb.opt/inline-locals.c
index f8910dadca..cf5a96ad97 100644
--- a/gdb/testsuite/gdb.opt/inline-locals.c
+++ b/gdb/testsuite/gdb.opt/inline-locals.c
@@ -39,7 +39,7 @@ init_array (int *array, int n)
inline ATTR int func1(int arg1)
{
- int array[64];
+ int array[64]; /* array-d1 */
init_array (array, 64);
array_p = array;
array[0] = result;
diff --git a/gdb/testsuite/gdb.opt/inline-locals.exp b/gdb/testsuite/gdb.opt/inline-locals.exp
index ff9dd1b49e..04b8b9cd8e 100644
--- a/gdb/testsuite/gdb.opt/inline-locals.exp
+++ b/gdb/testsuite/gdb.opt/inline-locals.exp
@@ -29,6 +29,8 @@ if { [skip_inline_var_tests] } {
return
}
+set array_d1 [gdb_get_line_number "array-d1" ${srcfile}]
+
set no_frames [skip_inline_frame_tests]
set line1 [gdb_get_line_number "set breakpoint 1 here" ${srcfile2}]
@@ -45,7 +47,7 @@ if { ! $no_frames } {
gdb_test "info frame" ".*inlined into frame.*" "func1 inlined 2"
set pass_re "array = \\{0 <repeats 64 times>\\}"
set kfail_re [multi_line $pass_re \
- "array = <optimized out>"]
+ "array = <optimized out>\t<shadowed: decl at line $array_d1>"]
gdb_test_multiple "info locals" "info locals above bar 2" {
-re -wrap $pass_re {
pass $gdb_test_name
@@ -94,7 +96,7 @@ if { ! $no_frames } {
gdb_test "info frame" ".*inlined into frame.*" "func1 inlined 3"
set pass_re "array = {$decimal, \[^\r\n\]*}"
set kfail_re [multi_line $pass_re \
- "array = <optimized out>"]
+ "array = <optimized out>\t<shadowed: decl at line $array_d1>"]
gdb_test_multiple "info locals" "info locals above bar 3" {
-re -wrap $pass_re {
pass $gdb_test_name
@@ -134,7 +136,7 @@ proc check_scoped_locals {bp_label pass_re} {
gdb_breakpoint $srcfile:$locals_bp
gdb_continue_to_breakpoint "$bp_label" ".*$srcfile:$locals_bp.*"
- set kfail_re [multi_line $pass_re ".*<optimized out>"]
+ set kfail_re [multi_line $pass_re ".*<optimized out>\t(<shadowed: decl at line .*>)?"]
gdb_test_multiple "info locals" "scoped info locals at $bp_label" {
-re -wrap $pass_re {
pass $gdb_test_name
diff --git a/gdb/testsuite/gdb.rust/var_reuse.exp b/gdb/testsuite/gdb.rust/var_reuse.exp
new file mode 100755
index 0000000000..c535aaa759
--- /dev/null
+++ b/gdb/testsuite/gdb.rust/var_reuse.exp
@@ -0,0 +1,33 @@
+# Copyright 2021 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 rust-support.exp
+if {[skip_rust_tests]} {
+ continue
+}
+
+standard_testfile .rs
+if {[prepare_for_testing "failed to prepare" \
+ $testfile $srcfile {debug rust}]} {
+ return -1
+}
+
+set line [gdb_get_line_number "set breakpoint here"]
+if {![runto ${srcfile}:$line]} {
+ untested "could not run to breakpoint"
+ return -1
+}
+
+gdb_test "info local _x" "_x = 12" "print local _x variable"
diff --git a/gdb/testsuite/gdb.rust/var_reuse.rs b/gdb/testsuite/gdb.rust/var_reuse.rs
new file mode 100755
index 0000000000..63d165c4aa
--- /dev/null
+++ b/gdb/testsuite/gdb.rust/var_reuse.rs
@@ -0,0 +1,20 @@
+// Copyright (C) 2021 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/>.
+
+fn main() {
+ let _x = 5;
+ let _x = _x + 7;
+ let _y = 8; // set breakpoint here
+}
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 15506fdbf4..12292a3b6d 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1059,7 +1059,8 @@ struct add_local_symbols_data
static void
do_collect_symbol (const char *print_name,
struct symbol *sym,
- void *cb_data)
+ void *cb_data,
+ bool shadowed)
{
struct add_local_symbols_data *p = (struct add_local_symbols_data *) cb_data;
diff --git a/gdb/value.h b/gdb/value.h
index 6fa57e0da6..781d1eb546 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1118,7 +1118,8 @@ extern void print_variable_and_value (const char *name,
struct symbol *var,
struct frame_info *frame,
struct ui_file *stream,
- int indent);
+ int indent,
+ bool shadowed);
extern void typedef_print (struct type *type, struct symbol *news,
struct ui_file *stream);
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] gdb: add shadowed_loc field in '-stack-list-locals/variables' mi commands
2021-12-29 11:14 [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case abdul.b.ijaz
2021-12-29 11:14 ` [PATCH v2 1/2] gdb: " abdul.b.ijaz
@ 2021-12-29 11:14 ` abdul.b.ijaz
2022-01-01 20:14 ` [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case Philippe Waroquiers
2 siblings, 0 replies; 8+ messages in thread
From: abdul.b.ijaz @ 2021-12-29 11:14 UTC (permalink / raw)
To: gdb-patches
Cc: abdul.b.ijaz, jonah, Tankut.Baris.Aktemur, philippe.waroquiers,
cristian.sandu
For C/C++/Fortran languages GDB prints same name variable multiple times in
case of variable shadowing and it is confusing for user to identify which
variable belongs to the current scope. So add 'shadowed_loc' field in
'-stack-list-locals' and '-stack-list-variables' mi commands for value of
super-block shadowed variable.
Suppose we have
1:int x = 42;
2: {
3: int x = 99;
4: x = 99; /* break here */
5: }
The "-stack-list-locals" and "-stack-list-variables" mi commands at the
"break here" line gives the following output:
(gdb)
-stack-list-locals 0
^done,locals=[name="x",name="x"]
(gdb)
-stack-list-locals 1
^done,locals=[{name="x",value="99"},{name="x",value="42"}]
(gdb)
-stack-list-locals 2
^done,locals=[{name="x",type="int",value="99"},{name="x",type="int",value="42"}]
(gdb)
-stack-list-variables 0
^done,variables=[{name="x"},{name="x"}]
(gdb)
-stack-list-variables 1
^done,variables=[{name="x",value="99"},{name="x",value="42"}]
(gdb)
-stack-list-variables 2
^done,variables=[{name="x",type="int",value="99"},{name="x",type="int",value="42"}]
So far this scenario is only known case for C/C++/Fortran language so
to tackle in this case, when iterating over the stack frame variables keep
the track of inner block variables and when same variable is found in the
outer-block then add shadowed_loc in mi command output with only exception to
'stack-list-locals 0' case where it is not added. Iteration of stack
frame variables always starts with inner block so gdb now print shadowed
field with value true for super-block duplicate variables like this:
(gdb)
-stack-list-locals 0
^done,locals=[name="x",name="x"]
(gdb)
-stack-list-locals 1
^done,locals=[{name="x",value="99"},{name="x",shadowed_loc="1",value="42"}]
(gdb)
-stack-list-locals 2
^done,locals=[{name="x",type="int",value="99"},{name="x",shadowed_loc="1",type="int",value="42"}]
(gdb)
-stack-list-variables 0
^done,variables=[{name="x"},{name="x",shadowed_loc="1"}]
(gdb)
-stack-list-variables 1
^done,variables=[{name="x",value="99"},{name="x",shadowed_loc="1",value="42"}]
(gdb)
-stack-list-variables 2
^done,variables=[{name="x",type="int",value="99"},{name="x",shadowed_loc="1",type="int",value="42"}]
In case of rust language it is possible to declare variable with same name
many times so in this case just print only first instance of variable.
gdb/ChangeLog:
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
* mi/mi-cmd-stack.c (list_arg_or_local): Print shadowed_loc field
with line info after the value of super-block variable in
case of variable shadowing.
(list_args_or_locals): Declare unordered_set type object and
pass it to function 'list_arg_or_local'.
gdb/doc/ChangeLog:
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
* gdb.textinfo: Update with shadowed variable line info.
gdb/testsuite/ChangeLog:
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
* gdb.mi/mi2-amd64-entry-value.exp: Update regex expressions for
testing '-stack-list-variables' mi command output to handle
shadowed_loc field.
* gdb.mi/mi2-amd64-entry-value.c: Add new comments to
refer lines.
* gdb.trace/entry-values.exp: Change test cases to
support the shadowed variable print feature.
* gdb.mi/mi-var-shadowing.c: New file.
* gdb.mi/mi-var-shadowing.exp: New file.
2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
---
gdb/doc/gdb.texinfo | 12 ++
gdb/mi/mi-cmd-stack.c | 38 ++++-
gdb/testsuite/gdb.mi/mi-var-shadowing.c | 48 ++++++
gdb/testsuite/gdb.mi/mi-var-shadowing.exp | 138 ++++++++++++++++++
gdb/testsuite/gdb.mi/mi2-amd64-entry-value.c | 6 +-
.../gdb.mi/mi2-amd64-entry-value.exp | 23 +--
gdb/testsuite/gdb.trace/entry-values.exp | 2 +-
7 files changed, 250 insertions(+), 17 deletions(-)
create mode 100644 gdb/testsuite/gdb.mi/mi-var-shadowing.c
create mode 100644 gdb/testsuite/gdb.mi/mi-var-shadowing.exp
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 83994c3486..894ab0d737 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -32943,6 +32943,18 @@ If the @code{--skip-unavailable} option is specified, local variables
and arguments that are not available are not listed. Partially
available arguments and local variables are still displayed, however.
+@smallexample
+1: int x = 3;
+2: @{
+3: int x = 4; // breakpt
+4: @}
+(gdb) -stack-list-variables 2
+^done,variables=[@{name="x",type="int",value="4"@},@{name="x",shadowed_loc="1",type="int",value="3"@}]
+@end smallexample
+
+If a variable is shadowed, the @code{shadowed_loc} attribute is added and
+the line of declaration is displayed.
+
@subsubheading Example
@smallexample
diff --git a/gdb/mi/mi-cmd-stack.c b/gdb/mi/mi-cmd-stack.c
index 9bc9010239..f7211fbfa4 100644
--- a/gdb/mi/mi-cmd-stack.c
+++ b/gdb/mi/mi-cmd-stack.c
@@ -36,6 +36,7 @@
#include "mi-parse.h"
#include "gdbsupport/gdb_optional.h"
#include "safe-ctype.h"
+#include <unordered_set>
enum what_to_list { locals, arguments, all };
@@ -484,7 +485,8 @@ mi_cmd_stack_list_variables (const char *command, char **argv, int argc)
static void
list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
- enum print_values values, int skip_unavailable)
+ enum print_values values, int skip_unavailable,
+ std::unordered_set<std::string> &collected_vars)
{
struct ui_out *uiout = current_uiout;
@@ -514,6 +516,15 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
tuple_emitter.emplace (uiout, nullptr);
string_file stb;
+ bool already_collected
+ = collected_vars.find (arg->sym->print_name ()) != collected_vars.end ();
+
+ /* In case of rust language it is possible to declare variable with
+ same name multiple times and only latest declaration of variable
+ is accessible. So print only the first instance and there is no
+ need of printing duplicates. */
+ if (current_language->la_language == language_rust && already_collected)
+ return;
stb.puts (arg->sym->print_name ());
if (arg->entry_kind == print_entry_values_only)
@@ -523,6 +534,24 @@ list_arg_or_local (const struct frame_arg *arg, enum what_to_list what,
if (what == all && SYMBOL_IS_ARGUMENT (arg->sym))
uiout->field_signed ("arg", 1);
+ /* Only for C/C++/Fortran languages, in case of variables shadowing
+ print shadowed_loc field after the superblock variable. Iteration of
+ block starts from inner block so collected_vars variable keeps
+ track of the variables in the innerblock. */
+ if ((current_language->la_language == language_c
+ || current_language->la_language == language_cplus
+ || current_language->la_language == language_fortran)
+ && !(values == PRINT_NO_VALUES && what == locals)
+ && already_collected)
+ {
+ if (arg->sym->line > 0)
+ uiout->field_unsigned ("shadowed_loc", arg->sym->line);
+ else
+ uiout->field_string ("shadowed_loc", "NA");
+ }
+ else
+ collected_vars.insert (arg->sym->print_name ());
+
if (values == PRINT_SIMPLE_VALUES)
{
check_typedef (arg->sym->type);
@@ -572,6 +601,7 @@ list_args_or_locals (const frame_print_options &fp_opts,
struct type *type;
const char *name_of_result;
struct ui_out *uiout = current_uiout;
+ std::unordered_set<std::string> collected_vars;
block = get_frame_block (fi, 0);
@@ -663,9 +693,11 @@ list_args_or_locals (const frame_print_options &fp_opts,
}
if (arg.entry_kind != print_entry_values_only)
- list_arg_or_local (&arg, what, values, skip_unavailable);
+ list_arg_or_local (&arg, what, values,
+ skip_unavailable, collected_vars);
if (entryarg.entry_kind != print_entry_values_no)
- list_arg_or_local (&entryarg, what, values, skip_unavailable);
+ list_arg_or_local (&entryarg, what, values,
+ skip_unavailable, collected_vars);
}
}
diff --git a/gdb/testsuite/gdb.mi/mi-var-shadowing.c b/gdb/testsuite/gdb.mi/mi-var-shadowing.c
new file mode 100644
index 0000000000..e5e5a8faf5
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-var-shadowing.c
@@ -0,0 +1,48 @@
+/* Copyright (C) 2021 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/>. */
+
+#include <stdlib.h>
+
+void
+shadowing ()
+{
+ int a;
+ unsigned int val1 = 1; /* val1-d1 */
+ unsigned int val2 = 2; /* val2-d1 */
+ a = 101; /* bp for locals 1 */
+ {
+ unsigned int val2 = 3; /* val2-d2 */
+ unsigned int val3 = 4; /* val3-d1 */
+ a = 102; /* bp for locals 2 */
+ {
+ unsigned int val1 = 5; /* val1-d2 */
+ a = 103; /* bp for locals 3 */
+ {
+ unsigned int val1 = 6;
+ unsigned int val2 = 7;
+ unsigned int val3 = 8;
+ a = 104; /* bp for locals 4 */
+ }
+ }
+ }
+ a = 105; /* bp for locals 5 */
+}
+
+int
+main (void)
+{
+ shadowing ();
+ return 0;
+}
diff --git a/gdb/testsuite/gdb.mi/mi-var-shadowing.exp b/gdb/testsuite/gdb.mi/mi-var-shadowing.exp
new file mode 100644
index 0000000000..25d16db683
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-var-shadowing.exp
@@ -0,0 +1,138 @@
+# Copyright 2021 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 mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+ continue
+}
+
+standard_testfile
+
+set opts {debug}
+if [build_executable ${testfile}.exp ${testfile} ${srcfile} $opts] {
+ return -1
+}
+
+mi_clean_restart $binfile
+
+if {[mi_runto_main] < 0} {
+ return -1
+}
+
+set bp_line1 [gdb_get_line_number "bp for locals 1" ${srcfile}]
+set bp_line2 [gdb_get_line_number "bp for locals 2" ${srcfile}]
+set bp_line3 [gdb_get_line_number "bp for locals 3" ${srcfile}]
+set bp_line4 [gdb_get_line_number "bp for locals 4" ${srcfile}]
+set bp_line5 [gdb_get_line_number "bp for locals 5" ${srcfile}]
+
+set val1_d1 [gdb_get_line_number "val1-d1" ${srcfile}]
+set val1_d2 [gdb_get_line_number "val1-d2" ${srcfile}]
+set val2_d1 [gdb_get_line_number "val2-d1" ${srcfile}]
+set val2_d2 [gdb_get_line_number "val2-d2" ${srcfile}]
+set val3_d1 [gdb_get_line_number "val3-d1" ${srcfile}]
+
+set stack_test1_regx "\\^done,(locals|variables)=\\\[\{name=\"a\",type=\"int\",value=\"$decimal\"\},\{name=\"val1\",type=\"unsigned int\",value=\"1\"\},{name=\"val2\",type=\"unsigned int\",value=\"2\"\}\\\]"
+set stack_test2_regx "\\^done,(locals|variables)=\\\[\{name=\"val2\",type=\"unsigned int\",value=\"3\"\},\{name=\"val3\",type=\"unsigned int\",value=\"4\"\},\{name=\"a\",type=\"int\",value=\"101\"\},\{name=\"val1\",type=\"unsigned int\",value=\"1\"\},\{name=\"val2\",shadowed_loc=\"$val2_d1\",type=\"unsigned int\",value=\"2\"\}\\\]"
+set stack_test3_regx "\\^done,(locals|variables)=\\\[\{name=\"val1\",type=\"unsigned int\",value=\"5\"},{name=\"val2\",type=\"unsigned int\",value=\"3\"\},\{name=\"val3\",type=\"unsigned int\",value=\"4\"\},\{name=\"a\",type=\"int\",value=\"102\"\},\{name=\"val1\",shadowed_loc=\"$val1_d1\",type=\"unsigned int\",value=\"1\"\},\{name=\"val2\",shadowed_loc=\"$val2_d1\",type=\"unsigned int\",value=\"2\"\}\\\]"
+set stack_test4_regx "\\^done,(locals|variables)=\\\[\{name=\"val1\",type=\"unsigned int\",value=\"6\"\},\{name=\"val2\",type=\"unsigned int\",value=\"7\"\},\{name=\"val3\",type=\"unsigned int\",value=\"8\"\},\{name=\"val1\",shadowed_loc=\"$val1_d2\",type=\"unsigned int\",value=\"5\"\},\{name=\"val2\",shadowed_loc=\"$val2_d2\",type=\"unsigned int\",value=\"3\"\},\{name=\"val3\",shadowed_loc=\"$val3_d1\",type=\"unsigned int\",value=\"4\"\},\{name=\"a\",type=\"int\",value=\"103\"\},\{name=\"val1\",shadowed_loc=\"$val1_d1\",type=\"unsigned int\",value=\"1\"\},\{name=\"val2\",shadowed_loc=\"$val2_d1\",type=\"unsigned int\",value=\"2\"\}\\\]"
+set stack_test5_regx "\\^done,(locals|variables)=\\\[\{name=\"a\",type=\"int\",value=\"104\"\},\{name=\"val1\",type=\"unsigned int\",value=\"1\"\},\{name=\"val2\",type=\"unsigned int\",value=\"2\"\}\\\]"
+
+mi_gdb_test \
+ "-break-insert --source ${srcfile} --line ${bp_line1}" \
+ "\\^done.*source ${srcfile} \\-line ${bp_line1}.*" \
+ "bp at outermost level"
+mi_execute_to "exec-continue" "breakpoint-hit" ".*" ".*" ".*" "${bp_line1}" \
+ { "" "disp=\"keep\"" } "continue to outermost level"
+mi_gdb_test "-stack-list-locals 0" \
+ "\\^done,locals=\\\[name=\"a\",name=\"val1\",name=\"val2\"\\\]" \
+ "-stack-list-locals 0 at outermost level"
+mi_gdb_test "-stack-list-variables 0" \
+ "\\^done,variables=\\\[{name=\"a\"},{name=\"val1\"},{name=\"val2\"}\\\]" \
+ "-stack-list-variables 0 at outermost level"
+mi_gdb_test "-stack-list-locals 2" "${stack_test1_regx}" \
+ "-stack-list-locals 2 at outermost level"
+mi_gdb_test "-stack-list-variables 2" "${stack_test1_regx}" \
+ "-stack-list-variables 2 at outermost level"
+
+mi_gdb_test \
+ "-break-insert --source ${srcfile} --line ${bp_line2}" \
+ "\\^done.*source ${srcfile} \\-line ${bp_line2}.*" \
+ "bp at first level"
+mi_execute_to "exec-continue" "breakpoint-hit" ".*" ".*" ".*" "${bp_line2}" \
+ { "" "disp=\"keep\"" } "continue to first level"
+mi_gdb_test "-stack-list-locals 0" \
+ "\\^done,locals=\\\[name=\"val2\",name=\"val3\",name=\"a\",name=\"val1\",name=\"val2\"\\\]" \
+ "-stack-list-locals 0 at first level"
+mi_gdb_test "-stack-list-variables 0" \
+ "\\^done,variables=\\\[{name=\"val2\"},{name=\"val3\"},{name=\"a\"},{name=\"val1\"},{name=\"val2\",shadowed_loc=\"$val2_d1\"}\\\]" \
+ "-stack-list-variables 0 at first level"
+mi_gdb_test "-stack-list-locals 2" "${stack_test2_regx}" \
+ "-stack-list-locals 2 at first level"
+mi_gdb_test "-stack-list-variables 2" "${stack_test2_regx}" \
+ "-stack-list-variables 2 at first level"
+
+mi_gdb_test \
+ "-break-insert --source ${srcfile} --line ${bp_line3}" \
+ "\\^done.*source ${srcfile} \\-line ${bp_line3}.*" \
+ "bp at second level"
+mi_execute_to "exec-continue" "breakpoint-hit" ".*" ".*" ".*" "${bp_line3}" \
+ { "" "disp=\"keep\"" } "continue to second level"
+mi_gdb_test "-stack-list-locals 0" \
+ "\\^done,locals=\\\[name=\"val1\",name=\"val2\",name=\"val3\",name=\"a\",name=\"val1\",name=\"val2\"\\\]" \
+ "-stack-list-locals 0 at second level"
+mi_gdb_test "-stack-list-variables 0" \
+ "\\^done,variables=\\\[{name=\"val1\"},{name=\"val2\"},{name=\"val3\"},{name=\"a\"},{name=\"val1\",shadowed_loc=\"$val1_d1\"},{name=\"val2\",shadowed_loc=\"$val2_d1\"}\\\]" \
+ "-stack-list-variables 0 at second level"
+mi_gdb_test "-stack-list-locals 2" "${stack_test3_regx}" \
+ "-stack-list-locals 2 at second level"
+mi_gdb_test "-stack-list-variables 2" "${stack_test3_regx}" \
+ "-stack-list-variables 2 at second level"
+
+mi_gdb_test \
+ "-break-insert --source ${srcfile} --line ${bp_line4}" \
+ "\\^done.*source ${srcfile} \\-line ${bp_line4}.*" \
+ "bp at third level"
+mi_execute_to "exec-continue" "breakpoint-hit" ".*" ".*" ".*" "${bp_line4}" \
+ { "" "disp=\"keep\"" } "continue to third level"
+mi_gdb_test "-stack-list-locals 0" \
+ "\\^done,locals=\\\[name=\"val1\",name=\"val2\",name=\"val3\",name=\"val1\",name=\"val2\",name=\"val3\",name=\"a\",name=\"val1\",name=\"val2\"\\\]" \
+ "-stack-list-locals 0 at third level"
+mi_gdb_test "-stack-list-variables 0" \
+ "\\^done,variables=\\\[{name=\"val1\"},{name=\"val2\"},{name=\"val3\"},{name=\"val1\",shadowed_loc=\"$val1_d2\"},{name=\"val2\",shadowed_loc=\"$val2_d2\"},{name=\"val3\",shadowed_loc=\"$val3_d1\"},{name=\"a\"},{name=\"val1\",shadowed_loc=\"$val1_d1\"},{name=\"val2\",shadowed_loc=\"$val2_d1\"}\\\]" \
+ "-stack-list-variables 0 at third level"
+mi_gdb_test "-stack-list-locals 2" "${stack_test4_regx}" \
+ "-stack-list-locals 2 at third level"
+mi_gdb_test "-stack-list-variables 2" "${stack_test4_regx}" \
+ "-stack-list-variables 2 at third level"
+
+mi_gdb_test \
+ "-break-insert --source ${srcfile} --line ${bp_line5}" \
+ "\\^done.*source ${srcfile} \\-line ${bp_line5}.*" \
+ "bp at outermost level last"
+mi_execute_to "exec-continue" "breakpoint-hit" ".*" ".*" ".*" "${bp_line5}" \
+ { "" "disp=\"keep\"" } "continue to outermost level last"
+mi_gdb_test "-stack-list-locals 0" \
+ "\\^done,locals=\\\[name=\"a\",name=\"val1\",name=\"val2\"\\\]" \
+ "-stack-list-locals 0 at outermost level last"
+mi_gdb_test "-stack-list-variables 0" \
+ "\\^done,variables=\\\[{name=\"a\"},{name=\"val1\"},{name=\"val2\"}\\\]" \
+ "-stack-list-variables at outermost level last"
+mi_gdb_test "-stack-list-locals 2" "${stack_test5_regx}" \
+ "-stack-list-locals 2 at outermost level last"
+mi_gdb_test "-stack-list-variables 2" "${stack_test5_regx}" \
+ "-stack-list-variables 2 at outermost level last"
diff --git a/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.c b/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.c
index f1aacce62e..5ab08e5eee 100644
--- a/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.c
+++ b/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.c
@@ -36,7 +36,7 @@ data2 (void)
}
static int __attribute__((noinline, noclone))
-different (int val)
+different (int val) /* different-d1 */
{
val++;
e (val, val);
@@ -45,7 +45,7 @@ asm ("breakhere_different:");
}
static int __attribute__((noinline, noclone))
-validity (int lost, int born)
+validity (int lost, int born) /* validity-d1 */
{
lost = data ();
e (0, 0.0);
@@ -54,7 +54,7 @@ asm ("breakhere_validity:");
}
static void __attribute__((noinline, noclone))
-invalid (int inv)
+invalid (int inv) /* invalid-d1 */
{
e (0, 0.0);
asm ("breakhere_invalid:");
diff --git a/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.exp b/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.exp
index cf60242c34..a2378fe85a 100644
--- a/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.exp
+++ b/gdb/testsuite/gdb.mi/mi2-amd64-entry-value.exp
@@ -46,6 +46,9 @@ foreach name {different breakhere_different breakhere_validity breakhere_invalid
mi_create_breakpoint $name "break $name"
}
+set different_d1 [gdb_get_line_number "different-d1" ${testfile}.c]
+set validity_d1 [gdb_get_line_number "validity-d1" ${testfile}.c]
+set invalid_d1 [gdb_get_line_number "invalid-d1" ${testfile}.c]
# Test various kinds of `set print entry-values'.
@@ -132,16 +135,16 @@ with_test_prefix "entry-values=both" {
mi_gdb_test "-gdb-set print entry-values both" {\^done} "both: set print entry-values"
mi_send_resuming_command "exec-continue" "both: entry_equal: continue"
mi_expect_stop "breakpoint-hit" .* {{name="val",value="5"},{name="val@entry",value="5"}} .* .* {.* disp="keep"} "both: entry_equal: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="val",arg="1",value="5"},{name="val@entry",arg="1",value="5"}\]} "both: entry_equal: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"val\",arg=\"1\",value=\"5\"\},\{name=\"val@entry\",arg=\"1\",shadowed_loc=\"$different_d1\",value=\"5\"\}\\\]" "both: entry_equal: -stack-list-variables"
mi_send_resuming_command "exec-continue" "both: entry_different: continue"
mi_expect_stop "breakpoint-hit" .* {{name="val",value="6"},{name="val@entry",value="5"}} .* .* {.* disp="keep"} "both: entry_different: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="val",arg="1",value="6"},{name="val@entry",arg="1",value="5"}\]} "both: entry_different: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"val\",arg=\"1\",value=\"6\"\},\{name=\"val@entry\",arg=\"1\",shadowed_loc=\"$different_d1\",value=\"5\"\}\\\]" "both: entry_different: -stack-list-variables"
mi_send_resuming_command "exec-continue" "both: validity: continue"
mi_expect_stop "breakpoint-hit" .* {{name="lost",value="<optimized out>"},{name="lost@entry",value="5"},{name="born",value="10"},{name="born@entry",value="<optimized out>"}} .* .* {.* disp="keep"} "both: validity: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="lost",arg="1",value="<optimized out>"},{name="lost@entry",arg="1",value="5"},{name="born",arg="1",value="10"},{name="born@entry",arg="1",value="<optimized out>"}\]} "both: validity: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"lost\",arg=\"1\",value=\"<optimized out>\"\},\{name=\"lost@entry\",arg=\"1\",shadowed_loc=\"$validity_d1\",value=\"5\"\},\{name=\"born\",arg=\"1\",value=\"10\"\},\{name=\"born@entry\",arg=\"1\",shadowed_loc=\"$validity_d1\",value=\"<optimized out>\"\}\\\]" "both: validity: -stack-list-variables"
mi_send_resuming_command "exec-continue" "both: invalid: continue"
mi_expect_stop "breakpoint-hit" .* {{name="inv",value="<optimized out>"},{name="inv@entry",value="<optimized out>"}} .* .* {.* disp="keep"} "both: invalid: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="inv",arg="1",value="<optimized out>"},{name="inv@entry",arg="1",value="<optimized out>"}\]} "both: invalid: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"inv\",arg=\"1\",value=\"<optimized out>\"\},\{name=\"inv@entry\",arg=\"1\",shadowed_loc=\"$invalid_d1\",value=\"<optimized out>\"\}\\\]" "both: invalid: -stack-list-variables"
}
with_test_prefix "entry-values=compact" {
@@ -151,13 +154,13 @@ with_test_prefix "entry-values=compact" {
mi_gdb_test "-gdb-set print entry-values compact" {\^done} "compact: set print entry-values"
mi_send_resuming_command "exec-continue" "compact: entry_equal: continue"
mi_expect_stop "breakpoint-hit" .* {{name="val",value="5"},{name="val@entry",value="5"}} .* .* {.* disp="keep"} "compact: entry_equal: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="val",arg="1",value="5"},{name="val@entry",arg="1",value="5"}\]} "compact: entry_equal: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"val\",arg=\"1\",value=\"5\"\},\{name=\"val@entry\",arg=\"1\",shadowed_loc=\"$different_d1\",value=\"5\"\}\\\]" "compact: entry_equal: -stack-list-variables"
mi_send_resuming_command "exec-continue" "compact: entry_different: continue"
mi_expect_stop "breakpoint-hit" .* {{name="val",value="6"},{name="val@entry",value="5"}} .* .* {.* disp="keep"} "compact: entry_different: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="val",arg="1",value="6"},{name="val@entry",arg="1",value="5"}\]} "compact: entry_different: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"val\",arg=\"1\",value=\"6\"\},\{name=\"val@entry\",arg=\"1\",shadowed_loc=\"$different_d1\",value=\"5\"\}\\\]" "compact: entry_different: -stack-list-variables"
mi_send_resuming_command "exec-continue" "compact: validity: continue"
mi_expect_stop "breakpoint-hit" .* {{name="lost@entry",value="5"},{name="born",value="10"}} .* .* {.* disp="keep"} "compact: validity: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="lost@entry",arg="1",value="5"},{name="born",arg="1",value="10"}\]} "compact: validity: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"lost@entry\",arg=\"1\",value=\"5\"\},\{name=\"born\",arg=\"1\",value=\"10\"\}\\\]" "compact: validity: -stack-list-variables"
mi_send_resuming_command "exec-continue" "compact: invalid: continue"
mi_expect_stop "breakpoint-hit" .* {{name="inv",value="<optimized out>"}} .* .* {.* disp="keep"} "compact: invalid: stop"
mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="inv",arg="1",value="<optimized out>"}\]} "compact: invalid: -stack-list-variables"
@@ -170,13 +173,13 @@ with_test_prefix "entry-values=default" {
mi_gdb_test "-gdb-set print entry-values default" {\^done} "default: set print entry-values"
mi_send_resuming_command "exec-continue" "default: entry_equal: continue"
mi_expect_stop "breakpoint-hit" .* {{name="val",value="5"},{name="val@entry",value="5"}} .* .* {.* disp="keep"} "default: entry_equal: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="val",arg="1",value="5"},{name="val@entry",arg="1",value="5"}\]} "default: entry_equal: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"val\",arg=\"1\",value=\"5\"\},\{name=\"val@entry\",arg=\"1\",shadowed_loc=\"$different_d1\",value=\"5\"\}\\\]" "default: entry_equal: -stack-list-variables"
mi_send_resuming_command "exec-continue" "default: entry_different: continue"
mi_expect_stop "breakpoint-hit" .* {{name="val",value="6"},{name="val@entry",value="5"}} .* .* {.* disp="keep"} "default: entry_different: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="val",arg="1",value="6"},{name="val@entry",arg="1",value="5"}\]} "default: entry_different: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"val\",arg=\"1\",value=\"6\"\},\{name=\"val@entry\",arg=\"1\",shadowed_loc=\"$different_d1\",value=\"5\"\}\\\]" "default: entry_different: -stack-list-variables"
mi_send_resuming_command "exec-continue" "default: validity: continue"
mi_expect_stop "breakpoint-hit" .* {{name="lost",value="<optimized out>"},{name="lost@entry",value="5"},{name="born",value="10"}} .* .* {.* disp="keep"} "default: validity: stop"
- mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="lost",arg="1",value="<optimized out>"},{name="lost@entry",arg="1",value="5"},{name="born",arg="1",value="10"}\]} "default: validity: -stack-list-variables"
+ mi_gdb_test "-stack-list-variables --all-values" "\\^done,variables=\\\[\{name=\"lost\",arg=\"1\",value=\"<optimized out>\"\},\{name=\"lost@entry\",arg=\"1\",shadowed_loc=\"$validity_d1\",value=\"5\"\},\{name=\"born\",arg=\"1\",value=\"10\"\}\\\]" "default: validity: -stack-list-variables"
mi_send_resuming_command "exec-continue" "default: invalid: continue"
mi_expect_stop "breakpoint-hit" .* {{name="inv",value="<optimized out>"}} .* .* {.* disp="keep"} "default: invalid: stop"
mi_gdb_test "-stack-list-variables --all-values" {\^done,variables=\[{name="inv",arg="1",value="<optimized out>"}\]} "default: invalid: -stack-list-variables"
diff --git a/gdb/testsuite/gdb.trace/entry-values.exp b/gdb/testsuite/gdb.trace/entry-values.exp
index bd8a8c5559..3485b6215c 100644
--- a/gdb/testsuite/gdb.trace/entry-values.exp
+++ b/gdb/testsuite/gdb.trace/entry-values.exp
@@ -239,6 +239,6 @@ gdb_test "bt 1" "#0 .* foo \\(i=\[-\]?$decimal, i@entry=2, j=\[-\]?$decimal, j@e
# Test that unavailable "j@entry" is not shown when command option
# --skip-unavailable is used.
gdb_test "interpreter-exec mi \"-stack-list-arguments --skip-unavailable --simple-values\"" \
- "\r\n\\^done,stack-args=\\\[frame={level=\"0\",args=\\\[{name=\"i\",type=\"int\",value=\".*\"},{name=\"i@entry\",type=\"int\",value=\"2\"},{name=\"j\",type=\"int\",value=\".*\"}\\\]},frame=.*\\\].*"
+ "\r\n\\^done,stack-args=\\\[frame={level=\"0\",args=\\\[{name=\"i\",type=\"int\",value=\".*\"},{name=\"i@entry\",shadowed_loc=\"NA\",type=\"int\",value=\"2\"},{name=\"j\",type=\"int\",value=\".*\"}\\\]},frame=.*\\\].*"
gdb_test "tfind" "Target failed to find requested trace frame\..*"
--
2.31.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case
2021-12-29 11:14 [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case abdul.b.ijaz
2021-12-29 11:14 ` [PATCH v2 1/2] gdb: " abdul.b.ijaz
2021-12-29 11:14 ` [PATCH v2 2/2] gdb: add shadowed_loc field in '-stack-list-locals/variables' mi commands abdul.b.ijaz
@ 2022-01-01 20:14 ` Philippe Waroquiers
2 siblings, 0 replies; 8+ messages in thread
From: Philippe Waroquiers @ 2022-01-01 20:14 UTC (permalink / raw)
To: abdul.b.ijaz, gdb-patches
Cc: abdul.b.ijaz, jonah, Tankut.Baris.Aktemur, cristian.sandu
On Wed, 2021-12-29 at 12:14 +0100, abdul.b.ijaz wrote:
>
> @Philippe can you please review if new patch set fulfills the location information for shadowed variables.
Hello Abdul,
Thanks for this work.
IMO, this version of the patch gives the information needed to tell where
are the shadowed variables.
A remark: you might enable this functionality also for Ada language, so as to
also have shadow information for the following ashadow.adb file, to be compiled
with
gnatmake -g ashadow
with Text_Io; use Text_Io;
procedure Ashadow is
I : Integer := 1;
begin
declare
I : Integer := 11;
begin
declare
I : Integer := 111;
begin
Put_Line ("hello");
end;
end;
end;
Then with a breakpoint at line 11, currently gdb gives:
(gdb) run
Starting program: /home/philippe/gdb/littleprogs/ashadow
Breakpoint 1, ashadow () at ashadow.adb:11
11 Put_Line ("hello");
(gdb) info local
i = 111
i = 11
i = 1
(gdb)
Thanks
Philippe
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] gdb: add annotation in 'info locals' command for variables shadowing case
2021-12-29 11:14 ` [PATCH v2 1/2] gdb: " abdul.b.ijaz
@ 2022-01-05 15:20 ` Andrew Burgess
2022-01-05 15:55 ` Pedro Alves
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Burgess @ 2022-01-05 15:20 UTC (permalink / raw)
To: abdul.b.ijaz; +Cc: gdb-patches
* abdul.b.ijaz <abijaz@ecsmtp.iul.intel.com> [2021-12-29 12:14:45 +0100]:
> For C/C++/Fortran languages GDB prints same name variable multiple times in
> case of variable shadowing and it is confusing for user to identify which
> variable belongs to the current scope. So add '<shadowed: decl at line #>'
> annotation after value of super-block shadowed variable.
>
> Suppose we have
>
> 1:int x = 42;
> 2: {
> 3: int x = 99;
> 4: x = 99; /* break here */
> 5: }
>
> The "info locals" command at the "break here" line gives the following
> output:
>
> (gdb) info locals
> x = 99
> x = 42
>
> So far this scenario is only known case for C/C++/Fortran language so
> to tackle in this case, when iterating over the stack frame variables it keep
> the track of inner block variables to identify shadowed variables. Iteration
> of stack frame variables always starts with inner block so gdb now print
> <shadowed: decl at line #> for super-block duplicate variables like this:
>
> (gdb) info locals
> x = 99
> x = 42 <shadowed: decl at line 1>
Just printing the line number like this is not going to be enough if
the original definition is in some other file. Consider this crude
example:
### test.c ###
static void
breakpt (int x, int y)
{
}
int
main ()
{
#include "other-file.c"
int foo = 888;
{
int var = 123;
int foo = 456;
breakpt (var, foo);
};
return 0;
}
### other-file.c ###
int var = 999;
### END ###
Now the GDB session:
(gdb) b breakpt(int, int)
Breakpoint 1 at 0x401110: file test.c, line 4.
(gdb) r
Starting program: /home/andrew/tmp/shadow/test.x
Breakpoint 1, breakpt (x=123, y=456) at test.c:4
4 }
(gdb) up
#1 0x0000000000401146 in main () at test.c:14
14 breakpt (var, foo);
(gdb) info locals
var = 123
foo = 456
var = 999 <shadowed: decl at line 1>
foo = 888 <shadowed: decl at line 10>
(gdb)
As you can see the information for var is not helpful. I suspect we
might want to print something like:
var = 999 <shadowed: other-file.c:1>
foo = 888 <shadowed: test.c:10>
or something like that...
Thanks,
Andrew
>
> In case of rust language it is possible to declare variable with same name
> many times so in this case just print first instance of variaable.
>
> gdb/ChangeLog:
> 2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
>
> * printcmd.c (print_variable_and_value): Print
> <shadowed: decl at line #> after the value of
> super-block variable in case of variable shadowing.
> * stack.c (iterate_over_block_locals): Detect variable shadowing
> using vector to track the inner block variables.
> (iterate_over_block_local_vars): Declare unordered_set type object
> and pass it to function 'iterate_over_block_locals'.
> (do_print_variable_and_value): Add Boolean variable in call back
> function to indicate variable shadowing.
> * stack.h (iterate_over_block_arg_local_vars_cb) : Ditto.
> * tracepoint.c (do_collect_symbol): Ditto.
> * value.h (print_variable_and_value): Add Boolean variable in
> function declaration to indicate variable shadowing.
>
> gdb/doc/ChangeLog:
> 2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
>
> * gdb.texinfo: Update with shadowed variable line info.
>
> gdb/testsuite/ChangeLog:
> 2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
>
> * gdb.opt/inline-locals.exp: Update regex expressions for testing
> 'info locals' command output to handle <shadowed: decl at line #>
> annotation.
> * gdb.opt/inline-locals.c: Add new comments to refer lines.
> * gdb.base/var-shadowing.c: New file.
> * gdb.base/var-shadowing.exp: New file.
> * gdb.rust/var_reuse.exp: New file.
> * gdb.rust/var_reuse.rs: New file.
>
> 2021-12-29 Abdul Basit Ijaz <abdul.b.ijaz@intel.com>
> ---
> gdb/doc/gdb.texinfo | 12 ++++
> gdb/printcmd.c | 15 ++++-
> gdb/stack.c | 41 ++++++++++--
> gdb/stack.h | 3 +-
> gdb/testsuite/gdb.base/var-shadowing.c | 52 +++++++++++++++
> gdb/testsuite/gdb.base/var-shadowing.exp | 85 ++++++++++++++++++++++++
> gdb/testsuite/gdb.opt/inline-locals.c | 2 +-
> gdb/testsuite/gdb.opt/inline-locals.exp | 8 ++-
> gdb/testsuite/gdb.rust/var_reuse.exp | 33 +++++++++
> gdb/testsuite/gdb.rust/var_reuse.rs | 20 ++++++
> gdb/tracepoint.c | 3 +-
> gdb/value.h | 3 +-
> 12 files changed, 263 insertions(+), 14 deletions(-)
> create mode 100755 gdb/testsuite/gdb.base/var-shadowing.c
> create mode 100755 gdb/testsuite/gdb.base/var-shadowing.exp
> create mode 100755 gdb/testsuite/gdb.rust/var_reuse.exp
> create mode 100755 gdb/testsuite/gdb.rust/var_reuse.rs
>
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 7da6805f80..83994c3486 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -8590,6 +8590,18 @@ The optional flag @samp{-q}, which stands for @samp{quiet}, disables
> printing header information and messages explaining why no local variables
> have been printed.
>
> +@smallexample
> +1: int x = 3;
> +2: @{
> +3: int x = 4; // breakpt
> +4: @}
> +(gdb) info locals
> +x = 4
> +x = 3 <shadowed: decl at line 1>
> +@end smallexample
> +
> +If a variable is shadowed, the declaration line is also printed.
> +
> @item info locals [-q] [-t @var{type_regexp}] [@var{regexp}]
> Like @kbd{info locals}, but only print the local variables selected
> with the provided regexp(s).
> diff --git a/gdb/printcmd.c b/gdb/printcmd.c
> index e408b19db6..5aea39370f 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -2387,7 +2387,8 @@ clear_dangling_display_expressions (struct objfile *objfile)
> void
> print_variable_and_value (const char *name, struct symbol *var,
> struct frame_info *frame,
> - struct ui_file *stream, int indent)
> + struct ui_file *stream, int indent,
> + bool shadowed)
> {
>
> if (!name)
> @@ -2410,6 +2411,18 @@ print_variable_and_value (const char *name, struct symbol *var,
> opts.deref_ref = 1;
> common_val_print_checked (val, stream, indent, &opts, current_language);
>
> + /* Print <shadowed: decl at line %line> after the variable value only
> + when it is variable shadowing case. */
> + if (shadowed)
> + {
> + if (var->line > 0)
> + fprintf_styled (stream, metadata_style.style (),
> + _("\t<shadowed: decl at line %d>"), var->line);
> + else
> + fprintf_styled (stream, metadata_style.style (),
> + _("\t<shadowed: decl line not available>"));
> + }
> +
> /* common_val_print invalidates FRAME when a pretty printer calls inferior
> function. */
> frame = NULL;
> diff --git a/gdb/stack.c b/gdb/stack.c
> index 66517cf186..049a4455ed 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -55,6 +55,7 @@
> #include "gdbsupport/def-vector.h"
> #include "cli/cli-option.h"
> #include "cli/cli-style.h"
> +#include <unordered_set>
>
> /* The possible choices of "set print frame-arguments", and the value
> of this setting. */
> @@ -2243,13 +2244,17 @@ backtrace_command_completer (struct cmd_list_element *ignore,
> static void
> iterate_over_block_locals (const struct block *b,
> iterate_over_block_arg_local_vars_cb cb,
> - void *cb_data)
> + void *cb_data,
> + std::unordered_set<std::string> &collected_vars)
> {
> struct block_iterator iter;
> struct symbol *sym;
>
> ALL_BLOCK_SYMBOLS (b, iter, sym)
> {
> + const char *name = sym->print_name ();
> + bool already_collected
> + = collected_vars.find (name) != collected_vars.end ();
> switch (SYMBOL_CLASS (sym))
> {
> case LOC_CONST:
> @@ -2262,7 +2267,28 @@ iterate_over_block_locals (const struct block *b,
> break;
> if (SYMBOL_DOMAIN (sym) == COMMON_BLOCK_DOMAIN)
> break;
> - (*cb) (sym->print_name (), sym, cb_data);
> + /* Only for C/C++/Fortran languages, in case of variables shadowing
> + print <shadowed: decl at line #> annotation after the superblock
> + variable. Iteration of block starts from inner block so
> + collected_vars variable keeps track of the variables in the
> + innerblock. */
> + if ((current_language->la_language == language_c
> + || current_language->la_language == language_cplus
> + || current_language->la_language == language_fortran)
> + && already_collected)
> + (*cb) (name, sym, cb_data, true);
> + /* In case of rust language it is possible to declare variable with
> + same name multiple times and only latest declaration of variable
> + is accessible. So print only the first instance and there is no
> + need of printing duplicates. */
> + else if (current_language->la_language == language_rust
> + && already_collected)
> + break;
> + else
> + {
> + collected_vars.insert (name);
> + (*cb) (name, sym, cb_data, false);
> + }
> break;
>
> default:
> @@ -2280,9 +2306,10 @@ iterate_over_block_local_vars (const struct block *block,
> iterate_over_block_arg_local_vars_cb cb,
> void *cb_data)
> {
> + std::unordered_set<std::string> collected_vars;
> while (block)
> {
> - iterate_over_block_locals (block, cb, cb_data);
> + iterate_over_block_locals (block, cb, cb_data, collected_vars);
> /* After handling the function's top-level block, stop. Don't
> continue to its superblock, the block of per-file
> symbols. */
> @@ -2310,7 +2337,8 @@ struct print_variable_and_value_data
> static void
> do_print_variable_and_value (const char *print_name,
> struct symbol *sym,
> - void *cb_data)
> + void *cb_data,
> + bool shadowed)
> {
> struct print_variable_and_value_data *p
> = (struct print_variable_and_value_data *) cb_data;
> @@ -2332,7 +2360,8 @@ do_print_variable_and_value (const char *print_name,
> return;
> }
>
> - print_variable_and_value (print_name, sym, frame, p->stream, p->num_tabs);
> + print_variable_and_value (print_name, sym, frame, p->stream,
> + p->num_tabs, shadowed);
>
> /* print_variable_and_value invalidates FRAME. */
> frame = NULL;
> @@ -2520,7 +2549,7 @@ iterate_over_block_arg_vars (const struct block *b,
>
> sym2 = lookup_symbol_search_name (sym->search_name (),
> b, VAR_DOMAIN).symbol;
> - (*cb) (sym->print_name (), sym2, cb_data);
> + (*cb) (sym->print_name (), sym2, cb_data, false);
> }
> }
> }
> diff --git a/gdb/stack.h b/gdb/stack.h
> index 17d4119988..b063cb6348 100644
> --- a/gdb/stack.h
> +++ b/gdb/stack.h
> @@ -32,7 +32,8 @@ gdb::unique_xmalloc_ptr<char> find_frame_funname (struct frame_info *frame,
>
> typedef void (*iterate_over_block_arg_local_vars_cb) (const char *print_name,
> struct symbol *sym,
> - void *cb_data);
> + void *cb_data,
> + bool shadowed);
>
> void iterate_over_block_arg_vars (const struct block *block,
> iterate_over_block_arg_local_vars_cb cb,
> diff --git a/gdb/testsuite/gdb.base/var-shadowing.c b/gdb/testsuite/gdb.base/var-shadowing.c
> new file mode 100755
> index 0000000000..6363821dba
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/var-shadowing.c
> @@ -0,0 +1,52 @@
> +/* Copyright (C) 2021 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/>. */
> +
> +/* This is only ever run if it is compiled with a new-enough GCC, but
> + we don't want the compilation to fail if compiled by some other
> + compiler. */
> +
> +#include <stdlib.h>
> +
> +void
> +shadowing ()
> +{
> + int a;
> + unsigned int val1 = 1; /* val1-d1 */
> + unsigned int val2 = 2; /* val2-d1 */
> + a = 101; /* bp for locals 1 */
> + {
> + unsigned int val2 = 3; /* val2-d2 */
> + unsigned int val3 = 4; /* val3-d1 */
> + a = 102; /* bp for locals 2 */
> + {
> + unsigned int val1 = 5; /* val1-d2 */
> + a = 103; /* bp for locals 3 */
> + {
> + unsigned int val1 = 6;
> + unsigned int val2 = 7;
> + unsigned int val3 = 8;
> + a = 104; /* bp for locals 4 */
> + }
> + }
> + }
> + a = 0; /* bp for locals 5 */
> +}
> +
> +int
> +main (void)
> +{
> + shadowing ();
> + return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/var-shadowing.exp b/gdb/testsuite/gdb.base/var-shadowing.exp
> new file mode 100755
> index 0000000000..b43f5bfc7c
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/var-shadowing.exp
> @@ -0,0 +1,85 @@
> +# Copyright 2021 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/>.
> +
> +standard_testfile
> +if [prepare_for_testing "failed to prepare" $testfile $srcfile] {
> + return -1
> +}
> +
> +if ![runto_main] {
> + untested "failed to run to main"
> + return -1
> +}
> +
> +set bp_line1 [gdb_get_line_number "bp for locals 1" ${srcfile}]
> +set bp_line2 [gdb_get_line_number "bp for locals 2" ${srcfile}]
> +set bp_line3 [gdb_get_line_number "bp for locals 3" ${srcfile}]
> +set bp_line4 [gdb_get_line_number "bp for locals 4" ${srcfile}]
> +set bp_line5 [gdb_get_line_number "bp for locals 5" ${srcfile}]
> +
> +set val1_d1 [gdb_get_line_number "val1-d1" ${srcfile}]
> +set val1_d2 [gdb_get_line_number "val1-d2" ${srcfile}]
> +set val2_d1 [gdb_get_line_number "val2-d1" ${srcfile}]
> +set val2_d2 [gdb_get_line_number "val2-d2" ${srcfile}]
> +set val3_d1 [gdb_get_line_number "val3-d1" ${srcfile}]
> +
> +gdb_breakpoint $srcfile:$bp_line1
> +gdb_test "continue" ".*bp for locals 1.*" "continue to outermost level"
> +gdb_test "info locals" [multi_line \
> + "val1 = 1" \
> + "val2 = 2" \
> + ] "info locals at outermost level"
> +
> +gdb_breakpoint $srcfile:$bp_line2
> +gdb_test "continue" ".*bp for locals 2.*" "continue to first level"
> +gdb_test "info locals" [multi_line \
> + "val2 = 3" \
> + "val3 = 4" \
> + "a = 101" \
> + "val1 = 1" \
> + "val2 = 2\t<shadowed: decl at line $val2_d1>" \
> + ] "info locals first level"
> +
> +gdb_breakpoint $srcfile:$bp_line3
> +gdb_test "continue" ".*bp for locals 3.*" "continue to second level"
> +gdb_test "info locals" [multi_line \
> + "val1 = 5" \
> + "val2 = 3" \
> + "val3 = 4" \
> + "a = 102" \
> + "val1 = 1\t<shadowed: decl at line $val1_d1>" \
> + "val2 = 2\t<shadowed: decl at line $val2_d1>" \
> + ] "info locals second level"
> +
> +gdb_breakpoint $srcfile:$bp_line4
> +gdb_test "continue" ".*bp for locals 4.*" "continue to innermost level"
> +gdb_test "info locals" [multi_line \
> + "val1 = 6" \
> + "val2 = 7" \
> + "val3 = 8" \
> + "val1 = 5\t<shadowed: decl at line $val1_d2>" \
> + "val2 = 3\t<shadowed: decl at line $val2_d2>" \
> + "val3 = 4\t<shadowed: decl at line $val3_d1>" \
> + "a = 103" \
> + "val1 = 1\t<shadowed: decl at line $val1_d1>" \
> + "val2 = 2\t<shadowed: decl at line $val2_d1>" \
> + ] "info locals at innermost level"
> +
> +gdb_breakpoint $srcfile:$bp_line5
> +gdb_test "continue" ".*bp for locals 5.*" "continue to outermost level last"
> +gdb_test "info locals" [multi_line \
> + "val1 = 1" \
> + "val2 = 2" \
> + ] "info locals at outermost level last"
> diff --git a/gdb/testsuite/gdb.opt/inline-locals.c b/gdb/testsuite/gdb.opt/inline-locals.c
> index f8910dadca..cf5a96ad97 100644
> --- a/gdb/testsuite/gdb.opt/inline-locals.c
> +++ b/gdb/testsuite/gdb.opt/inline-locals.c
> @@ -39,7 +39,7 @@ init_array (int *array, int n)
>
> inline ATTR int func1(int arg1)
> {
> - int array[64];
> + int array[64]; /* array-d1 */
> init_array (array, 64);
> array_p = array;
> array[0] = result;
> diff --git a/gdb/testsuite/gdb.opt/inline-locals.exp b/gdb/testsuite/gdb.opt/inline-locals.exp
> index ff9dd1b49e..04b8b9cd8e 100644
> --- a/gdb/testsuite/gdb.opt/inline-locals.exp
> +++ b/gdb/testsuite/gdb.opt/inline-locals.exp
> @@ -29,6 +29,8 @@ if { [skip_inline_var_tests] } {
> return
> }
>
> +set array_d1 [gdb_get_line_number "array-d1" ${srcfile}]
> +
> set no_frames [skip_inline_frame_tests]
>
> set line1 [gdb_get_line_number "set breakpoint 1 here" ${srcfile2}]
> @@ -45,7 +47,7 @@ if { ! $no_frames } {
> gdb_test "info frame" ".*inlined into frame.*" "func1 inlined 2"
> set pass_re "array = \\{0 <repeats 64 times>\\}"
> set kfail_re [multi_line $pass_re \
> - "array = <optimized out>"]
> + "array = <optimized out>\t<shadowed: decl at line $array_d1>"]
> gdb_test_multiple "info locals" "info locals above bar 2" {
> -re -wrap $pass_re {
> pass $gdb_test_name
> @@ -94,7 +96,7 @@ if { ! $no_frames } {
> gdb_test "info frame" ".*inlined into frame.*" "func1 inlined 3"
> set pass_re "array = {$decimal, \[^\r\n\]*}"
> set kfail_re [multi_line $pass_re \
> - "array = <optimized out>"]
> + "array = <optimized out>\t<shadowed: decl at line $array_d1>"]
> gdb_test_multiple "info locals" "info locals above bar 3" {
> -re -wrap $pass_re {
> pass $gdb_test_name
> @@ -134,7 +136,7 @@ proc check_scoped_locals {bp_label pass_re} {
> gdb_breakpoint $srcfile:$locals_bp
>
> gdb_continue_to_breakpoint "$bp_label" ".*$srcfile:$locals_bp.*"
> - set kfail_re [multi_line $pass_re ".*<optimized out>"]
> + set kfail_re [multi_line $pass_re ".*<optimized out>\t(<shadowed: decl at line .*>)?"]
> gdb_test_multiple "info locals" "scoped info locals at $bp_label" {
> -re -wrap $pass_re {
> pass $gdb_test_name
> diff --git a/gdb/testsuite/gdb.rust/var_reuse.exp b/gdb/testsuite/gdb.rust/var_reuse.exp
> new file mode 100755
> index 0000000000..c535aaa759
> --- /dev/null
> +++ b/gdb/testsuite/gdb.rust/var_reuse.exp
> @@ -0,0 +1,33 @@
> +# Copyright 2021 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 rust-support.exp
> +if {[skip_rust_tests]} {
> + continue
> +}
> +
> +standard_testfile .rs
> +if {[prepare_for_testing "failed to prepare" \
> + $testfile $srcfile {debug rust}]} {
> + return -1
> +}
> +
> +set line [gdb_get_line_number "set breakpoint here"]
> +if {![runto ${srcfile}:$line]} {
> + untested "could not run to breakpoint"
> + return -1
> +}
> +
> +gdb_test "info local _x" "_x = 12" "print local _x variable"
> diff --git a/gdb/testsuite/gdb.rust/var_reuse.rs b/gdb/testsuite/gdb.rust/var_reuse.rs
> new file mode 100755
> index 0000000000..63d165c4aa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.rust/var_reuse.rs
> @@ -0,0 +1,20 @@
> +// Copyright (C) 2021 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/>.
> +
> +fn main() {
> + let _x = 5;
> + let _x = _x + 7;
> + let _y = 8; // set breakpoint here
> +}
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index 15506fdbf4..12292a3b6d 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -1059,7 +1059,8 @@ struct add_local_symbols_data
> static void
> do_collect_symbol (const char *print_name,
> struct symbol *sym,
> - void *cb_data)
> + void *cb_data,
> + bool shadowed)
> {
> struct add_local_symbols_data *p = (struct add_local_symbols_data *) cb_data;
>
> diff --git a/gdb/value.h b/gdb/value.h
> index 6fa57e0da6..781d1eb546 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -1118,7 +1118,8 @@ extern void print_variable_and_value (const char *name,
> struct symbol *var,
> struct frame_info *frame,
> struct ui_file *stream,
> - int indent);
> + int indent,
> + bool shadowed);
>
> extern void typedef_print (struct type *type, struct symbol *news,
> struct ui_file *stream);
> --
> 2.31.1
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] gdb: add annotation in 'info locals' command for variables shadowing case
2022-01-05 15:20 ` Andrew Burgess
@ 2022-01-05 15:55 ` Pedro Alves
2022-01-05 16:26 ` Andrew Burgess
0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2022-01-05 15:55 UTC (permalink / raw)
To: Andrew Burgess, abdul.b.ijaz; +Cc: gdb-patches
On 2022-01-05 15:20, Andrew Burgess via Gdb-patches wrote:
> (gdb) info locals
> var = 123
> foo = 456
> var = 999 <shadowed: decl at line 1>
> foo = 888 <shadowed: decl at line 10>
> (gdb)
>
> As you can see the information for var is not helpful. I suspect we
> might want to print something like:
>
> var = 999 <shadowed: other-file.c:1>
> foo = 888 <shadowed: test.c:10>
I just read this and my first impression from:
shadowed: test.c:10
... without thinking much was that the ":" after "shadowed" meant "shadowed by". It's a super
minor thing, but a colon or semicolon instead might drop that ambiguity, without reintroducing
the wordier "decl at":
foo = 888 <shadowed, test.c:10>
Also, instead of printing extra info just for the shadowed ones, would it be useful to
print the disambiguation info for all variable names that are ambiguous?
In that case, maybe put "shadowed" at the end, as most frequently the variables are
shadowed in the same file, so you get better alignment. Like:
var = 123 <file.c:1>
var = 999 <file.c:10, shadowed>
Just some 2c. I think the feature is quite useful, whatever the finer details end up as.
Thanks for working on it.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] gdb: add annotation in 'info locals' command for variables shadowing case
2022-01-05 15:55 ` Pedro Alves
@ 2022-01-05 16:26 ` Andrew Burgess
2022-01-11 8:55 ` Ijaz, Abdul B
0 siblings, 1 reply; 8+ messages in thread
From: Andrew Burgess @ 2022-01-05 16:26 UTC (permalink / raw)
To: Pedro Alves; +Cc: abdul.b.ijaz, gdb-patches
* Pedro Alves <pedro@palves.net> [2022-01-05 15:55:05 +0000]:
> On 2022-01-05 15:20, Andrew Burgess via Gdb-patches wrote:
> > (gdb) info locals
> > var = 123
> > foo = 456
> > var = 999 <shadowed: decl at line 1>
> > foo = 888 <shadowed: decl at line 10>
> > (gdb)
> >
> > As you can see the information for var is not helpful. I suspect we
> > might want to print something like:
> >
> > var = 999 <shadowed: other-file.c:1>
> > foo = 888 <shadowed: test.c:10>
>
> I just read this and my first impression from:
>
> shadowed: test.c:10
>
> ... without thinking much was that the ":" after "shadowed" meant "shadowed by". It's a super
> minor thing, but a colon or semicolon instead might drop that ambiguity, without reintroducing
> the wordier "decl at":
>
> foo = 888 <shadowed, test.c:10>
>
> Also, instead of printing extra info just for the shadowed ones, would it be useful to
> print the disambiguation info for all variable names that are ambiguous?
> In that case, maybe put "shadowed" at the end, as most frequently the variables are
> shadowed in the same file, so you get better alignment. Like:
>
> var = 123 <file.c:1>
> var = 999 <file.c:10, shadowed>
Oh, I like this. This one please.
Then in the MI output we'd have something like:
^done,locals=[{name="var",type="int",value="132",file="file.c",line="1"},
{name="var",type="int",value="999",file="file.c",line="10",shadowed="True"}]
>
> Just some 2c. I think the feature is quite useful, whatever the finer details end up as.
> Thanks for working on it.
Agree this seems like a great addition.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH v2 1/2] gdb: add annotation in 'info locals' command for variables shadowing case
2022-01-05 16:26 ` Andrew Burgess
@ 2022-01-11 8:55 ` Ijaz, Abdul B
0 siblings, 0 replies; 8+ messages in thread
From: Ijaz, Abdul B @ 2022-01-11 8:55 UTC (permalink / raw)
To: Andrew Burgess, Pedro Alves, Philippe Waroquiers
Cc: abdul.b.ijaz, gdb-patches, Aktemur, Tankut Baris
Hello Andrew and Pedro and Philippe,
Thanks for the feedback. I will update the next patch accordingly to shows shadowed information like your feedback. Just want to mention here with this change this would be quite impact on the testsuite since for this change we will be adding file and line info by default for info locals / -stack-list-locals/variables etc commands so a lot of tests will be updated accordingly.
> var = 123 <file.c:1>
> var = 999 <file.c:10, shadowed>
> ^done,locals=[{name="var",type="int",value="132",file="file.c",line="1"},
> {name="var",type="int",value="999",file="file.c",line="10",shadowed="True"}]
Thanks & Best Regards
Abdul Basit
-----Original Message-----
From: Andrew Burgess <aburgess@redhat.com>
Sent: Wednesday, January 5, 2022 5:27 PM
To: Pedro Alves <pedro@palves.net>
Cc: abdul.b.ijaz <abijaz@ecsmtp.iul.intel.com>; gdb-patches@sourceware.org
Subject: Re: [PATCH v2 1/2] gdb: add annotation in 'info locals' command for variables shadowing case
* Pedro Alves <pedro@palves.net> [2022-01-05 15:55:05 +0000]:
> On 2022-01-05 15:20, Andrew Burgess via Gdb-patches wrote:
> > (gdb) info locals
> > var = 123
> > foo = 456
> > var = 999 <shadowed: decl at line 1>
> > foo = 888 <shadowed: decl at line 10>
> > (gdb)
> >
> > As you can see the information for var is not helpful. I suspect we
> > might want to print something like:
> >
> > var = 999 <shadowed: other-file.c:1>
> > foo = 888 <shadowed: test.c:10>
>
> I just read this and my first impression from:
>
> shadowed: test.c:10
>
> ... without thinking much was that the ":" after "shadowed" meant
> "shadowed by". It's a super minor thing, but a colon or semicolon
> instead might drop that ambiguity, without reintroducing the wordier "decl at":
>
> foo = 888 <shadowed, test.c:10>
>
> Also, instead of printing extra info just for the shadowed ones, would
> it be useful to print the disambiguation info for all variable names that are ambiguous?
> In that case, maybe put "shadowed" at the end, as most frequently the
> variables are shadowed in the same file, so you get better alignment. Like:
>
> var = 123 <file.c:1>
> var = 999 <file.c:10, shadowed>
Oh, I like this. This one please.
Then in the MI output we'd have something like:
^done,locals=[{name="var",type="int",value="132",file="file.c",line="1"},
{name="var",type="int",value="999",file="file.c",line="10",shadowed="True"}]
>
> Just some 2c. I think the feature is quite useful, whatever the finer details end up as.
> Thanks for working on it.
Agree this seems like a great addition.
Thanks,
Andrew
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-11 8:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-29 11:14 [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case abdul.b.ijaz
2021-12-29 11:14 ` [PATCH v2 1/2] gdb: " abdul.b.ijaz
2022-01-05 15:20 ` Andrew Burgess
2022-01-05 15:55 ` Pedro Alves
2022-01-05 16:26 ` Andrew Burgess
2022-01-11 8:55 ` Ijaz, Abdul B
2021-12-29 11:14 ` [PATCH v2 2/2] gdb: add shadowed_loc field in '-stack-list-locals/variables' mi commands abdul.b.ijaz
2022-01-01 20:14 ` [PATCH v2 0/2] add annotation in 'info locals' command for variables shadowing case Philippe Waroquiers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).