Issue metadata
Sign in to add a comment
|
clang: negated const unsigned char is treated as int |
||||||||||||||||||||||||
Issue description
Building the v4.14 kernel with clang results in warnings/errors about implicit conversions:
USE="kasan clang" emerge-grunt -j chromeos-kernel-4_14
../../../../../tmp/portage/sys-kernel/chromeos-kernel-4_14-4.14.43-r295/work/chromeos-kernel-4_14-4.14.43/arch/x86/kvm/mmu.c:4266:39: error: implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from -205 to 51 [-Werror,-Wconstant-conversion]
u8 wf = (pfec & PFERR_WRITE_MASK) ? ~w : 0;
~~ ^~
../../../../../tmp/portage/sys-kernel/chromeos-kernel-4_14-4.14.43-r295/work/chromeos-kernel-4_14-4.14.43/arch/x86/kvm/mmu.c:4268:38: error: implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from -241 to 15 [-Werror,-Wconstant-conversion]
u8 uf = (pfec & PFERR_USER_MASK) ? ~u : 0;
~~ ^~
../../../../../tmp/portage/sys-kernel/chromeos-kernel-4_14-4.14.43-r295/work/chromeos-kernel-4_14-4.14.43/arch/x86/kvm/mmu.c:4270:39: error: implicit conversion from 'int' to 'u8' (aka 'unsigned char') changes value from -171 to 85 [-Werror,-Wconstant-conversion]
u8 ff = (pfec & PFERR_FETCH_MASK) ? ~x : 0;
~~ ^~
The code in question is:
https://elixir.bootlin.com/linux/v4.14.48/source/arch/x86/kvm/mmu.c#L4244
I narrowed it down to this repro case:
unsigned char test_ko(void)
{
const unsigned char w = 128;
return ~w;
}
The 'expansion' to int only occurs for const variables and values > 127.
Apparently clang treats the const variable like a literal, not taking its actual data type into account.
,
Jun 11 2018
I can identify where the warning is coming from (and have a local patch to silent the warning) but not sure of a correct fix. Opened an upstream bug https://bugs.llvm.org/show_bug.cgi?id=37771 meanwhile.
,
Jun 11 2018
,
Jun 13 2018
Nick also commented on the bug. So far it seems like that the warning is actually correct because of an implicit conversion and GCC also flags this. To silence the warning, an explicit cast return (unsigned char) ~w; can be added. Nick Desaulniers from comment #2) > gcc also flags these test cases with -Woverflow. I suspect the kernel code > should be re-evaluated for correctness in light of implicit integer > promotions. > > See also: > https://wiki.sei.cmu.edu/confluence/display/c/EXP14-C. > +Beware+of+integer+promotion+when+performing+bitwise+operations+on+integer+ty > pes+smaller+than+int
,
Jun 13 2018
Seems like GCC versions as old as 4.4 emit this warning. So is this warning disabled in kernel for GCC by adding -Wno-overflow flag somewhere? If so, maybe it makes sense to disable it for clang as well i.e. add -Wno-constant-conversion to compile-clang config?
,
Jun 13 2018
IIUC the difference with gcc is that clang emits the warning on const variables, but gcc only for literals or pre-processor constants. In my tests gcc doesn't generate the warning for the above code, even when invoked with -Woverflow.
,
Jun 13 2018
I actually see GCC generating the -Woverflow warning for the code in #0 https://godbolt.org/g/6JWfWk
,
Jun 13 2018
Interesting, Debians/gLinux gcc doesn't do that, I guess they specifically disabled it. It's probably worth a try to add the cast if gcc's default behavior is the same.
,
Jun 15 2018
,
Oct 2
Mka@ jus wondering what is the status for this? Did your/Nick fixes land in upstream kernel?
,
Oct 2
We tried to fix it upstream, but it was not accepted because "it would make the code uglier", instead it was suggested to disable the warning globally. I didn't proceed with this since I consider the warning potentially useful and there is only a small number of instances in the kernel where code would need to be 'fixed'. For the CrOS kernel we disabled the warning for the affected file only. We can close this issue, we don't expect any action from your team on this. |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by mka@chromium.org
, Jun 5 2018