Permit DW_OP_GNU_uninit to be used with DW_OP_piece
Commit Message
This commit implements a fix for a bug reported against GDB on
Fedora bugzilla...
https://bugzilla.redhat.com/show_bug.cgi?id=2166796
The test case in that bug report involved running gdb against the 'jq'
program (which is a command-line JSON processor) on Fedora 37. Since
the debug info is compiler (and compile-time option) dependent, it
won't necessarily show up in other distributions or even past or
future versions of Fedora. (E.g. when trying the example shown below
on Fedora 38, GDB says that the value of 'value' has been optimized
out. I.e. it does not demonstrate the same DWARF error that can be
see when using Fedora 37.)
That said, on Fedora 37, the bug could be reproduced as follows:
[kev@f37-1 ~]$ gdb jq -q -ex 'b src/util.c:415' -ex 'r </dev/null'
Reading symbols from jq...
This GDB supports auto-downloading debuginfo from the following URLs:
<https://debuginfod.fedoraproject.org/>
Enable debuginfod for this session? (y or [n]) y
Debuginfod has been enabled.
To make this setting permanent, add 'set debuginfod enabled on' to .gdbinit.
Reading symbols from /home/kev/.cache/debuginfod_client/9d3c8b4197350a190a74972d481de32abf641aa4/debuginfo...
No source file named src/util.c.
Make breakpoint pending on future shared library load? (y or [n]) y
Breakpoint 1 (src/util.c:415) pending.
Starting program: /usr/bin/jq </dev/null
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Breakpoint 1, jq_util_input_next_input (state=0x55555555d7f0) at src/util.c:416
416 if (state->parser == NULL) {
(gdb) p value
DWARF-2 expression error: DW_OP_GNU_uninit must always be the very last op.
This is undesirable - rather than output an error about the DWARF
info, we'd prefer to see a value, even if it is uninitialized.
Examination of the debuginfo showed the following:
<1><468f1>: Abbrev Number: 112 (DW_TAG_subprogram)
<468f2> DW_AT_external : 1
<468f2> DW_AT_name : (indirect string, offset: 0x4781): jq_util_input_next_input
<468f6> DW_AT_decl_file : 10
<468f6> DW_AT_decl_line : 411
<468f8> DW_AT_decl_column : 4
<468f9> DW_AT_prototyped : 1
<468f9> DW_AT_type : <0x3f2>
<468fd> DW_AT_sibling : <0x4692e>
...
<2><46921>: Abbrev Number: 102 (DW_TAG_variable)
<46922> DW_AT_name : (indirect string, offset: 0x8cb): value
<46926> DW_AT_decl_file : 10
<46926> DW_AT_decl_line : 414
<46928> DW_AT_decl_column : 6
<46929> DW_AT_type : <0x3f2>
Note that there's no DW_AT_location, so I looked for an abstract origin entry:
<2><2dfa0>: Abbrev Number: 90 (DW_TAG_variable)
<2dfa1> DW_AT_abstract_origin: <0x46921>
<2dfa5> DW_AT_location : 0x27cf1 (location list)
<2dfa9> DW_AT_GNU_locviews: 0x27ce1
(Note that the DW_AT_abstract_origin attribute's value is 0x46921 which
is the DIE for the local variable "value".)
Looking at the location list, I see:
00027cf1 v000000000000000 v000000000000000 views at 00027ce1 for:
000000000002f8fe 000000000002f92e (DW_OP_reg13 (r13); DW_OP_GNU_uninit; DW_OP_piece: 8; DW_OP_reg12 (r12); DW_OP_GNU_uninit; DW_OP_piece: 8)
While DW_OP_GNU_uninit is not the very last op, it is the last op
prior to DW_OP_piece. The fix involved changing the DW_OP_GNU_uninit
case in dwarf_expr_context::execute_stack_op in gdb/dwarf2/expr.c so
that DW_OP_GNU_uninit may appear just before DW_OP_piece.
With the fix in place, attempting to print 'value' now looks like
this:
(gdb) p value
$1 = [uninitialized] {kind_flags = 0 '\000', pad_ = 0 '\000', offset = 0,
size = 0, u = {ptr = 0x0, number = 0}}
Note that "[uninitialized]" is part of the output. (But also note
that there's an extra space character.)
I've made a new test case,
gdb.dwarf2/DW_OP_piece_with_DW_OP_GNU_uninit.exp, by adapting an
existing one, gdb.dwarf2/opt-out-not-implptr.exp. Since it uses the
DWARF assembler, the test case does not depend on a specific compiler
version or compiler options.
Tested on Fedora 37 and Fedora 38.
---
gdb/dwarf2/expr.c | 2 +-
.../DW_OP_piece_with_DW_OP_GNU_uninit.exp | 94 +++++++++++++++++++
2 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 gdb/testsuite/gdb.dwarf2/DW_OP_piece_with_DW_OP_GNU_uninit.exp
Comments
>>>>> "Kevin" == Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> writes:
Kevin> The test case in that bug report involved running gdb against the 'jq'
Kevin> program (which is a command-line JSON processor) on Fedora 37. Since
Kevin> the debug info is compiler (and compile-time option) dependent, it
Kevin> won't necessarily show up in other distributions or even past or
Kevin> future versions of Fedora. (E.g. when trying the example shown below
Kevin> on Fedora 38, GDB says that the value of 'value' has been optimized
Kevin> out. I.e. it does not demonstrate the same DWARF error that can be
Kevin> see when using Fedora 37.)
Ages ago, during one of the rewrites of the DWARF expression evaluator,
I found out that DW_OP_GNU_uninit was not really documented. So, I just
made it continue to work the way it had been described in the gdb
sources.
My recollection is that it required a special compiler argument to even
get gcc to emit it. It's still undocumented as far as I can tell.
So I'd suggest perhaps you could get someone to remove it from GCC, or
from the jq build. It's barely used and IMO not really useful.
That said, it's fine by me if you want to keep it around.
Kevin> case DW_OP_GNU_uninit:
Kevin> - if (op_ptr != op_end)
Kevin> + if (op_ptr != op_end && *op_ptr != DW_OP_piece)
Kevin> error (_("DWARF-2 expression error: DW_OP_GNU_uninit must always "
Kevin> "be the very last op."));
If DW_OP_GNU_uninit can end a piece then the error message here should
be updated.
Also did you look at the other spots that understand DWARF expressions?
There are 3 or 4 in gdb.
Aside from this the patch is OK.
Tom
On Mon, 05 Jun 2023 07:17:08 -0600
Tom Tromey <tom@tromey.com> wrote:
> >>>>> "Kevin" == Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Kevin> The test case in that bug report involved running gdb against the 'jq'
> Kevin> program (which is a command-line JSON processor) on Fedora 37. Since
> Kevin> the debug info is compiler (and compile-time option) dependent, it
> Kevin> won't necessarily show up in other distributions or even past or
> Kevin> future versions of Fedora. (E.g. when trying the example shown below
> Kevin> on Fedora 38, GDB says that the value of 'value' has been optimized
> Kevin> out. I.e. it does not demonstrate the same DWARF error that can be
> Kevin> see when using Fedora 37.)
>
> Ages ago, during one of the rewrites of the DWARF expression evaluator,
> I found out that DW_OP_GNU_uninit was not really documented. So, I just
> made it continue to work the way it had been described in the gdb
> sources.
>
> My recollection is that it required a special compiler argument to even
> get gcc to emit it. It's still undocumented as far as I can tell.
>
> So I'd suggest perhaps you could get someone to remove it from GCC, or
> from the jq build. It's barely used and IMO not really useful.
>
> That said, it's fine by me if you want to keep it around.
So long as there's a producer for it, even if only rarely used,
I think we should keep it around.
>
> Kevin> case DW_OP_GNU_uninit:
> Kevin> - if (op_ptr != op_end)
> Kevin> + if (op_ptr != op_end && *op_ptr != DW_OP_piece)
> Kevin> error (_("DWARF-2 expression error: DW_OP_GNU_uninit must always "
> Kevin> "be the very last op."));
>
> If DW_OP_GNU_uninit can end a piece then the error message here should
> be updated.
I've updated the error message - see below.
> Also did you look at the other spots that understand DWARF expressions?
> There are 3 or 4 in gdb.
I found another spot in gdb/dwarf2/expr.c which deals DW_OP_GNU_uninit.
It has already been updated to handle both DW_OP_piece and DW_OP_bit_piece.
I updated my patch to also handle DW_OP_bit_piece.
The other occurrences of DW_OP_GNU_uninit also didn't need updating.
They were just cases in switch statements.
> Aside from this the patch is OK.
Thanks - I've pushed it.
The test case stayed the same, but this is what the change to
gdb/dwarf2/expr.c ended up looking like:
diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
index ccdd87f3a55..7e1666165d7 100644
--- a/gdb/dwarf2/expr.c
+++ b/gdb/dwarf2/expr.c
@@ -2200,9 +2200,11 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
goto no_push;
case DW_OP_GNU_uninit:
- if (op_ptr != op_end)
+ if (op_ptr != op_end && *op_ptr != DW_OP_piece
+ && *op_ptr != DW_OP_bit_piece)
error (_("DWARF-2 expression error: DW_OP_GNU_uninit must always "
- "be the very last op."));
+ "be the very last op in a DWARF expression or "
+ "DW_OP_piece/DW_OP_bit_piece piece."));
this->m_initialized = false;
goto no_push;
Kevin> The test case stayed the same, but this is what the change to
Kevin> gdb/dwarf2/expr.c ended up looking like:
...
FWIW see dwarf_expr_require_composition
Tom
@@ -2198,7 +2198,7 @@ dwarf_expr_context::execute_stack_op (const gdb_byte *op_ptr,
goto no_push;
case DW_OP_GNU_uninit:
- if (op_ptr != op_end)
+ if (op_ptr != op_end && *op_ptr != DW_OP_piece)
error (_("DWARF-2 expression error: DW_OP_GNU_uninit must always "
"be the very last op."));
new file mode 100644
@@ -0,0 +1,94 @@
+# Copyright 2023 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 use of DW_OP_GNU_uninit with DW_OP_piece. For this case, GDB
+# should not print:
+#
+# DWARF-2 expression error: DW_OP_GNU_uninit must always be the very last op.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+standard_testfile main.c -dw.S
+
+set asm_file [standard_output_file $srcfile2]
+
+set c64 6639779683436459270
+set c32 1779823878
+
+Dwarf::assemble $asm_file {
+ cu {} {
+ compile_unit {} {
+ declare_labels i64_type i32_type
+
+ i64_type: base_type {
+ {name "int64_t"}
+ {encoding @DW_ATE_signed}
+ {byte_size 8 DW_FORM_sdata}
+ }
+
+ i32_type: base_type {
+ {name "int32_t"}
+ {encoding @DW_ATE_signed}
+ {byte_size 4 DW_FORM_sdata}
+ }
+
+ DW_TAG_variable {
+ {name i64_var}
+ {type :$i64_type}
+ {location {
+ DW_OP_constu $::c64
+ DW_OP_stack_value
+ DW_OP_GNU_uninit
+ DW_OP_piece 8
+ } SPECIAL_expr}
+ }
+
+ DW_TAG_variable {
+ {name i32_var}
+ {type :$i32_type}
+ {location {
+ DW_OP_constu $::c32
+ DW_OP_stack_value
+ DW_OP_GNU_uninit
+ DW_OP_piece 4
+ } SPECIAL_expr}
+ }
+ }
+ }
+}
+
+if {[build_executable ${testfile}.exp ${testfile} \
+ [list $srcfile $asm_file] {nodebug}]} {
+ return -1
+}
+
+clean_restart ${testfile}
+
+if {![runto_main]} {
+ return -1
+}
+
+set cmd "print i64_var"
+if { [is_64_target] } {
+ gdb_test $cmd \
+ " = +\\\[uninitialized\\\] $c64"
+} else {
+ unsupported $cmd
+}
+
+gdb_test "print i32_var" \
+ " = +\\\[uninitialized\\\] $c32"