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

[Tristan Gingold <gingold@adacore.com>]

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