From patchwork Wed Oct 11 17:00:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 23483 Received: (qmail 8902 invoked by alias); 11 Oct 2017 17:00:39 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 7049 invoked by uid 89); 11 Oct 2017 17:00:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f173.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=ZA8w3pNmsPcSYivwvNmyr9A2idmVQ9/cptX65h6sw2w=; b=n06MAk46o26YhAP5+ImDyPgI3BkPxPc5s6n/DUeHedyc5o5VDG71xw38fXLb11a9V7 HEt6bfB99MUBKTwvBbFFltqePCL1foK/iYZThyxRbVEDSILNiowixIHVZN9Kf0odSftO 1aCHfhz59HZYsTQ7+YIPriYmr3UsZ8tIFUX1eShLMSpLBsJ1idyiSQZCRjlbICls3gLo lc11ZQKeHDWOnXTb/URYHKEM+i73TkUQy114ThhSxWfkbH7f1oKR3wEQp+tyvRsOD5dV Xlm5Rj4InoY4Bt2RZJKhyuDvMOX2qnnSwL+O4e/bOES/tSZipJzz5RnViaxJ8DRmvht3 ujtA== X-Gm-Message-State: AMCzsaV/fAJNdyNPlE6hzqET/hJ6fQWQDSceOvyVgNyGfVFc/Y8pJOhu RWmtW7n2AQhobZnquezH57JTEuDD61k= X-Google-Smtp-Source: AOwi7QC8vhhCeCOU1QEGNa94WRjyXxJcUxOmE4/h/0udBOUxYOOWc9pSHUBpqm6GEurLwwKIqGV4fw== X-Received: by 10.200.27.171 with SMTP id z40mr467560qtj.151.1507741232004; Wed, 11 Oct 2017 10:00:32 -0700 (PDT) Subject: Re: [PATCH] malloc: Use compat_symbol_reference in libmcheck [BZ #22050] To: Florian Weimer , Joseph Myers Cc: Zack Weinberg , GNU C Library References: <20170831095213.42A4F43994318@oldenburg.str.redhat.com> <2269f61e-d718-b90e-6d08-4e325dad4ac9@redhat.com> <2ad943f4-1b66-5df7-b348-ab0652716470@redhat.com> <61a94166-1768-4014-27f7-34a9a6ff24b8@redhat.com> <5bf3d95b-cdfb-c55f-478f-be61baa846d3@redhat.com> <834aa29d-d8ad-b516-ab12-98b99ad4faf5@redhat.com> From: Carlos O'Donell Message-ID: <60d34595-b3a7-c170-d325-59b1234e3479@redhat.com> Date: Wed, 11 Oct 2017 10:00:27 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <834aa29d-d8ad-b516-ab12-98b99ad4faf5@redhat.com> On 10/05/2017 03:45 AM, Florian Weimer wrote: > On 09/18/2017 04:24 PM, Florian Weimer wrote: >> On 08/31/2017 06:08 PM, Carlos O'Donell wrote: >>> On 08/31/2017 11:04 AM, Joseph Myers wrote: >>>> On Thu, 31 Aug 2017, Florian Weimer wrote: >>>> >>>>> On 08/31/2017 05:42 PM, Zack Weinberg wrote: >>>>>> On Thu, Aug 31, 2017 at 11:37 AM, Carlos O'Donell wrote: >>>>>>> I would like to see a new macro that does what it says, rather than use the >>>>>>> existing macro in the wrong way. Even if the new macro is just a copy. >>>>>>> >>>>>>> This looks like a real problem for glibc, particularly if we need to continue >>>>>>> to use, at least internally, certain old versions of symbols. So having a >>>>>>> new macro for this is fine. >>>>>> >>>>>> I see immediate uses for this macro in the test suite, verifying that >>>>>> compat symbols continue to work correctly...  (particularly thinking >>>>>> of the messy and totally untested old-FILE support). >>>>> >>>>> That's the exact purpose of compat_symbol_reference.  I think Carlos is >>>>> objecting to its use for a *definition*. >>>> >>>> Well, I used it for the definitions of matherr and _LIB_VERSION in my >>>> tests of those compat symbols, because it does exactly what's expected: >>>> makes the definitions in the tests refer to the same entity as the compat >>>> symbols in the shared libraries, rather than being completely independent >>>> entities as they would by default. >>> While it does what's expected, the macro API wasn't designed with that in >>> mind was it? >> >> I would have used it in tst-mallocstate for >> __malloc_initialize_hook if I had understood what was going on. I >> think the usages are so similar that we do not need a new macro. > > Carlos, do you still have objections to the patch as posted? > >   > > I do not think we need another macro with a different name for this. I would like to see something like this added: --- And remove the "this is a hack" comments from your patch, since now you are using the macro API within the usages of the definition. diff --git a/include/shlib-compat.h b/include/shlib-compat.h index d872afc..ba99f9b 100644 --- a/include/shlib-compat.h +++ b/include/shlib-compat.h @@ -78,8 +78,12 @@ #endif -/* Use compat_symbol_reference for a reference to a specific version - of a symbol. Use compat_symbol to define such a symbol. */ +/* Use compat_symbol_reference for a reference *or* definition of a + specific version of a symbol. Definitions are primarily used to + ensure tests reference the exact compat symbol required, or define an + interposing symbol of the right version e.g. __malloc_initialize_hook + in mcheck-init.c. Use compat_symbol to define such a symbol within + the shared libraries that are built for users. */ #define compat_symbol_reference(lib, local, symbol, version) \ compat_symbol_reference_1 (lib, local, symbol, version) #define compat_symbol_reference_1(lib, local, symbol, version) \