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

[Joel Brobecker <brobecker@adacore.com>]

> 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.

> 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