This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Warn on and fold NULL checks against inline functions
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 27 May 2014 16:54:50 +0200
- Subject: Re: [PATCH] Warn on and fold NULL checks against inline functions
- Authentication-results: sourceware.org; auth=none
- References: <1401130917-4121-1-git-send-email-patrick at parcs dot ath dot cx> <CAFiYyc0DpCubEXt058E-gC4pFfOTxDS4BXdchQvopv+0z0gLww at mail dot gmail dot com> <CA+C-WL9pYBdGcGqzgP4_+EyXQzMLS9AaGwW032r_Ff0CoFzK-Q at mail dot gmail dot com> <CA+C-WL9iMWXWrEYUJLCugv9yF12XrP-LVF83h=4K8Vnyg6M=0w at mail dot gmail dot com>
On Tue, May 27, 2014 at 4:06 PM, Patrick Palka <email@example.com> wrote:
> On Tue, May 27, 2014 at 8:32 AM, Patrick Palka <firstname.lastname@example.org> wrote:
>> On Tue, May 27, 2014 at 3:33 AM, Richard Biener
>> <email@example.com> wrote:
>>> On Mon, May 26, 2014 at 9:01 PM, Patrick Palka <firstname.lastname@example.org> wrote:
>>>> This patch teaches the C++ frontend to warn on NULL checks against
>>>> inline functions and it teaches the middle-end to fold NULL checks
>>>> against inline functions. These two things are currently done for
>>>> non-inline C++ functions, but inline functions are exceptional in that
>>>> the C++ frontend marks them as weak, and NULL checks against weak
>>>> symbols cannot be folded in general because the symbol may be mapped to
>>>> NULL at link-time.
>>>> But in the case of an inline function, the fact that it's a weak symbol
>>>> is an implementation detail. And because it is not permitted to
>>>> explicitly give an inline function the "weak" attribute (see
>>>> handle_weak_attribute), in order to acheive $SUBJECT it suffices to
>>>> assume that all inline functions are non-null, which is what this patch
>>>> Bootstrap and regtest against x86_64-unknown-linux-gnu in progress. Is
>>>> this patch OK assuming no regressions?
>>> What about always_inline function wrappers as used in FORTIFY_SOURCE?
>>> IIRC the actual referenced function may be declared weak and thus the
>>> address can compare to NULL? Sth like
>>> extern void __foo (void) __attribute__((weak,asm("foo")));
>>> extern inline __attribute__((always_inline,gnu_inline)) void foo (void)
>>> __foo ();
>>> int main()
>>> if (foo == 0)
>>> return 0;
>>> abort ();
>>> The issue is that the inline and alias may be hidden to the user and thus
>>> he'll get unwanted and confusing warnings.
>> So as it stands the foo == 0 check in the above example gets folded
>> with or without my patch. The issue here seems to be related to the
>> use of gnu_inline. The address of an inline function marked
>> gnu_inline shouldn't be considered non-NULL because a standalone
>> function does not actually get emitted. Of course, one could inspect
>> aliases but in general it is not correct to assume such functions are
>> non-NULL. Does this sound right?
>> This issue has slight overlap with the issue that my patch tries to
>> fix. I could try to handle it as well.
> Oh, disregard the above. I think I see what you mean now, Richard.
> Because __foo is an alias for foo, and __foo is weak, is it safe to
> assume that foo is non-NULL? That is a tricky question...
No (I wasn't making a folding validity remark). I want to make sure
we won't see diagnostics on code like
if (malloc != 0)
just because it is built with -D_FORTIFY_SOURCE. Whether we
may fold that check at all is another question, of course (and again
that should _not_ differ between -D_FORTIFY_SOURCE or -U_FORTIFY_SOURCE,
but it may well also be a glibc issue).
And yes, aliases are tricky beasts ...