Re: [patch#2] fetch result of locdesc expressions as integer (not address)

Tristan Gingold
2011-10-17T07:13:27Z

On Oct 17, 2011, at 1:32 AM, Joel Brobecker wrote:

>> It is not an offset, the problem is it really is an address.
>>
>> DWARF-4:
>> # The beginning address is pushed on the DWARF stack before the location
>> # description is evaluated; the result of the evaluation is the base address
>> # of the member entry.
>
> Ah - I missed that part of the standard, and that helps understand
> the command that pushes the null address indeed.
>
>> Both former and current FSF GDB pushes bogus address 0 first
>> simulating base address of the struct.
>> - stack[stacki] = 0;
>> - stack[++stacki] = 0;
>> + /* DW_AT_data_member_location expects the structure address to be pushed on
>> + the stack. Simulate the offset by address 0. */
>> + dwarf_expr_push_address (ctx, 0, 0);
>
> What we could perhaps try is to push 'integer-to-address(0)', rather
> than zero itself. Then, if we want the offset, then subtract that
> same integer-to-address(0) value from the result.
>
>> While trying to fix it I faced for example the exception for address 0 - isn't
>> it broken for AVR? Isn't SRAM address 0 a valid address?
>> static CORE_ADDR
>> avr_make_saddr (CORE_ADDR x)
>> {
>> /* Return 0 for NULL. */
>> if (x == 0)
>> return 0;
>>
>> return ((x) | AVR_SMEM_START);
>> }
>> Unfortunately I cannot argue about AVR arch issues.
>
> Me neither, and the documentations I have been able to find wheren't
> very clear or complete. I suspect that this is because it's not
> typical CPU, but rather a micro controler. I'm copying Tristan
> who knows this architecture better.

SRAM address 0 is valid, but if you convert NULL to a non-0 value, many things get broken!

Tristan.

>
>> Joel, do you run the testsuite with iron AVR or is it OK to run it
>> some way with sim/avr/ ?
>
> We run our testsuite with the GDB simulator. For the official testsuite,
> there should be a way to do it, and I have an email from Kevin that
> should put me on the track, but for now, we have never really done it.
>
>> gdb/
>> 2011-10-09 Jan Kratochvil <jan.kratochvil@redhat.com>
>>
>> Revert:
>> 2011-07-27 Jan Kratochvil <jan.kratochvil@redhat.com>
>> * dwarf2expr.c (ctx_no_read_reg): New function.
>> * dwarf2expr.h (ctx_no_read_reg): New declaration.
>> * dwarf2read.c (read_2_signed_bytes, read_4_signed_bytes): Remove.
>> (decode_locdesc_read_mem, decode_locdesc_ctx_funcs): New.
>> (decode_locdesc): Replace by a caller of dwarf_expr_eval.
>>
>> gdb/testsuite/
>> 2011-10-09 Jan Kratochvil <jan.kratochvil@redhat.com>
>>
>> * gdb.dwarf2/dw2-simple-locdesc.exp (p &s.shl): KFAIL it.
>> Revert the part of:
>> 2011-07-27 Jan Kratochvil <jan.kratochvil@redhat.com>
>> * gdb.dwarf2/dw2-stack-boundary.exp (check partial symtab errors):
>> Change the expected string.
>
> Based on the reasons you provided, it seems indeed that this patch
> is no longer really necessary. On the other hand, you are right
> to say that it's a bit of a miracle that things are working so far.
>
> --
> Joel