From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 114500 invoked by alias); 2 May 2017 22:30:26 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 114253 invoked by uid 89); 2 May 2017 22:30:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.99.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 spammy=sorting X-Spam-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: gnu.wildebeest.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (212.238.236.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 02 May 2017 22:30:14 +0000 Received: from stream.wildebeest.org (deer0x01.wildebeest.org [172.31.17.131]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 40E433001761; Wed, 3 May 2017 00:30:12 +0200 (CEST) Received: by stream.wildebeest.org (Postfix, from userid 1000) id C1206FF8A2; Wed, 3 May 2017 00:30:10 +0200 (CEST) Date: Wed, 03 May 2017 11:04:00 -0000 From: Mark Wielaard To: Ulf Hermann Cc: elfutils-devel@sourceware.org Subject: Re: [PATCH] Make elf section sorting more deterministic Message-ID: <20170502223010.GH3321@stream> References: <20170427194124.GE2061@stream> <355f6ed3-88b1-adf3-e2b0-7ba52589e4ea@qt.io> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="sgneBHv3152wZ8jf" Content-Disposition: inline In-Reply-To: <355f6ed3-88b1-adf3-e2b0-7ba52589e4ea@qt.io> User-Agent: Mutt/1.8.0 (2017-02-23) X-IsSubscribed: yes X-SW-Source: 2017-q2/txt/msg00137.txt.bz2 --sgneBHv3152wZ8jf Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 2334 On Fri, Apr 28, 2017 at 12:35:26PM +0200, Ulf Hermann wrote: > 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. OK. > 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. Aha. I see that we treat the end special in find_section () /* Consider the limit of a section to be inside it, unless it's inside the next one. A section limit address can appear in line records. */ if (*addr == sections->refs[idx].end && idx + 1 < sections->count && *addr == sections->refs[idx + 1].start) ++idx; So we do indeed want the zero sized section to be the first. How about the attached? Cheers, Mark --sgneBHv3152wZ8jf Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-Make-elf-section-sorting-more-deterministic.patch" Content-length: 1707 >From d7e1e08f68afd6c3363231e3100fa0124a2d5cb6 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Thu, 20 Apr 2017 16:54:26 +0200 Subject: [PATCH] Make elf section sorting more deterministic 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 section index to tell entries apart. Signed-off-by: Ulf Hermann Signed-off-by: Mark Wielaard --- libdwfl/ChangeLog | 6 ++++++ libdwfl/derelocate.c | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 859b2ff..9bce6b1 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,6 +1,12 @@ 2017-04-20 Ulf Hermann Mark Wielaard + * derelocate.c (compare_secrefs): Compare by end address and then by + section number if addresses are equal. + +2017-04-20 Ulf Hermann + Mark Wielaard + * linux-kernel-modules.c: Always return NULL from kernel_release() on non-linux systems and set errno to ENOTSUP. diff --git a/libdwfl/derelocate.c b/libdwfl/derelocate.c index e5c3e12..2f80b20 100644 --- a/libdwfl/derelocate.c +++ b/libdwfl/derelocate.c @@ -67,7 +67,13 @@ compare_secrefs (const void *a, const void *b) if ((*p1)->start > (*p2)->start) return 1; - return 0; + if ((*p1)->end < (*p2)->end) + return -1; + if ((*p1)->end > (*p2)->end) + return 1; + + /* Same start/end, then just compare which section came first. */ + return elf_ndxscn ((*p1)->scn) - elf_ndxscn ((*p2)->scn); } static int -- 2.9.3 --sgneBHv3152wZ8jf--