Re: [patch x64 SEH]: Sort pdata section ascending

[Kai Tietz <ktietz70 at googlemail dot com>]

2010/9/11 Dave Korn <dave.korn.cygwin@gmail.com>:
> On 11/09/2010 11:14, Kai Tietz wrote:
>> Hello,
>>
>> this patch adds feature of sorting .pdata elements ascending.
>>
>> ChangeLog
>>
>> 2010-09-11 ?Kai Tietz
>>
>> ? ? ? ? * peXXigen.c (sort_x64_pdata): New helper.
>> ? ? ? ? (_bfd_XXi_final_link_postscript): Do pdata sorting.
>>
>> Tested for x86_64-w64-mingw32 and i686-pc-cygwin. Ok for apply?
>
> ?Well, one thing ...
>
> +sort_x64_pdata (const void *l, const void *r)
> +{
> + ?const void *lp = ((const void *) l);
> + ?const void *rp = ((const void *) r);
>
> ... why are you casting those const void *s to const void *? ?It looks like a
> thinko to me.

Well, those lines are a habit (normally qsort works on arrays, so a
dereferencing is necessary, but here the field itself is sorted and so
this cast is superflous). I'll update it.

> ?BTW, the fact that you can just sort all the pdata elements like that and
> there's nothing to update in the testsuite... that indicates there's nothing
> testing that .pdata section contents get correctly generated, doesn't it? ?How
> would we know if the sort routine is even working at all? ?(Well obviously I
> suppose you've checked some files manually, but this could really do with at
> least a trivial dump test to make sure that the whole shebang keeps working.)
>
> ? ?cheers,
> ? ? ?DaveK

Well, there is a way to check this (if you having a gcc version which
is able to produce pdata entries). The objdump tools supports dumping
of pdata/xdata for x64 and here you can see that entries are sorted
afterwards. The sorting itself is btw mentioned in pe-coff
specification, so I assume that other targets - like arm wince - need
here sorting in future, too. But as they aren't using SEH at the
moment (at least status of binutils indicates that), I spared here the
casing for other targets for this.

We will add testcases for x64 SEH, when initial support of x64 SEH is in gcc.

Regards,
Kai

-- 
|? (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

Attachment: sort_pdata.diff
Description: Binary data