Re: RFC: implement typed DWARF stack

["Ulrich Weigand" <uweigand@de.ibm.com>]

Tom Tromey wrote:

> Ulrich> I ran a test on Cell, and interestingly it turns out that while
> Ulrich> Cell mixed-architecture debugging appears to be OK, just plain
> Ulrich> ppc32 debugging on a ppc64 host is now broken.  When the
> Ulrich> (32-bit) address of a stack variable has its high bit set,
> Ulrich> dwarf2_evaluate_loc_desc_full now returns a value with a
> Ulrich> sign-extended 64-bit CORE_ADDR address (0xffffffff....).  This
> Ulrich> address later on causes a memory_error when accessed.
> 
> Can you send me your test case?
> I have a PPC64 gdb now; I built one on gcc40, on the GCC compile farm.

The specific test case I was looking at was one of the Cell test cases
(gdb.cell/bt.exp), which failed already in a PowerPC-only part.

However, you don't really need to look specifically into Cell tests;
with your patch applied, I'm seeing a whole bunch of test suite
regressions in an -m32 test on ppc64.

Typical errors look like:

#0  0x0fd65e40 in raise () from /home/uweigand/fsf/gdb-head-build-rhel5/gdb/testsuite/gdb.base/break-interp-BINprelinkNOdebugNOpieNO.d/libc.so.6^M
#1  0x0ffd16f4 in libfunc (action=Cannot access memory at address 0xfffae1c8^M
) at ../../../gdb-head/gdb/testsuite/gdb.base/break-interp-lib.c:33^M
#2  0x100016ac in main ()^M
(gdb) FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: BINprelinkNOdebugNOpieNO: core: core main bt

Note the "Cannot access memory ..." because the address (internally,
not visible in the output) is sign-extended to 64-bit.

Likewise, a bunch of tests fail with

Could not insert hardware watchpoint 11.^M
Could not insert hardware breakpoints:^M
You may have requested too many hardware breakpoints/watchpoints.^M

which seems to be caused by attempting to set a hardware watchpoint
at an address that is sign-extended to 64-bit.

> Ulrich> B.t.w. your patch always performs an unsigned shift for
> Ulrich> DW_OP_shr, even on signed operands.  However, for DW_OP_shra,
> Ulrich> your patch respects the sign of the operands and might actually
> Ulrich> perform an unsigned shift (even though the opcode explicitly
> Ulrich> says "arithmetic right shift" ...)  This looks like another of
> Ulrich> those signed/unsigned inconsistencies with the proposal to me.
> 
> Yes.  My understanding is that for new-style typed values, DW_OP_shr and
> DW_OP_shra are actually the same -- the type indicates the operation to
> perform.  But, for old-style values, we must cast to unsigned for
> DW_OP_shr.

I see.  However, the code as implemented casts *all* signed values to
unsigned for DW_OP_shr, not just old-style values.  That's what got
me confused ...

> Ulrich> Looking into that, your code does run through value_as_address, but
> Ulrich> since the platform does not define gdbarch_integer_to_address, this
> Ulrich> falls through to unpack_long, and since the underlying type is a
> Ulrich> TYPE_CODE_INT and not a pointer type, this in turn now respects
> Ulrich> the TYPE_UNSIGNED flag and does a signed conversion ...
> 
> What if we add a cast to dwarf_expr_fetch_address, like the appended?
> I am not really sure whether this is ok.

Huh, interesting approach.  In a sense, that might be OK, since it mirrors
what we're doing in dwarf_expr_read_reg by calling address_from_register.
On the other hand, I'm not sure value_cast always does the right thing if
the size of a pointer type differs from the size of the DWARF address type ...

In any case, that patch does fix the regressions I'm seeing on PowerPC.


Another issue that just occurred to me: your patch creates possibly many
temporary struct value objects.  I'm wondering whether those ought to be
released from the value chain at some point ...


Bye,
Ulrich

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