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

[Max Asbock]

On Wed, 2004-06-09 at 15:49, David Mosberger wrote:
> >>>>> 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.
> 
The code works for x86 because the table has 4-byte entries and
sizeof(unw_word_t) == 4. The table in x86_64 has 4-byte entries as well
so it doesn't easily fit into a struct table_entry with unword_t entries
that are 8 bytes long.
 
>   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.

I do see your point here. It would be slow. I am sure there are other
solutions.

> 
>  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?
The original code allows DW_EH_PE_sdata4 for 4 byte pointer sizes and
DW_EH_PE_sdata8 for 8 byte pointer sizes. This would exclude x86_64.  

>   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?
Yes this is bogus. It was meant to allow only 
(DW_EH_PE_daterel | DW_EH_PE_sdata4) since that's what the
implementation in the patch allows. 

max