This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: implement typed DWARF stack


Tom Tromey wrote:

> 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 ... ]
> 
> Tom> I agree, for DW_OP_shr.  I will do that.  Unfortunately I think
> Tom> DW_OP_mod still needs special magic.
> 
> Actually in the end I thought it was perhaps better to keep DW_OP_shr
> more or less as it was.  My reasoning is that by uniformly using the
> value API, we make it simpler to handle new cases in the future.  E.g.,
> consider if we want to support 128-bit integer types -- we could do this
> via value changes, but this would leave an odd hole in DWARF that the
> patch writer would have to discover.

OK, fine with me.  I guess I still find it a bit odd why we need those
DWARF-specific types in the first place (apparently solely for DW_OP_mod),
but if we really want to be completely compatible with the prior behaviour,
it seems we have to.  (Unless DW_OP_mod is defined to always be an unsigned
operation, just like DW_OP_shr ...)

B.t.w. your patch always performs an unsigned shift for DW_OP_shr, even
on signed operands.  However, for DW_OP_shra, your patch respects the sign
of the operands and might actually perform an unsigned shift (even though
the opcode explicitly says "arithmetic right shift" ...)  This looks like
another of those signed/unsigned inconsistencies with the proposal to me.

> Tom> Thanks very much for the review.  I'll post a new patch when I've made
> Tom> the needed changes.
> 
> Appended.

I ran a test on Cell, and interestingly it turns out that while Cell
mixed-architecture debugging appears to be OK, just plain ppc32 debugging
on a ppc64 host is now broken.  When the (32-bit) address of a stack
variable has its high bit set, dwarf2_evaluate_loc_desc_full now returns
a value with a sign-extended 64-bit CORE_ADDR address (0xffffffff....).
This address later on causes a memory_error when accessed.

Looking into that, your code does run through value_as_address, but
since the platform does not define gdbarch_integer_to_address, this
falls through to unpack_long, and since the underlying type is a
TYPE_CODE_INT and not a pointer type, this in turn now respects
the TYPE_UNSIGNED flag and does a signed conversion ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]