diff mbox series

[v3,1/3] compiler_types.h: make __builtin_types_compatible_p() noop for Sparse

Message ID 1542623503-3755-1-git-send-email-yamada.masahiro@socionext.com
State New
Headers show
Series [v3,1/3] compiler_types.h: make __builtin_types_compatible_p() noop for Sparse | expand

Commit Message

Masahiro Yamada Nov. 19, 2018, 10:31 a.m. UTC 
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. Make it no-op.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

Changes in v3:
  - New patch

Changes in v2: None

 include/linux/compiler_types.h | 1 +
 1 file changed, 1 insertion(+)

-- 
2.7.4

Comments

Luc Van Oostenryck Nov. 19, 2018, 12:33 p.m. UTC | #1 
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.

> 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.

Regards,
-- Luc
Masahiro Yamada Nov. 20, 2018, 1:32 a.m. UTC | #2 
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
diff mbox series

Patch

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)
 #else /* __CHECKER__ */
 # ifdef STRUCTLEAK_PLUGIN
 #  define __user __attribute__((user))