Re: [linux-next:master 10774/15793] kernel/kcsan/core.c:829:1: warning: no previous prototype for function '__tsan_volatile_read1'

[Philip Li <philip.li@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 4266 bytes --]

On Fri, Jun 05, 2020 at 12:13:36PM -0700, Nick Desaulniers wrote:
> On Fri, Jun 5, 2020 at 10:58 AM 'Marco Elver' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
> >
> > On Fri, 5 Jun 2020 at 19:42, kernel test robot <lkp@intel.com> wrote:
> > >
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > > head:   af30725c132e2e5c5369b60369ff0771fde7d4ff
> > > commit: 4e23395b9e97562d12b87a330a2fca3bf10c8663 [10774/15793] kcsan: Support distinguishing volatile accesses
> > > config: x86_64-randconfig-r013-20200605 (attached as .config)
> > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 6dd738e2f0609f7d3313b574a1d471263d2d3ba1)
> > > reproduce (this is a W=1 build):
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # install x86_64 cross compiling tool for clang build
> > >         # apt-get install binutils-x86-64-linux-gnu
> > >         git checkout 4e23395b9e97562d12b87a330a2fca3bf10c8663
> > >         # save the attached .config to linux build tree
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>, old ones prefixed by <<):
> > >
> > > DEFINE_TSAN_READ_WRITE(4);
> > > ^
> > > kernel/kcsan/core.c:757:7: note: expanded from macro 'DEFINE_TSAN_READ_WRITE'
> > > void __tsan_read##size(void *ptr)                                                      ^
> > > <scratch space>:92:1: note: expanded from here
> > > __tsan_read4
> > > ^
> > > kernel/kcsan/core.c:776:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > > DEFINE_TSAN_READ_WRITE(4);
> > > ^
> > > static
> > > kernel/kcsan/core.c:757:2: note: expanded from macro 'DEFINE_TSAN_READ_WRITE'
> > > void __tsan_read##size(void *ptr)                                                 ^
> > > kernel/kcsan/core.c:776:1: warning: no previous prototype for function '__tsan_write4' [-Wmissing-prototypes]
> > > DEFINE_TSAN_READ_WRITE(4);
> > > ^
> > [...]
> >
> > In the case here (and for all other *SAN or compiler-based
> > instrumentation), it's clearly fine since the compiler is the one
> > emitting calls to them.
> >
> > How is one meant to act on W=1 reports like this? The kernel seems to
> > be full of these. It seems wrong, just to shut up W=1, to start adding
> > -Wno-missing-prototypes everywhere.
> 
> + Philip, because the team is probably getting asked this a lot, and
> should link to this response.
thanks a lot, i had added your response at https://github.com/intel/lkp-tests/wiki/LKP-FAQ
which is very helpful to explain this.

> 
> `make W=1` enabled -Wmissing-prototypes via scripts/Makfile.extrawarn.
> 
> Both GCC and Clang will warn for -Wmissing-prototypes when you define
> a function without a previous declaration of it, for non-static
> functions.  If the linkage is extern, and there was no previous
> declaration, then callers may have the wrong signature.  Mostly, this
> isn't a bug, which is why it's not an error. But sometimes the
> signature has changed but the callers have not been updated, which is
> an ABI breakage resulting in a bug.  Mostly this is not an issue due
> to -Wimplicit-function-declaration being on by default, which also
> helps in this case.
> 
> See: https://godbolt.org/z/aYrYfS
> 
> Functions with external linkage should have a forward declaration, or
> be marked static if in a .c file, or be marked static inline if in a
> header file.
> 
> For this specific case, it looks like kcsan.h should forward declare
> the functions defined by DEFINE_TSAN_READ_WRITE, to avoid any ABI
> mismatch due to different call signatures between callers and callee.
> Though I can't see where the callees are; if they're only within
> kernel/kcsan/core.c, then defining them as static qualified may be
> better. (Probably not, since they're exported via EXPORT_SYMBOL).
> -- 
> Thanks,
> ~Nick Desaulniers