Re: [PATCH] Make elf section sorting more deterministic

[Ulf Hermann <ulf dot hermann at qt dot io>]

On 04/27/2017 09:41 PM, Mark Wielaard wrote:
> On Thu, Apr 20, 2017 at 04:54:26PM +0200, Ulf Hermann wrote:
>> At least one test (dwfl-addr-sect) depends on the order of elf sections
>> with equal addresses. This is not guaranteed by the code. Compare also
>> by end address and name to tell entries apart.
> 
> O, interesting find. If the start addresses match the order depends on
> the specific qsort algorithm. So you need a real tie breaker.
> 
> I think it is simpler and more predictable if we just take the section
> number into account. It seem to have the added benefit that it provide
> the same ordering as before with the glibc qsort, so no testcases need
> to be adjusted. Does the following work for you?
> 
> diff --git a/libdwfl/derelocate.c b/libdwfl/derelocate.c
> index 439a24e..0d10672 100644
> --- a/libdwfl/derelocate.c
> +++ b/libdwfl/derelocate.c
> @@ -63,7 +63,10 @@ compare_secrefs (const void *a, const void *b)
>    if ((*p1)->start > (*p2)->start)
>      return 1;
>  
> -  return 0;
> +  /* Same start address, then just compare which section came first.  */
> +  size_t n1 = elf_ndxscn ((*p1)->scn);
> +  size_t n2 = elf_ndxscn ((*p2)->scn);
> +  return n1 - n2;

I would inline the whole thing to

return elf_ndxscn (p1->scn) - elf_ndxscn (p2->scn);

There is no point in forcing the compiler to keep the intermediate numbers as (signed) size_t. Also, I would still keep the check for p1->end and p2->end before this. If we have a section of size 0, and another one of size > 0 starting at the same place, we want them to be sorted by end address. The zero-sized section should be squeezed in before the one that actually has a size, not after it.

Ulf