Live data from Hacker News

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

lkml.org

41–50 of 67 posts

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

#41
post #38

Earlier quoted context omitted.

>It seems that the most efficient thing with %eax is not to mention it in an instruction. I think what you're missing is that UB is a property of the execution of the program , and not just of the code that was compiled. If in practice the if branch is always taken, then there is NO undefined behavior (surprisingly, perhaps)! That means GCC has to be prepared to handle that case, and has to set eax to 0 when the if b…

No, I mean the behavior is undefined in that case when the function returns the indeterminate value, which was initialized anyway. Look, GCC (what I have here: 7.3.0) is doing this even for the following trivial function: int undef(void) { int ret; return ret; } With -O2 this turns out: xorl %eax, %eax ret do we still know until run-time that this has UB?

>Look, GCC (what I have here: 7.3.0) is doing this even for the following trivial function:

You're absolutely right that in this case, it's wasteful. In fact, if you try Clang you will see that it only emits a ret.

Maybe GCC is doing you a courtesy (or maybe nobody taught it to completely omit an Undef return value, so it just picks zero!). Either way, it's unconditionally UB, GCC can do whatever it likes.

>do we still know until run-time that this has UB?

Well, there is no branch here, is there? I'm clearly not going to disagree with you on this one =]

To be clear: UB is still a property of the execution of the program, but here it's not hard to statically determine that all executions of this program are Undefined Behavior.

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

#42

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.

They were always meant for emphasis, which takes on variable definitions over time. Terminals were bad at italics due to character box layouts and so the literary approach of emphasis through italics mutated into emphasis through underlines. Now that we’re all free of monospaced terminal grids, it looks like we’re drifting back to emphasis through italics.

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

#43
post #37

Earlier quoted context omitted.

How do you gain a register by emitting xorl %eax, %eax versus simply not emitting that and simply letting the code access the garbage value that's already in %eax ?

So, if the if branch is taken, then the function could really return 0, right? So GCC has to have some instruction somewhere that ensures eax is 0 at the end of the function in case the if was taken. That's why it can't just leave a garbage value in eax. Sometimes it's not actually garbage. And when it really is garbage? Well who cares, might as well return 0.

I don't follow. If the branch on the inode link count is taken, then the external function shmem_reserve_inode is called; ret becomes its return value then. The only way the tail of the function runs with ret == 0 is if that external function happens to return zero. What I don't somehow see is the advantage of, or requirement for, preparing a zero value in ret that didn't come from shmem_reserve_inode, for a branch of the flow where UB is invoked which causes all behavioral requirements to be off the table.

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

#44
post #41

Earlier quoted context omitted.

No, I mean the behavior is undefined in that case when the function returns the indeterminate value, which was initialized anyway. Look, GCC (what I have here: 7.3.0) is doing this even for the following trivial function: int undef(void) { int ret; return ret; } With -O2 this turns out: xorl %eax, %eax ret do we still know until run-time that this has UB?

>Look, GCC (what I have here: 7.3.0) is doing this even for the following trivial function: You're absolutely right that in this case , it's wasteful. In fact, if you try Clang you will see that it only emits a ret. Maybe GCC is doing you a courtesy (or maybe nobody taught it to completely omit an Undef return value, so it just picks zero!). Either way, it's unconditionally UB, GCC can do whatever it likes. >do we st…

Moreover, it still does it with "-m32 -mtune=i386". :)

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

#45
post #37

Earlier quoted context omitted.

So, if the if branch is taken, then the function could really return 0, right? So GCC has to have some instruction somewhere that ensures eax is 0 at the end of the function in case the if was taken. That's why it can't just leave a garbage value in eax. Sometimes it's not actually garbage. And when it really is garbage? Well who cares, might as well return 0.

I don't follow. If the branch on the inode link count is taken, then the external function shmem_reserve_inode is called; ret becomes its return value then. The only way the tail of the function runs with ret == 0 is if that external function happens to return zero. What I don't somehow see is the advantage of, or requirement for, preparing a zero value in ret that didn't come from shmem_reserve_inode, for a branch o…

Oh, I see. It's really a behavior of x86 that's causing the confusion then. The gist of it is that ret is not eax. Let me explain.

When shmem_reserve_inode is called, under the x86 C ABIs (all of them!), it will place its return value in eax. So after the call, at this precise moment, "ret" is "eax", and we don't know it's value. Then we do "if (ret) return ret", after this line "ret" is still "eax", and this time we know for sure that its value is 0.

But eax is not bound to ret until the end of the function. EVERY function we call before returning is allowed to modify eax (whether that function call returns a result or not)!

And since we know that ret has the value 0, well we don't even need to keep the value in register eax, do we? We can just use eax for something else and not worry about storing "ret" anywhere, it's just a constant now.

>What I don't somehow see is the advantage of, or requirement for, preparing a zero value in ret that didn't come from shmem_reserve_inode

That's the subtelty. We don't prepare ret, we prepare eax.

Conceptually, at the C language level, at the tail of the function "ret" still contains 0. But at the x86 level, eax DOES NOT necessarily contain 0. (Because we call d_instantiate and other functions before returning, and they're allowed to modify eax).

So ret has the right value, but we still need to load the value of ret, into eax before returning. And the most efficient way to do that? Just set eax to 0.

I genuinely hope that helps =]

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

#46
post #45

Earlier quoted context omitted.

I don't follow. If the branch on the inode link count is taken, then the external function shmem_reserve_inode is called; ret becomes its return value then. The only way the tail of the function runs with ret == 0 is if that external function happens to return zero. What I don't somehow see is the advantage of, or requirement for, preparing a zero value in ret that didn't come from shmem_reserve_inode, for a branch o…

Oh, I see. It's really a behavior of x86 that's causing the confusion then. The gist of it is that ret is not eax. Let me explain. When shmem_reserve_inode is called, under the x86 C ABIs (all of them!), it will place its return value in eax. So after the call, at this precise moment, "ret" is "eax", and we don't know it's value. Then we do "if (ret) return ret", after this line "ret" is still "eax", and this time we…

I do understand that in all cases when the tail part of the function is executed such that it's well-defined (the only cases we care about), the return value is zero, which means it's effectively a constant. Since we have to prepare that return value in %eax (dictated by the ABI), then at some point we clear %eax; that's not semantically the same as initializing ret. If we do this later in the function, like just before returning, we can use %eax for other purposes in the meanwhile.

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

#47
post #45

Earlier quoted context omitted.

Oh, I see. It's really a behavior of x86 that's causing the confusion then. The gist of it is that ret is not eax. Let me explain. When shmem_reserve_inode is called, under the x86 C ABIs (all of them!), it will place its return value in eax. So after the call, at this precise moment, "ret" is "eax", and we don't know it's value. Then we do "if (ret) return ret", after this line "ret" is still "eax", and this time we…

I do understand that in all cases when the tail part of the function is executed such that it's well-defined (the only cases we care about), the return value is zero, which means it's effectively a constant. Since we have to prepare that return value in %eax (dictated by the ABI), then at some point we clear %eax; that's not semantically the same as initializing ret . If we do this later in the function, like just be…

>I do understand that in all cases when the tail part of the function is executed [...] then at some point we clear %eax;

>If we do this later in the function, like just before returning, we can use %eax for other purposes in the meanwhile.

Okay, I think we completely agree here!

>that's not semantically the same as initializing ret.

Hmm, so is it that you're asking why we're initializing ret? Well, that sounds like a good excuse as any for another incredibly long and boring wall of text =]

If I'm extra lucky I'll get called out as the amateur I am and learn something in the process (!)

---

So, here's the thing. Talking about the compiler initializing ret as a variable, separate from its storage in eax is not really what the compiler is trying to do, not as I understand it. We really aren't "initializing it" so much as trying to guess it's value in all possible executions (because that's just what compilers do these days).

The way the compiler works is that first it gets rid of the idea of variables that can be assigned multiple times, it switches to something called SSA form where every "variable" is initialized exactly once. Want to reassign a variable? Just declare one with a new name instead and use it going forward.

The funny part of SSA is that when there's a branch, after it joins back you end up having to define a variable that could have two possible values (branch taken, not taken). That's not something you should normally be able to do with the SSA rules, so it's represented by a special Phi "value" in the compiler's intermediate representation.

A Phi basically just says "either we came from path A and we have value Va, or from B and it's Vb".

But what the compiler really wants is to forget about the branch and the Phi business, and just deduce a plain value for ret so it can move on with cold hard numbers in mind.

Since we have undefined behavior here, we can simplify Phi into just [0, 0]. And that's how the compiler "forgets" that ret was uninitialized in the first place. It just optimizes the undefinedness away while trying to guess values, if you will.

So long story short we're not so much trying to intialize ret than we're trying to guess it's value, and forget that there were multiple branches in the first place.

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

#48
post #22

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…

Linux has support for kernel ASAN and I think UBSAN too. User mode linux is a also thing and does roughly what you described, and I think you can run it under valgrind too.

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

#49
post #35

Earlier quoted context omitted.

>No it isn't; it costs an extra instruction to initialize that register or memory location to zero. If you're talking about the "xor eax, eax" that was emitted, then absolutely not . GCC is doing the most efficient thing possible with eax here. That xor is not inserted because GCC kindly initializes the variable for you to save you from your mistakes, but because it has to initialize eax before returning (look, there…

> GCC is doing the most efficient thing possible with eax here. It seems that the most efficient thing with %eax is not to mention it in an instruction. There may be some quirk/feature of modern Intel processors that clearing a register will dis-entangle it from considerations of prior hazards. So that is to say, when we execute 'xorl %eax, %eax', the processor knows that any prior value in `%eax` is no longer requir…

> There may be some quirk/feature of modern Intel processors

not so much of a quirk, but basic behaviour of any OoO processor due to register renaming. This is true on the vast majority of intel (and AMD) cpus (in fact probably all the currently sold ones as even atom has acquired a limited OoO engine and the knight variants are no more).

It is so fundamental that, as described elsethread zeroing a register is almost free (it still costs decode bandwidth and icache size).

Bottom line in the vast majority of cases, zeroing the registers is almost always a win.

edit: having said that I doubt that gcc is trying to optimize the UB case, probably it the optimizer just requires that the variable has a value at that point.

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

#50
post #22

Earlier quoted context omitted.

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…

Linux has support for kernel ASAN and I think UBSAN too. User mode linux is a also thing and does roughly what you described, and I think you can run it under valgrind too.

To be fair, UML runs at such a low level (raw syscalls rather than calls to, for instance, malloc and free) that it really limits what valgrind can tell you.
Post reply on HN