Re: MIPS dwarf2 location lists

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Daniel Jacobowitz wrote:

> Subject: [rfc] Fix address vs. offset handling in DWARF-2 location lists
> http://sourceware.org/ml/gdb-patches/2009-07/msg00378.html
> 
> stopped us from using dwarf2_read_address for the offsets in location
> lists.  The comment there talks specifically about MIPS:
> 
>   /* For most architectures, calling extract_unsigned_integer() alone
>      is sufficient for extracting an address.  However, some
>      architectures (e.g. MIPS) use signed addresses and using
>      extract_unsigned_integer() will not produce a correct
>      result.  Make sure we invoke gdbarch_integer_to_address()
>      for those architectures which require it.
> 
> This comment does apply to the calls you removed.  GCC typically
> generates base_address == 0 and puts the whole address in the
> offsets.  Therefore they must be sign extended, and it's
> gdbarch_integer_to_address which does that.  So now we're getting
> zero-extended addresses, and they don't match anything.

Hmm.  The problem I was fixing is that on the Cell/B.E., the transformation
of a target integer into an GDB address is more complex than simply choosing
between sign- or zero-extending.

On Cell, the GDB addresses encode the SPU ID for addresses within
SPU contexts attached to the process.  In the target format (which
is also used in object files, including DWARF addresses etc.), an
address is a plain SPU local store address.

When gdbarch_integer_to_address is called, we attach the SPU ID
into the high word of the 64-bit CORE_ADDR; for example, an SPU
local store address of 0x3f000 in SPU context #7 gets encoded
into a CORE_ADDR value of 0x800000070003f000.

gdbarch_integer_to_address is the right place to do this, because
we want the transformation to also happen for addresses entered
as literals on the command line, for example.

But this means that we need to distinguish very clearly between
*addresses* and *offsets*.  A transformation like the above must
only be done on base addresses; offsets are simply treated as
integers that are added onto an already transformed base address,
and must not themselves be transformed as well.

However, it seems that there is still one choice that isn't
fully specified: should an offset be treated as *signed* or
*unsigned* integer?  The DWARF standard is silent on this;
maybe it assumes this doesn't matter as address arithmetic
on these offsets is supposed to be performed within the DWARF
address size precision? 

> This ties in to my message last night, which was about a different
> instance of a similar bug:
> 
> Subject: CORE_ADDR representation
> http://sourceware.org/ml/gdb/2010-02/msg00118.html
> 
> Unlike the place referenced there, however, this is just comparing the
> results.  So we have a lot of flexibility; the "abstractly right" fix
> would work, but so would a simple mask and check.

Masking high bits off will actually break Cell, because it would
cause the SPU ID to be simply ignored in comparisons.  In the past,
the only places where addresses were truncated according to
gdbarch_addr_bit did so to output an address in human- or machine-
readable string form (as user output, or remote/monitor protocol
elements).  At these points, address truncation in a sense serves
as the reverse of the gdbarch_integer_to_address transform; this
is exactly what's needed on Cell.

However, there have been a couple of places recently introduced
where truncated address values are subsequently used as CORE_ADDR
by common code, in particular the place you point out above.  It
seems that this will break Cell as well (and presumably breaks
MIPS as you noticed).  Using address truncation like this doesn't
seem right to me.  (The other recently affected places are
dwarf2loc.c:read_pieced_value and solib-svr4.c:enable_break.)


> Do you have any better idea than the attached?

One way would be to treat address offsets as signed or unsigned
depending on bfd_get_sign_extend_vma.

A maybe more generic way should be to perform the arithmetic
on plain integral values, and only call gdbarch_integer_to_address
on the *sums* (i.e. the final DWARF addresses), along the lines of:

      low = extract_unsigned_integer (loc_ptr, addr_size, byte_order);
      loc_ptr += addr_size;

      high = extract_unsigned_integer (loc_ptr, addr_size, byte_order);
      loc_ptr += addr_size;

      /* A base-address-selection entry.  */
      if (low == base_mask)
        {
          base_address = high;
          continue;
        }

      /* An end-of-list entry.  */
      if (low == 0 && high == 0)
        return NULL;

      /* Otherwise, a location expression entry.  */
      store_unsigned_integer (tmp, addr_size, byte_order, low + base_address);
      low = dwarf2_read_address (gdbarch, tmp, tmp + addr_size, addr_size);

      store_unsigned_integer (tmp, addr_size, byte_order, high + base_address);
      high = dwarf2_read_address (gdbarch, tmp, tmp + addr_size, addr_size);


As to build_section_addr_info_from_objfile, I'm wondering what problem the
masking was supposed to address:

      sap->other[i].addr = (bfd_get_section_vma (objfile->obfd, sec)
                            + objfile->section_offsets->offsets[i]) & mask;

It looks like this happens if an object is loaded below its original load
address (I guess this can happen if a prelinked object is loaded somewhere
else), and thus the section_offsets need to be negative, which may not
work as intended as CORE_ADDR is unsigned.  It seems that whoever computes
the section_offsets in this particular case ought to be able to compensate ...

Bye,
Ulrich

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