Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Doug Evans <dje@google.com>
To: David Blaikie <dblaikie@gmail.com>
Cc: Andrew Pinski <pinskia@gmail.com>,
	gdb-patches <gdb-patches@sourceware.org>,
		Eric Christopher <echristo@gmail.com>
Subject: Re: [patch] Fix unused static symbols so they're not dropped by clang
Date: Mon, 14 Apr 2014 22:56:00 -0000	[thread overview]
Message-ID: <CADPb22QOwf828nnPGKVGFAgMVVLoKuUUGDzQoWmRu1Gm+bHRqA@mail.gmail.com> (raw)
In-Reply-To: <CAENS6EtFYEyyJjb77ks_bX8Y675GDy8ikqWkqgHQ3BT1VorLyA@mail.gmail.com>

On Sun, Apr 13, 2014 at 12:11 AM, David Blaikie <dblaikie@gmail.com> wrote:
> On Fri, Apr 11, 2014 at 9:00 PM, Andrew Pinski <pinskia@gmail.com> wrote:
>> On Fri, Apr 11, 2014 at 1:17 PM, David Blaikie <dblaikie@gmail.com> wrote:
>>> On Fri, Apr 11, 2014 at 11:23 AM, Doug Evans <dje@google.com> wrote:
>>>> On Thu, Apr 10, 2014 at 11:51 PM, David Blaikie <dblaikie@gmail.com> 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.


  reply	other threads:[~2014-04-14 22:56 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-11  6:51 David Blaikie
2014-04-11  7:03 ` pinskia
2014-04-11 18:23 ` Doug Evans
2014-04-11 19:32   ` Doug Evans
2014-04-11 20:16     ` David Blaikie
2014-04-11 20:17   ` David Blaikie
2014-04-12  4:00     ` Andrew Pinski
2014-04-13  7:11       ` David Blaikie
2014-04-14 22:56         ` Doug Evans [this message]
2014-04-15  3:24           ` David Blaikie
2014-04-23 21:50             ` Doug Evans
2014-04-25  5:36               ` David Blaikie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADPb22QOwf828nnPGKVGFAgMVVLoKuUUGDzQoWmRu1Gm+bHRqA@mail.gmail.com \
    --to=dje@google.com \
    --cc=dblaikie@gmail.com \
    --cc=echristo@gmail.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pinskia@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox