Re: [PATCH] Makefile: Fix build with scan-build
From: Amadeusz Sławiński
Date: Fri Jan 21 2022 - 06:21:02 EST
On 1/20/2022 12:08 AM, Nathan Chancellor wrote:
On Wed, Jan 19, 2022 at 02:19:39PM -0700, Nathan Chancellor wrote:
On Wed, Jan 19, 2022 at 02:51:47PM +0100, Amadeusz Sławiński wrote:
When building kernel with scan-build for analysis:
$ scan-build make defconfig
$ scan-build make menuconfig # disable RETPOLINE
$ scan-build make -j16 bindeb-pkg
since commit 7d73c3e9c514 ("Makefile: remove stale cc-option checks")
it fails with:
CC scripts/mod/empty.o
could not find clang line
make[4]: *** [scripts/Makefile.build:287: scripts/mod/empty.o] Error 1
Seems like changes to how -fconserve-stack support was detected broke
build with scan-build. Revert part of mentioned commit which changed
that.
Fixes: 7d73c3e9c514 ("Makefile: remove stale cc-option checks")
CC: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
Reviewed-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
---
Makefile | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 765115c99655..1174ccd182f5 100644
--- a/Makefile
+++ b/Makefile
@@ -991,9 +991,7 @@ KBUILD_CFLAGS += -fno-strict-overflow
KBUILD_CFLAGS += -fno-stack-check
# conserve stack if available
-ifdef CONFIG_CC_IS_GCC
-KBUILD_CFLAGS += -fconserve-stack
-endif
+KBUILD_CFLAGS += $(call cc-option,-fconserve-stack)
# Prohibit date/time macros, which would make the build non-deterministic
KBUILD_CFLAGS += -Werror=date-time
--
2.25.1
Okay, I think I understand why this happens...
scan-build points CC to its CC wrapper [1], ccc-analyzer, which builds the
code with a compiler [2] then runs clang for the static analyzer [3].
The problem is that the default compiler for ccc-analyzer is GCC, which
means that CONFIG_CC_IS_GCC gets set and flags that are supported by GCC
but not clang will cause the clang analyzer part of ccc-analyzer to
error because ccc-analyzer just passes all '-f' flags along [4].
Prior to 7d73c3e9c514, there was no error because cc-option would run
the flag against ccc-analyzer, which would error out for the reason I
just described, which would prevent the flag from getting added to
KBUILD_CFLAGS.
Now, -fconserve-stack gets passed along to both gcc and clang but clang
does not recognize it and errors out.
This should be fixed in clang, which already has the machinery to
recognize but ignore GCC flags for compatibility reasons (which is
probably how gcc and clang can use the same flags). I have pushed a
patch to Phabricator for review:
https://reviews.llvm.org/D117717
You need to disable CONFIG_RETPOLINE for the same reason but I don't
think working around that in clang is as simple.
Until that fix can proliferate through distributions and such, this is
not an unreasonable workaround (unless Masahiro or Nick have a better
idea) but I would really like a comment so that we can revert this once
that fix is more widely available (it is unlikely that clang will
actually support this option).
[1]: https://github.com/llvm/llvm-project/blob/3062a1469da0569e714aa4634b29345f6d8c874c/clang/tools/scan-build/bin/scan-build#L1080
[2]: https://github.com/llvm/llvm-project/blob/fd0782a37bbf7dd4ece721df92c703a381595661/clang/tools/scan-build/libexec/ccc-analyzer#L457
[3]: https://github.com/llvm/llvm-project/blob/fd0782a37bbf7dd4ece721df92c703a381595661/clang/tools/scan-build/libexec/ccc-analyzer#L783
[4]: https://github.com/llvm/llvm-project/blob/fd0782a37bbf7dd4ece721df92c703a381595661/clang/tools/scan-build/libexec/ccc-analyzer#L661-L665
Thinking more about this after Fangrui commented on the clang patch
above, using scan-build with GCC as the compiler is going to be hard to
support, as we are basically trying to support using two different
compilers with a unified set of '-f' flags, which I see as problematic
for a few reasons.
1. It restricts our ability to do cc-option cleanups like Nick did.
We should be eliminating cc-option calls that we know are specific to
one compiler because checking the Kconfig variables (CONFIG_CC_IS_...)
is much cheaper than invoking the compiler.
2. Necessary GCC specific flags will get dropped.
Adding back the call to cc-option will allow the build to succeed but it
drops the flag from KBUILD_CFLAGS. If there were ever a time where an
'-f' flag was needed to get a working kernel with GCC, it would not get
added because clang would reject it.
We already have a static-analyzer target that requires using CC=clang so
I think there is some precedent here to say we require the kernel to be
built with clang to use the static analyzer. The fact that it did prior
to 7d73c3e9c514 can just be chalked up to luck.
$ make -j"$(nproc)" LLVM=1 defconfig bindeb-pkg static-analyzer
would be the equivalent command to the original patch.
You can still use scan-build with the '--use-cc=clang' flag, which will
use clang for the compilation and analysis, if you so prefer.
Masahiro and Nick may have further thoughts and I am open to other
opinions but my vote is to say this is an issue we won't fix or
workaround.
Cheers,
Nathan
Thank you for detailed explanation. Well I guess question then is: how
much scan-build is supported? And if it should even support mixing clang
and gcc? Alternatively maybe use clang as default if CC environment
variable is not set?
What I like about scan-build is that it generates html report file.
'--use-cc=clang' worked fine for me.
I've also tried
> $ make -j"$(nproc)" LLVM=1 defconfig bindeb-pkg static-analyzer
although there seems to be no static-analyzer target, I guess you meant
clang-analyzer instead, but although it seems to generate a lot of text
on terminal, it doesn't seem that useful to me. Not sure if this is
expected?
Quoting a piece of log:
./include/linux/xarray.h:54:2: error: expected '(' after 'asm'
[clang-diagnostic-error]
WARN_ON((long)v < 0);
^
./include/asm-generic/bug.h:123:3: note: expanded from macro 'WARN_ON'
__WARN(); \
^
./include/asm-generic/bug.h:96:19: note: expanded from macro '__WARN'
#define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
^
./arch/x86/include/asm/bug.h:79:2: note: expanded from macro '__WARN_FLAGS'
_BUG_FLAGS(ASM_UD2, BUGFLAG_WARNING|(flags)); \
^
./arch/x86/include/asm/bug.h:27:2: note: expanded from macro '_BUG_FLAGS'
asm_inline volatile("1:\t" ins "\n" \
^
./include/linux/compiler_types.h:281:24: note: expanded from macro
'asm_inline'
#define asm_inline asm __inline
^
./include/linux/xarray.h:1616:2: error: expected '(' after 'asm'
[clang-diagnostic-error]
BUG_ON(order > 0);
^
./include/asm-generic/bug.h:65:57: note: expanded from macro 'BUG_ON'
#define BUG_ON(condition) do { if (unlikely(condition)) BUG(); } while (0)
^
./arch/x86/include/asm/bug.h:66:2: note: expanded from macro 'BUG'
_BUG_FLAGS(ASM_UD2, 0); \
^
./arch/x86/include/asm/bug.h:27:2: note: expanded from macro '_BUG_FLAGS'
asm_inline volatile("1:\t" ins "\n" \
^
./include/linux/compiler_types.h:281:24: note: expanded from macro
'asm_inline'
#define asm_inline asm __inline
^
Found compiler error(s).
21 errors generated.
Error while processing /home/xxxxxxxx/linux/drivers/hid/hid-ezkey.c.
error: too many errors emitted, stopping now [clang-diagnostic-error]
error: unknown argument: '-fno-stack-clash-protection'
[clang-diagnostic-error]
error: unknown warning option '-Wno-frame-address'; did you mean
'-Wno-address'? [clang-diagnostic-unknown-warning-option]
error: unknown warning option '-Wno-pointer-to-enum-cast'; did you mean
'-Wno-pointer-compare'? [clang-diagnostic-unknown-warning-option]
Unless I did something wrong, this doesn't seem that useful to me
compared to what I get from scan-build?
Cheers,
Amadeusz