Show optimized out local variables in "info locals"

Message ID 1510679293-8244-1-git-send-email-simon.marchi@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi Nov. 14, 2017, 5:08 p.m. UTC
  Currently, optimized out variables are not shown when doing "info
locals".  Some users found that confusing, thinking GDB forgot to print
their variable.  This patch adds them to the "info locals" output.  I
added a test in gdb.dwarf2 to test for that behavior.  I think doing a
synthetic DWARF test is the easiest way to have an optimized out local
variable for sure.

However, this change reveals what I think is a bug in GDB, see:

http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2017-September/004394.html

This patch marks the tests in inline-locals.exp that start failing as
KFAIL.  I'd like to tackle this bug eventually, but I don't have the
time right now.  I think it's still better to show an extra erroneous
entry than to not show the optimized out variables at all.  I haven't
created a bug in bugzilla yet, but if we agree it's indeed a bug,  I'll
create one and update the setup_kfail lines with the actual bug number
before pushing.

gdb/ChangeLog:

	* stack.c (iterate_over_block_locals): Add LOC_OPTIMIZED_OUT
	case in switch.

gdb/testsuite/ChangeLog:

	* gdb.opt/inline-locals.exp: Mark tests as KFAIL.
	* gdb.dwarf2/info-locals-optimized-out.exp: New file.
	* gdb.dwarf2/info-locals-optimized-out.c: New file.
---
 gdb/stack.c                                        |  1 +
 .../gdb.dwarf2/info-locals-optimized-out.c         | 23 +++++++
 .../gdb.dwarf2/info-locals-optimized-out.exp       | 74 ++++++++++++++++++++++
 gdb/testsuite/gdb.opt/inline-locals.exp            |  2 +
 4 files changed, 100 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.exp
  

Comments

Yao Qi Nov. 14, 2017, 10:44 p.m. UTC | #1
On Tue, Nov 14, 2017 at 5:08 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
>
> However, this change reveals what I think is a bug in GDB, see:
>
> http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2017-September/004394.html
>

IMO, it is not necessary to emit DW_TAG_lexical_block in concrete instances.
See comment #4 in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37801
At least, looks gcc generates unnecessary debug information, and we need
to fix GCC somehow.

Whether it is a bug in GDB or not, I don't know.  The answer depends on it is
*unnecessary* or *wrong* to have DW_TAG_lexical_block in concrete instances.
  
Simon Marchi Nov. 15, 2017, 4:54 a.m. UTC | #2
On 2017-11-14 05:44 PM, Yao Qi wrote:
> On Tue, Nov 14, 2017 at 5:08 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
>>
>> However, this change reveals what I think is a bug in GDB, see:
>>
>> http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2017-September/004394.html
>>
> 
> IMO, it is not necessary to emit DW_TAG_lexical_block in concrete instances.
> See comment #4 in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37801
> At least, looks gcc generates unnecessary debug information, and we need
> to fix GCC somehow.
> 
> Whether it is a bug in GDB or not, I don't know.  The answer depends on it is
> *unnecessary* or *wrong* to have DW_TAG_lexical_block in concrete instances.

I had an email discussion with some gcc developers (Nathan Sidwell, Richard Biener,
Jason Merill) after a chat on IRC.  Unfortunately, they answered privately so it's
not on dwarf-discuss.  I'll try to update the thread on dwarf-discuss with their
answers tomorrow, for future reference.  But the gist of it was:

Richard said:

> I think the lexical block is just the function scope itself and the inliner
> inserts this BLOCK which then corresponds to the DW_TAG_inlined_subroutine.
> I suppose we should avoid emitting that BLOCK itself as a DW_TAG_lexical_block
> but use the emitted DW_TAG_inlined_subroutine for that.
>
> Not sure if I remember the details correctly.
>
> I don't think the DWARF is invalid btw, with early LTO debug we have plenty of
> abstract origins where source and destination context don't match 1:1.  We're
> just using it as a "get some more info from this DIE" link which I think is
> all that is documented as semantics (though the 'inline' term pops up too
> often there and the relation to DW_AT_specification is unclear to me though
> the latter is restricted to DW_TAG_subroutine AFAIR).

Jason said (replying to Richard):

>> I think the lexical block is just the function scope itself and the inliner
>> inserts this BLOCK which then corresponds to the DW_TAG_inlined_subroutine.
>> I suppose we should avoid emitting that BLOCK itself as a DW_TAG_lexical_block
>> but use the emitted DW_TAG_inlined_subroutine for that.
>
> Agreed.  It's curious that we would generate the lexical block in the
> inlined instance and not the abstract.
>
>>  I don't think the DWARF is invalid btw, with early LTO debug we have plenty of
>>  abstract origins where source and destination context don't match 1:1.  We're
>>  just using it as a "get some more info from this DIE" link which I think is
>>  all that is documented as semantics (though the 'inline' term pops up too
>>  often there and the relation to DW_AT_specification is unclear to me though
>>  the latter is restricted to DW_TAG_subroutine AFAIR).
>
> Also agreed, GDB ought to be able to handle this situation.
>
> So, bugs on both sides...

So even though there might be something to fix in GCC, I think we'll have to handle
the current case in GDB as well.

Simon
  
Simon Marchi Nov. 22, 2017, 8:57 p.m. UTC | #3
On 2017-11-14 11:54 PM, Simon Marchi wrote:
> On 2017-11-14 05:44 PM, Yao Qi wrote:
>> On Tue, Nov 14, 2017 at 5:08 PM, Simon Marchi <simon.marchi@ericsson.com> wrote:
>>>
>>> However, this change reveals what I think is a bug in GDB, see:
>>>
>>> http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2017-September/004394.html
>>>
>>
>> IMO, it is not necessary to emit DW_TAG_lexical_block in concrete instances.
>> See comment #4 in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37801
>> At least, looks gcc generates unnecessary debug information, and we need
>> to fix GCC somehow.
>>
>> Whether it is a bug in GDB or not, I don't know.  The answer depends on it is
>> *unnecessary* or *wrong* to have DW_TAG_lexical_block in concrete instances.
> 
> I had an email discussion with some gcc developers (Nathan Sidwell, Richard Biener,
> Jason Merill) after a chat on IRC.  Unfortunately, they answered privately so it's
> not on dwarf-discuss.  I'll try to update the thread on dwarf-discuss with their
> answers tomorrow, for future reference.  But the gist of it was:
> 
> Richard said:
> 
>> I think the lexical block is just the function scope itself and the inliner
>> inserts this BLOCK which then corresponds to the DW_TAG_inlined_subroutine.
>> I suppose we should avoid emitting that BLOCK itself as a DW_TAG_lexical_block
>> but use the emitted DW_TAG_inlined_subroutine for that.
>>
>> Not sure if I remember the details correctly.
>>
>> I don't think the DWARF is invalid btw, with early LTO debug we have plenty of
>> abstract origins where source and destination context don't match 1:1.  We're
>> just using it as a "get some more info from this DIE" link which I think is
>> all that is documented as semantics (though the 'inline' term pops up too
>> often there and the relation to DW_AT_specification is unclear to me though
>> the latter is restricted to DW_TAG_subroutine AFAIR).
> 
> Jason said (replying to Richard):
> 
>>> I think the lexical block is just the function scope itself and the inliner
>>> inserts this BLOCK which then corresponds to the DW_TAG_inlined_subroutine.
>>> I suppose we should avoid emitting that BLOCK itself as a DW_TAG_lexical_block
>>> but use the emitted DW_TAG_inlined_subroutine for that.
>>
>> Agreed.  It's curious that we would generate the lexical block in the
>> inlined instance and not the abstract.
>>
>>>  I don't think the DWARF is invalid btw, with early LTO debug we have plenty of
>>>  abstract origins where source and destination context don't match 1:1.  We're
>>>  just using it as a "get some more info from this DIE" link which I think is
>>>  all that is documented as semantics (though the 'inline' term pops up too
>>>  often there and the relation to DW_AT_specification is unclear to me though
>>>  the latter is restricted to DW_TAG_subroutine AFAIR).
>>
>> Also agreed, GDB ought to be able to handle this situation.
>>
>> So, bugs on both sides...
> 
> So even though there might be something to fix in GCC, I think we'll have to handle
> the current case in GDB as well.
> 
> Simon
> 

I've updated the thread on dwarf-discuss, and pushed the patch.

Thanks,

Simon
  

Patch

diff --git a/gdb/stack.c b/gdb/stack.c
index 81032fc..6bd0d45 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1922,6 +1922,7 @@  iterate_over_block_locals (const struct block *b,
 	case LOC_REGISTER:
 	case LOC_STATIC:
 	case LOC_COMPUTED:
+	case LOC_OPTIMIZED_OUT:
 	  if (SYMBOL_IS_ARGUMENT (sym))
 	    break;
 	  if (SYMBOL_DOMAIN (sym) == COMMON_BLOCK_DOMAIN)
diff --git a/gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.c b/gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.c
new file mode 100644
index 0000000..e3ae4cd
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.c
@@ -0,0 +1,23 @@ 
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2017 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/>.  */
+
+int
+main (void)
+{
+  asm ("main_label: .globl main_label");
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.exp b/gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.exp
new file mode 100644
index 0000000..dd0a99b
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/info-locals-optimized-out.exp
@@ -0,0 +1,74 @@ 
+# Copyright 2017 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/>.
+
+# Test that "info locals" shows optimized out variables.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .c ${gdb_test_file_name}-dw.S
+
+# Make some DWARF for the test.
+
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcdir subdir srcfile
+    global dwarf_regnum regname
+
+    set buf_var [gdb_target_symbol buf]
+
+    cu {} {
+	DW_TAG_compile_unit {
+		{DW_AT_name info-locals-optimized-out.c}
+		{DW_AT_comp_dir /tmp}
+	} {
+	    declare_labels int_type_label
+
+	    # int
+	    int_type_label: base_type {
+		{name "int"}
+		{encoding @DW_ATE_signed}
+		{byte_size 4 DW_FORM_sdata}
+	    }
+
+	    DW_TAG_subprogram {
+		{MACRO_AT_func { main ${srcdir}/${subdir}/${srcfile} }}
+		{DW_AT_external 1 flag}
+	    } {
+		# A variable completely optimized out.
+		DW_TAG_variable {
+		    {name "opt_out"}
+		    {type :$int_type_label}
+		}
+	    }
+	}
+    }
+}
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} \
+	  [list $srcfile $asm_file] {nodebug}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Make sure "info locals" shows optimized out variables.
+gdb_test "info locals" ".*opt_out = <optimized out>.*" "info local shows optimized out variable"
diff --git a/gdb/testsuite/gdb.opt/inline-locals.exp b/gdb/testsuite/gdb.opt/inline-locals.exp
index 76463a6..7245df7 100644
--- a/gdb/testsuite/gdb.opt/inline-locals.exp
+++ b/gdb/testsuite/gdb.opt/inline-locals.exp
@@ -43,6 +43,7 @@  if { ! $no_frames } {
 	"backtrace from bar 2"
     gdb_test "up" "#1  .*func1 .* at .*" "up from bar 2"
     gdb_test "info frame" ".*inlined into frame.*" "func1 inlined 2"
+    setup_kfail "gdb/xyz" *-*-*
     gdb_test "info locals" "array = {.*}" "info locals above bar 2"
 
     set msg "info args above bar 2"
@@ -82,6 +83,7 @@  if { ! $no_frames } {
 	"backtrace from bar 3"
     gdb_test "up" "#1  .*func1 .* at .*" "up from bar 3"
     gdb_test "info frame" ".*inlined into frame.*" "func1 inlined 3"
+    setup_kfail "gdb/xyz" *-*-*
     gdb_test "info locals" "array = {.*}" "info locals above bar 3"
 
     set msg "info args above bar 3"