From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19332 invoked by alias); 25 Apr 2014 05:36:01 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 19321 invoked by uid 89); 25 Apr 2014 05:36:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f177.google.com Received: from mail-vc0-f177.google.com (HELO mail-vc0-f177.google.com) (209.85.220.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 25 Apr 2014 05:35:58 +0000 Received: by mail-vc0-f177.google.com with SMTP id if17so4137495vcb.8 for ; Thu, 24 Apr 2014 22:35:56 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.220.10.2 with SMTP id n2mr4936580vcn.26.1398404156493; Thu, 24 Apr 2014 22:35:56 -0700 (PDT) Received: by 10.58.186.176 with HTTP; Thu, 24 Apr 2014 22:35:56 -0700 (PDT) In-Reply-To: <21336.13736.18674.894975@ruffy.mtv.corp.google.com> References: <21336.13736.18674.894975@ruffy.mtv.corp.google.com> Date: Fri, 25 Apr 2014 05:36:00 -0000 Message-ID: Subject: Re: [patch] Fix unused static symbols so they're not dropped by clang From: David Blaikie To: Doug Evans Cc: Andrew Pinski , gdb-patches , Eric Christopher Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00526.txt.bz2 On Wed, Apr 23, 2014 at 2:50 PM, Doug Evans wrote: > David Blaikie writes: > > On Mon, Apr 14, 2014 at 3:56 PM, Doug Evans wrote: > > > On Sun, Apr 13, 2014 at 12:11 AM, David Blaikie wrote: > > >> On Fri, Apr 11, 2014 at 9:00 PM, Andrew Pinski wrote: > > >>> On Fri, Apr 11, 2014 at 1:17 PM, David Blaikie wrote: > > >>>> On Fri, Apr 11, 2014 at 11:23 AM, Doug Evans wrote: > > >>>>> On Thu, Apr 10, 2014 at 11:51 PM, David Blaikie wrote: > > >>>>>> Several tests used file-static functions and variables that were not > > >>>>>> referenced by the code. Even at -O0, clang omits these entities at the > > >>>>>> frontend so the tests fail. > > >>>>>> > > >>>>>> Since it doesn't look like these tests needed this functionality for > > >>>>>> what they were testing, I've modified the variables/functions to > > >>>>>> either be non-static, or marked them with __attribute__((used)). > > >>>>>> > > >>>>>> If it's preferred that I use the attribute more pervasively, rather > > >>>>>> than just making the entities non-static, I can provide a patch for > > >>>>>> that (or some other preferred solution). There's certainly precedent > > >>>>>> for both (non-static entities and __attribute__((used)) in the > > >>>>>> testsuite already and much more of the former than the latter). > > >>>>>> > > >>>>>> I have commit-after-review access, so just looking for sign-off here. > > >>>>> > > >>>>> Yikes. > > >>>>> > > >>>>> This is becoming more and more painful (not your fault of course!). > > >>>>> I can imagine this being a never ending source of regressions. > > >>>>> > > >>>>> Does clang perchance have a -O0-and-yes-I-really-mean-O0 option? > > >>>> > > >>>> Sort of. It does have -femit-all-decls, which, though poorly named, > > >>>> causes clang to produce definitions for unused static entities and > > >>>> even unused inline functions (which GCC doesn't do). > > >>> > > >>> By default GCC does not keep unused inline functions but there is an > > >>> option for that -fkeep-inline-functions. > > >> > > >> Ah, good to know. > > >> > > >> My point was that the GDB test suite passes without enabling that flag > > >> for GCC and I think that's somewhat akin to having the suite passable > > >> without having to add -femit-all-decls for Clang. I realize, of > > >> course, that most GDB developers won't be running the test suite with > > >> Clang, but I'm happy to contribute patches when this comes up from > > >> time to time. It's certainly not a pervasive habit across the test > > >> suite to keep everything static - just this handful of tests happen to > > >> do it. > > >> > > >> But I'm open to whatever you folks think is the best approach - if > > >> that means Clang only passes the suite when passing particular flags, > > >> so be it. Perhaps there'd be a way we could build that knowledge into > > >> the testsuite itself so that GDB developers who want to use Clang > > >> don't have to duplicate those details locally. > > > > > > I don't have a strong preference other than trying to keep things maintainable. > > > > > > Maybe it would be enough to document the issue in the testsuite coding > > > standards section of the manual. This is a really subtle portability > > > issue though ... *something* in the code would be nice. > > > > Given that there are, I assume, many test cases that use unused > > non-static functions, the functions after my patch will look just like > > those. It'd be weird to comment some but not all of them. > > > > But my initial plan had been to put __attribute__((used)) > > everywhere... I could still do that, if preferred, but I assume it'll > > be woefully inconsistent/arbitrary with some tests using "static > > __attribute__((used))" and others using non-static functions anyway. I > > suppose the presence of a smattering of static+attribute cases would > > remind people to do this in cases where they want/need to make the > > entity static, but I'm not sure how effective this would be. > > > > So: > > > > 1) Use non-static entities (patch already provided) > > 2) Use __attribute__((used)) (macro'd at the start of each file? in a > > common header? protected under #ifdefs or not (there seem to be a > > variety of attributes and gnu-isms not protected by #ifdefs, and some > > that are)?) > > 3) Require Clang run the test suite with non-default flags. > > -> preferably with some auto-detection in the test suite to add > > those flags whenever running with clang > > > > Are there other options to consider? (I suppose comments rather than > > attributes (2) would be an alternative - "this thing is non-static so > > Clang will preserve it") > > Let's go with (1) but add something to the wiki documenting the issue. Works for me. Committed in 22842ff63e28b86e0cd40a87186757b2525578f4 > https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards > > I'll do the latter. Thanks a bunch - let me know when you do, I'd be happy to review it. - David