Re: [patch] Fix unused static symbols so they're not dropped by clang

Doug Evans
2014-04-14T22:56:38Z

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.