From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30612 invoked by alias); 23 Apr 2014 21:50:37 -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 30602 invoked by uid 89); 23 Apr 2014 21:50:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oa0-f74.google.com Received: from mail-oa0-f74.google.com (HELO mail-oa0-f74.google.com) (209.85.219.74) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 23 Apr 2014 21:50:35 +0000 Received: by mail-oa0-f74.google.com with SMTP id i4so392684oah.5 for ; Wed, 23 Apr 2014 14:50:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:mime-version:content-type :content-transfer-encoding:message-id:date:to:cc:subject:in-reply-to :references; bh=EQubNtogyhTKzUDUOcJZOyk+AtsUa3vl5okWKJpSYZo=; b=DuRtE3Z0owhbmLVjyEl/N3r8ObDYoqvFqC5yBTU2mAFY/Ft8IeQM24si1dO+5tq9eG 52aK8/hFVzv7jTMNjzT8bPymHTxadI8pB+714YAnFkEy5u5/1UsaVXdxXSsTXo0xOpuB /28pI6XO9Q1USLXb+6isJf0UNH1zm4zaMgfgTg1Rze6Us9D+kRt37psAlV1FEiP7s7I2 9z+G4AyPtqT7By8e7gaD67LR4D5kOz4OduJqiOS7eIaQ37xtO29WJxcC9HmMJJPKxpis OZIr5cIJxKi47mHQ+9SJBBMFuYUV9rAtq7YqSMIyC0p+B5pkoDiG24gG26IuBPs8JdWR bx3A== X-Gm-Message-State: ALoCoQnDLFhQs+pven3MtYXTx6sClpmA1KqySaGLU3TpBJmZwus8V3mIvSx/YjtdtmXF56W054bEdZGyvxE/G4Gyxo1WUqBIl9V1Ja/udN+AVX9s3T4KRKUS3BgiRbkcbGYSOA6+ai/h9C90MI/9z2ipE3sVxnxeB+nKEAhWJT9/cFRFlgS/SvPmh4TtBXoJpOSBJl2b95uwPhFiFsveoO3/dz83NkGf4Q== X-Received: by 10.182.158.4 with SMTP id wq4mr28118200obb.18.1398289833450; Wed, 23 Apr 2014 14:50:33 -0700 (PDT) Received: from corp2gmr1-1.hot.corp.google.com (corp2gmr1-1.hot.corp.google.com [172.24.189.92]) by gmr-mx.google.com with ESMTPS id r61si285776yhf.1.2014.04.23.14.50.33 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 23 Apr 2014 14:50:33 -0700 (PDT) Received: from ruffy.mtv.corp.google.com (ruffy.mtv.corp.google.com [172.17.128.44]) by corp2gmr1-1.hot.corp.google.com (Postfix) with ESMTP id 91C6931C1A2; Wed, 23 Apr 2014 14:50:32 -0700 (PDT) From: Doug Evans MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <21336.13736.18674.894975@ruffy.mtv.corp.google.com> Date: Wed, 23 Apr 2014 21:50:00 -0000 To: David Blaikie Cc: Andrew Pinski , gdb-patches , Eric Christopher Subject: Re: [patch] Fix unused static symbols so they're not dropped by clang In-Reply-To: References: X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00472.txt.bz2 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. https://sourceware.org/gdb/wiki/Internals%20GDB-Testsuite-Coding-Standards I'll do the latter.