Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

GCC proves an uninitialized variable must be 0 and doesn't warn about it

lkml.org

Post reply on HN
Print view View original

21–30 of 67 posts

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#21

Post by [deleted] »

atq2119 wrote: post #18
tux3 wrote: post #12

Earlier quoted context omitted.

The compiler is allowed to do that. Since it would be undefined behavior to read the undefined variable, the compiler gets to pick whatever value it wants for it. So after the if, the compiler sees that ret is either 0, or an undefined value. It picks 0 as the undefined value since that's a good optimization, so ret is now always 0, and (in this case) there is no warning.

Right, the bug in the kernel code snippet is like GP describes, but the gcc bug isn't that the compiled code is incorrect, it's that people expect an "uninitialized variable" warning and the compiler doesn't produce one.

[deleted]

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#22

Post by mhh__ »

wyldfire wrote: post #13

Aside: static analysis is a great initial effort but to really get confidence you should use techniques like sanitizers (MSan in this case) or valgrind. (This is general advice, not always applicable to kernels).

You should really use both, however they are only a tool: I don't have any numbers but I imagine Sanitizers have a good detection rate, but one should still write code defensively. There are too many horror stories, be they lethal or expensive, to be risky about memory.

Not a kernel developer: Can you test Kernel code with a sanitizer (if that sanitizer doesn't work in a compiled kernel) by compiling into and using the code as blob in userspace code with mocks (Or whatever, unless it's just utility that doesn't need anything other than an interface)?

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#23

Post by wtracy »

wyldfire wrote: post #13

Aside: static analysis is a great initial effort but to really get confidence you should use techniques like sanitizers (MSan in this case) or valgrind. (This is general advice, not always applicable to kernels).

By msan, are you referring to this? https://clang.llvm.org/docs/MemorySanitizer.html

(I'd never heard of the tool before.)

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#24

Post by firethief »

ncmncm wrote: post #20

What I don't understand is why it doesn't optimize away the check on (inode->i_nlink), since it is assuming the code in the dependent block always runs. It has to be assuming that, because it is the only path that doesn't lead to undefined behavior.

The optimizer isn't required to eliminate code paths encountering dynamic UB; it is allowed to. It also doesn't have to be consistent in its "assumptions", although inconsistency may be a sign of missing efficiency somewhere

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#25

Post by saagarjha »

mhh__ wrote: post #22
wyldfire wrote: post #13

Aside: static analysis is a great initial effort but to really get confidence you should use techniques like sanitizers (MSan in this case) or valgrind. (This is general advice, not always applicable to kernels).

You should really use both, however they are only a tool: I don't have any numbers but I imagine Sanitizers have a good detection rate, but one should still write code defensively. There are too many horror stories, be they lethal or expensive, to be risky about memory. Not a kernel developer: Can you test Kernel code with a sanitizer (if that sanitizer doesn't work in a compiled kernel) by compiling into and using t…

Yup, Address Sanitizer can hide or fail to catch bugs occasionally.

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#26

Post by saagarjha »

dullroar wrote: post #17
icedchai wrote: post #15

Earlier quoted context omitted.

It's for emphasis. Some people use asterisks. He uses underscores.

Note he uses both, even in the same paragraph - asterisks typically mean "bold," whereas underscores typically mean "underlined."

Except in Slack, which somehow interprets this to be italics.

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#27

Post by kazinator »

tux3 wrote: post #12
zzo38computer wrote: post #2

It look like a bug to me, since inode->i_nlink may be zero, and then ret is uninitialized (optimizing the final "return ret" to "return 0" should be safe (but is not necessarily worth it, depending on the target instruction set and ABI), although if warnings are enabled, it seem like the warning should still be displayed).

The compiler is allowed to do that. Since it would be undefined behavior to read the undefined variable, the compiler gets to pick whatever value it wants for it. So after the if, the compiler sees that ret is either 0, or an undefined value. It picks 0 as the undefined value since that's a good optimization, so ret is now always 0, and (in this case) there is no warning.

> It picks 0 as the undefined value since that's a good optimization.

No it isn't; it costs an extra instruction to initialize that register or memory location to zero.

How C compilers optimize situations involving uninitialized variables is by leaving those variables alone. They do so with the standard's blessing, which grants them that uninitialized automatic variables are "indeterminately-valued" and that if the program uses an indeterminate value, its behavior is undefined. Correctly written programs that perform their own delayed initialization by later assignment benefit from the fact that the compiler didn't add wasteful code to initialize those objects, only to have that compiler-generated initial value be overwritten.

> ret is now always 0, and (in this case) there is no warning.

It's not correct for diagnostics to pertain to versions of the program that were logically altered by the compiler, rather than to the original program. It may be ISO-conforming, only because ISO C doesn't require those diagnostics in the first place, let alone requiring them to be reliable.

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#28

Post by rurban »

wtracy wrote: post #23
wyldfire wrote: post #13

Aside: static analysis is a great initial effort but to really get confidence you should use techniques like sanitizers (MSan in this case) or valgrind. (This is general advice, not always applicable to kernels).

By msan, are you referring to this? https://clang.llvm.org/docs/MemorySanitizer.html (I'd never heard of the tool before.)

Yes, this is msan. All the clang sanitizers have handy names, like tsan, msan, asan, ubsan, ksan. And they even made it to gcc.

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#29

Post by wyldfire »

mhh__ wrote: post #22
wyldfire wrote: post #13

Aside: static analysis is a great initial effort but to really get confidence you should use techniques like sanitizers (MSan in this case) or valgrind. (This is general advice, not always applicable to kernels).

You should really use both, however they are only a tool: I don't have any numbers but I imagine Sanitizers have a good detection rate, but one should still write code defensively. There are too many horror stories, be they lethal or expensive, to be risky about memory. Not a kernel developer: Can you test Kernel code with a sanitizer (if that sanitizer doesn't work in a compiled kernel) by compiling into and using t…

> but one should still write code defensively.

I'm not sure what this means. You should not write your code any differently regardless of whether you used a sanitizer to help gain confidence in your implementation.

You should not use sanitizers in production (at least not the current generation).

> Can you test Kernel code with a sanitizer

In fact you can! Both UBSan [1] and ASan [2] are in use to root out bugs in linux.

[1] https://www.kernel.org/doc/html/v4.14/dev-tools/ubsan.html

[2] https://www.kernel.org/doc/html/v4.14/dev-tools/kasan.html

Re: GCC proves an uninitialized variable must be 0 and doesn't warn about it

#30

Post by naikrovek »

saagarjha wrote: post #26
dullroar wrote: post #17

Earlier quoted context omitted.

Note he uses both, even in the same paragraph - asterisks typically mean "bold," whereas underscores typically mean "underlined."

Except in Slack, which somehow interprets this to be italics.

Markdown started that in general. There's no way to underline something in Markdown; underlining just isn't supported.
Post reply on HN