pmachata/reader_hooks review

[Roland McGrath <roland at redhat dot com>]

I'm looking at 6200ba6.

* Cosmetic trivia: casts should look like "(type) expression".
  When I either type is void * (like d_buf), don't cast at all.

* What is read_length for?  I don't think there can be any relocs in the
  DW_FORM_block* length fields.  What other use do you envision for it?

* read_offset should take another IDX_* argument for the target section into
  which the offset points.  (Sorry, I thought I had said that before.)  This
  lets us consolidate the basic offset sanity checking there.

* s/Dwarf_Addr/Dwarf_Off/ in the read_offset signature.

* DW_FORM_ref[48] should not have relocs.  Those values are CU-relative and
  must always be computed by arithmetic at assembly time.  Of the reference
  forms, I think only DW_FORM_ref_addr can have a reloc.

* In both pubnames and formref_die, s/address/offset/.

* In dwarf_getlocation_addr, s/IDX_debug_info/IDX_debug_loc/.

* Make __libdw_in_section return bool.  It only makes sense for helpers to
  return int (0/-1) when that might be usable directly as the return value
  by a public dwarf_* caller.

* In dwarf_getpubnames, only the cu_offset header field can have a reloc.
  The die_offset fields are CU-relative.

* There are two cases of reading a begin/end pair of address_size words: in
  location lists (dwarf_getlocation.c), and in range lists (dwarf_ranges.c).
  These should be consolidated into a single helper that we can make
  reloc-savvy (might as well put it in dwarf_ranges.c).  That can do these
  several common things:

  * Take the base-address logic from dwarf_getlocation.c (start with -1 and
    initialize it on demand).
  * A base address entry should be one where there is no reloc on the
    "begin" word, so it is literal -1.
  * An end of list entry should be one where there are no relocs on either
    word and both are literal 0.

  We don't need any new logic for the moment, just make it a stub for now
  and we can fill it in with the real reloc support when we do that to the
  other stubs.

* dwarf_formudata needs special treatment.

  If DW_FORM_data[48] have relocs for a "constant", those are the
  read_address flavor, not the read_offset flavor.  Really "address"
  doesn't mean "address", it means "relocated with reference to allocated
  sections".  So if it has any relocs at all, they will be that flavor.

  For the internal calls for the "class *ptr" attributes, we should use a
  new internal helper in place of dwarf_formudata.  That helper can take
  the attribute, the target-section IDX_*, and a Dwarf_Off * to fill in.
  It does the form decode and read_offset together.

  In fact, looking at all the read_offset uses, I think we should roll a
  bit more in there.  Instead of yielding a Dwarf_Off, it can just yield
  a const unsigned char *.  It should take an additional argument that is
  the DWARF_E_* code to fail with if the target section is missing.
  It might be simplest to make it also just yield an endp, to we can make
  everything use endp - readp < n comparisons instead of d_size checks.


Thanks,
Roland