Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1542623503-3755-1-git-send-email-yamada.masahiro@socionext.com> <20181119123334.6xr23ok4uy5cflzy@ltop.local>
In-Reply-To: <20181119123334.6xr23ok4uy5cflzy@ltop.local>
From:   Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Tue, 20 Nov 2018 10:32:40 +0900
Message-ID: <CAK7LNARrYp1KYe9GveVYz1oOj2_O5TXE368kKvSY2ASAYBeoKg@mail.gmail.com>
Subject: Re: [PATCH v3 1/3] compiler_types.h: make __builtin_types_compatible_p()
 noop for Sparse
To:     Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Kees Cook <keescook@chromium.org>,
        Josh Triplett <josh@joshtriplett.org>,
        Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Paul Burton <paul.burton@mips.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Nov 19, 2018 at 9:35 PM Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> On Mon, Nov 19, 2018 at 07:31:41PM +0900, Masahiro Yamada wrote:
> > When I tried to delete BUILD_BUG_ON stubs for sparse, the kbuild test
> > robot reported lots of Sparse warnings from container_of(), which
> > seem false positive.
> >
> > The following checker in container_of() seems to be causing something
> > strange for Sparse.
> >
> >   BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) &&   \
> >                    !__same_type(*(ptr), void),                    \
> >                    "pointer type mismatch in container_of()");    \
> >
> > I narrowed down the problem into the following test code:
> >
> >   --------------------(test_code.c begin)--------------------
> >   struct foo {
> >           int (*callback)(void);
> >   };
> >
> >   void assert(int);
> >
> >   static inline struct foo *get_foo(void)
> >   {
> >           assert(__builtin_types_compatible_p(void, void));
> >
> >           return (struct foo *)0;
> >   }
> >
> >   int test(void);
> >   int test(void)
> >   {
> >           return get_foo()->callback();
> >   }
> >   ---------------------(test_code.c end)---------------------
> >
> > Of course, GCC (and Clang as well) can compile it:
> >
> >   $ gcc -Wall -c -o test_code.o test_code.c
> >
> > However, Sparse complains about this obviously correct code:
> >
> >   $ sparse test_code.c
> >   test_code.c:9:45: warning: unknown expression (4 0)
> >   test_code.c:9:51: warning: unknown expression (4 0)
> >
> > Interstingly, just removing the 'inline' keyword in the test code
> > makes Sparse happy.
> >
> > I concluded that Sparse cannot handle __builtin_types_compatible_p()
> > correctly.
>
> I think it's only caused by comparing 'void' (which is never
> an l-value).
> I'll investigate. Thanks for the small test-case.


Yes, please.


> > Make it no-op.
>
> ...
>
> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index 4a3f9c0..9e7da0b 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -23,6 +23,7 @@
> >  extern void __chk_user_ptr(const volatile void __user *);
> >  extern void __chk_io_ptr(const volatile void __iomem *);
> >  # define ACCESS_PRIVATE(p, member) (*((typeof((p)->member) __force *) &(p)->member))
> > +# define __builtin_types_compatible_p(t1, t2)        (1)
>
> Now, BUILD_BUG_ON() becomes a no-op for sparse but all the other usages
> of __builtin_types_compatible_p() become potentially wrong and can now
> create their onw false warnings.

You are right.
This patch is probably a bad idea.


Thanks.


-- 
Best Regards
Masahiro Yamada