Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Simon Marchi <simark@simark.ca>
To: Jielun Wu <firmiana402@gmail.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb: Check DW_OP_deref_type size against type
Date: Mon, 17 Aug 2026 12:33:40 -0400	[thread overview]
Message-ID: <e75a0f8d-581f-4b99-9a46-3f0f94c6c417@simark.ca> (raw)
In-Reply-To: <20260814085303.2561156-1-firmiana402@gmail.com>

On 8/14/26 4:53 AM, Jielun Wu wrote:
> DWARF v5 requires the explicit size operand of DW_OP_deref_type to
> match the size of the referenced base type.  The evaluator currently
> passes both sizes to dwarf_expr_context::deref without validating the
> encoding, and the generic dereference helper accepts the mismatch by
> zero-extending the bytes read from memory.
> 
> Reject a mismatch after resolving the base type.  Add a DWARF assembler
> test covering both DW_OP_deref_type and DW_OP_GNU_deref_type.
> 
> Tested on x86_64-linux.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34277
> Signed-off-by: Jielun Wu <firmiana402@gmail.com>
> ---
>  gdb/dwarf2/expr.c                             |  3 +
>  .../gdb.dwarf2/dw2-deref-type-size.exp        | 78 +++++++++++++++++++
>  2 files changed, 81 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-deref-type-size.exp
> 
> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
> index 3a6b8f58199..ff23fb3e891 100644
> --- a/gdb/dwarf2/expr.c
> +++ b/gdb/dwarf2/expr.c
> @@ -2027,6 +2027,9 @@ dwarf_expr_context::execute_stack_op (gdb::array_view<const gdb_byte> expr)
>  		op_ptr = safe_read_uleb128 (op_ptr, op_end, &uoffset);
>  		cu_offset type_die_cu_off = (cu_offset) uoffset;
>  		type = get_base_type (type_die_cu_off);
> +		if (type->length () != addr_size)
> +		  error (_("DW_OP_deref_type has different sizes for type and "
> +			   "data"));

I think the error message could be clearer, "type" and "data" are very
vague.  Ideally, we would print the actual op used (DW_OP_deref_type vs
DW_OP_GNU_deref_type), since they don't have the same encoding.
We could also at least print the sizes involved, that could help someone
figure out what is wrong.  Suggestion:


                if (type->length () != addr_size)
                  error (_("%s has dereference size %d, but its "
                           "type has size %s"),
                         get_DW_OP_name (op), addr_size,
                         pulongest (type->length ()));

Orthogonal to your change: the addr_size variable seems misnamed, nothing
says that the value we are accessing is an address.

> +Dwarf::assemble $asm_file {
> +    cu {version 5} {
> +	compile_unit {} {
> +	    declare_labels uint64_label
> +
> +	    uint64_label: base_type {
> +		DW_AT_name "uint64_t"
> +		DW_AT_encoding @DW_ATE_unsigned
> +		DW_AT_byte_size 8 DW_FORM_sdata
> +	    }
> +
> +	    foreach {var op} {
> +		bad_deref_type DW_OP_deref_type
> +		bad_gnu_deref_type DW_OP_GNU_deref_type
> +	    } {
> +		DW_TAG_variable {
> +		    DW_AT_name $var
> +		    DW_AT_type :$uint64_label
> +		    DW_AT_external 1 DW_FORM_flag
> +		    DW_AT_location {
> +			variable _constants
> +			variable _cu_label
> +
> +			DW_OP_addr main_label
> +			_op .byte $_constants($op) $op
> +			_op .byte 4 "dereference size"
> +			_op .uleb128 "$uint64_label - $_cu_label" \
> +			    "base type DIE offset"

Instead of emitting the bytes in-line here, could you please add the
necessary "_handle_DW_OP_*" procs to lib/dwarf.exp?

> +			DW_OP_stack_value
> +		    } SPECIAL_expr
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} \
> +	 [list $srcfile $asm_file] nodebug]} {
> +    return
> +}
> +
> +if {![runto_main]} {
> +    return
> +}
> +
> +foreach_with_prefix var {
> +    bad_deref_type
> +    bad_gnu_deref_type
> +} {
> +    gdb_test "print $var" \
> +	"DW_OP_deref_type has different sizes for type and data"
> +}

Could you add some tests where the access works?  It doesn't seem like
we have coverage for that in the testsuite (at least for DWARF
assembler-based tests).

Simon

  parent reply	other threads:[~2026-08-17 16:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-14  8:53 Jielun Wu
2026-08-14 19:44 ` Keith Seitz
2026-08-17 16:33 ` Simon Marchi [this message]
2026-08-21 19:26 ` Tom Tromey
2026-08-22  2:22   ` JIELUN WU

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e75a0f8d-581f-4b99-9a46-3f0f94c6c417@simark.ca \
    --to=simark@simark.ca \
    --cc=firmiana402@gmail.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox