This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFC: implement typed DWARF stack
>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:
Tom> This patch converts the DWARF expression evaluator to use GDB's value
Tom> types. This approach made it easy to support floating point and also
Tom> decimal floating point; and also paves the way for any future
Tom> improvements.
Ulrich> Huh, so value_binop is back after I eliminated it :-)
Ulrich> http://sourceware.org/ml/gdb-patches/2010-06/msg00514.html
Yeah, I actually referred back to this while writing the patch :)
Ulrich> Due to the use of value_as_address in dwarf_expr_fetch_address, this
Ulrich> patch actually ought to still work on the SPU ... I'll do a test.
Thank you.
Tom> There is some ugliness involving signed and unsigned types; this arises
Tom> because "old-style" untyped DWARF values don't have a consistent type.
Tom> Also I needed a little bit of special code to handle logical right
Tom> shifts.
Ulrich> Yes, I'm wondering whether old-style values are handled correctly. We
Ulrich> used to make sure all arithmetic is performed in ctx->addr_size bits
Ulrich> (which is taken from the DWARF section headers). With your patch,
Ulrich> we now always use gdbarch_dwarf2_addr_size -- I'm not sure this is
Ulrich> always the same.
Good point; I am not sure either. Another option would be to use
ctx->addr_size to choose an arch type (e.g., builtin_uint32), and then
also carry along a flag indicating whether the value is "untyped".
I think this is needed because these untyped values must be treated
differently in a couple spots :-(
Yet another idea would be to lazily instantiate these special-to-DWARF
types and make struct dwarf_gdbarch_types a little bigger. I think I
like this idea the best. I think in practice we only need to support 3
such types (and if we run into more in the wild we can easily add them).
Ulrich> Also, what is the reason for handling the conversion to unsigned so
Ulrich> differently in the DW_OP_mod vs. DW_OP_shr cases?
There's no reason, I will clean this up.
Ulrich> [ In fact, maybe we don't need the whole value_cast business and
Ulrich> we could just operate on ULONGEST without involving value_binop,
Ulrich> since both cases only support integers anyway ... ]
I agree, for DW_OP_shr. I will do that. Unfortunately I think
DW_OP_mod still needs special magic.
Ulrich> I'd rather see a dwarf_expr_push_address to keep the
Ulrich> address-type abstraction local to dwarf2expr.c ...
Will do.
Ulrich> Does DW_OP_bra really require an integral type on the stack?
Ulrich> The standard wording isn't 100% clear to me here ...
A couple of oddities were clarified in this thread:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg00333.html
There, Cary said that DW_OP_bra should require an integral type and this
was just an oversight in the spec.
Ulrich> I guess a non_lval value would still seem cleaner here (just as done
Ulrich> below for DW_OP_GNU_reinterpret --- maybe this could be abstracted
Ulrich> into a new value_from_contents helper).
Will do.
Tom> @@ -182,7 +189,7 @@ struct dwarf_expr_piece
Tom>
Tom> /* The piece's register number or literal value, for
Tom> DWARF_VALUE_REGISTER or DWARF_VALUE_STACK pieces. */
Tom> - ULONGEST value;
Tom> + struct value *value;
Ulrich> Maybe now it would be cleaner to split this into two union members,
Ulrich> a plain "int regnum" for DWARF_VALUE_REGISTER, and the struct value
Ulrich> for DWARF_VALUE_STACK ...
That does seem better, I will do that too.
Ulrich> Otherwise this looks good to me.
Thanks very much for the review. I'll post a new patch when I've made
the needed changes.
Tom