[libunwind] Re: dwarf fixes: searching the table in eh_frame_hdr

[David Mosberger]

>>>>> On Wed, 09 Jun 2004 14:13:20 -0700, Max Asbock <address@hidden> said:

  Max> The int32_t was introduced here because the actual values in
  Max> the tables for the two architectures (x86 and x86_64) we ran
  Max> this code are 32bit signed integers. This is to be seen more
  Max> like a "make it work quickly" effort. However I agree that
  Max> generally this not the right way to approach this.  The EH
  Max> Frame header is defined in:
  Max> 
http://www.linuxbase.org/spec/refspecs/LSB_1.3.0/gLSB/gLSB/ehframehdr.html

Yes, but LSB isn't really complete in that regard and certainly
doesn't cover DWARF3.

  Max> The encoding of the binary search table entries is defined by
  Max> the table_enc entry in the dwarf_eh_frame_hdr struct.
  Max> table_enc for both x86 and x86-64 happens to be DW_EH_PE_sdata4
  Max> | DW_EH_PE_datarel where data relative means relative to the
  Max> beginning of the .eh_frame_hdr section. That means unw_word_t
  Max> does not work here.

I don't see why not---as long as the values are properly
sign-extended/zero-extended.  That code does work for x86, so the
x86-64 case should be fixable if it doesn't work already.

  Max> I think we really should use the dwarf_read_encoded_pointer
  Max> function to search the binary table.

That would most likely be too slow and pretty much defeat the whole
purpose of the optimization.

 Max> >>> Include dwarf_i.h because we are using dwarf_reads32 instead of 
access_mem
 Max> >>> (we really should use dwarf_read_encoded_pointer instead)

Sounds fine.

 Max> >>> The following changes are also related to the table encoding issue.
 Max> >>> DW_EH_PE_datarel | DW_EH_PE_sdata4 is the table encoding used with
 Max> >>> x86 and x86-64.

The original code already allowed (DW_EH_PE_datarel | DW_EH_PE_sdata4) so
why disallow the other cases?  There must be something else that's wrong
here.  In fact, the new code in the patch seems definitely bogus:

+  if (!hdr->table_enc == (DW_EH_PE_datarel | DW_EH_PE_sdata4))

why complementing hdr->table_enc here?

 Max> >>> Adding 2*sizeof unw_word_t looks wrong because table_len
 Max> >>> is the number of entries in the table not the number of bytes.
 Max> >>> But why add anything to the table length? What am I missing?
 Max> -  di->u.rti.table_len = fde_count + 2 * sizeof (unw_word_t);
 Max> +  di->u.rti.table_len = fde_count;

You're right, the addition of "2 * sizeof (unw_word_t)" looks
completely bogus.  This code went through a couple of iterations when
I wrote it originally, so this must have been a left-over from an
older version.

 Max> @@ -190,6 +189,8 @@
 Max>    "table_data=0x%lx\n", (char *) di->u.rti.name_ptr,
 Max>    (long) di->u.rti.segbase, (long) di->u.rti.table_len,
 Max>    (long) di->gp, (long) di->u.rti.table_data);
 Max> +
 Max> +
 Max>    return 1;
 Max>  }

Please remove such white-space only changes.

Thanks,

        --david