Re: [PATCH] gdb: Reject non-base type operands in typed DWARF expressions

[Tom Tromey <tom@tromey.com>]

>>>>> Jielun Wu <firmiana402@gmail.com> writes:

> DWARF typed expression operators require a type operand that names a
> DW_TAG_base_type DIE.  This applies to DW_OP_const_type,
> DW_OP_regval_type, DW_OP_deref_type, and the nonzero type operands of
> DW_OP_convert and DW_OP_reinterpret.

Thanks.

> dwarf_expr_context::get_base_type documents that contract, but its
> implementation accepted any DIE that resolved to a GDB type.  As a
> result, typed expression operators could accept non-base DIEs such as
> DW_TAG_structure_type.

> Add dwarf2_get_base_type to resolve the referenced DIE, reject invalid or
> non-base DIEs, and return the corresponding base type.  Use it from the
> DWARF expression evaluator.

> Add a DWARF assembler test covering DW_OP_const_type with a
> DW_TAG_structure_type type operand.

I wonder whether this is truly worth checking; or at least whether it is
worth checking in this way.

> +/* See read.h.  */
> +
> +struct type *
> +dwarf2_get_base_type (cu_offset die_offset, dwarf2_per_cu *per_cu,
> +		      dwarf2_per_objfile *per_objfile)
> +{
> +  dwarf2_cu *cu = per_objfile->get_cu (per_cu);
> +  if (cu == nullptr)
> +    cu = load_cu (per_cu, per_objfile, false);

This re-reads all the DIEs, which may be expensive.  But the only reason
this would be needed is to check a DWARF constraint.

It doesn't really matter, I think, if gdb accepts a "reasonable"
expression that isn't technically correct according to DWARF.  gdb does
similar things in many places already, there are a bunch of supported
DWARF extensions.

Now, at the same time, we may not want to allow any old type to be
returned.  But this could be done by looking for "base-like" types --
basically scalars of various kinds -- and rejecting others.

Full checking of this kind of thing could be done in a dwarflint tool,
should that ever end up being real.

> +		    _op .byte $_constants(DW_OP_const_type)
> +		    _op .uleb128 "$struct_label - $_cu_label"
> +		    _op .byte 4
> +		    _op .byte 1
> +		    _op .byte 2
> +		    _op .byte 3
> +		    _op .byte 4

See that other patch, a handler should be written for this.

Tom