5 ms·

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 i

• atq2119 8y ago
  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 8y ago
    [deleted]
• kazinator 8y ago
  > 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.
  • hyperman1 8y ago
    Actuallym it is: You're right to need an extra instruction, but you gain a register. Storing and reloading this register would take instructions too. I threw this in godbolt, so play around, e.g. initialize ret or invert the condition. Linus' version has 36 lines, most other things I tried end up with 40 or more. https://godbolt.org/z/xiDGam https://godbolt.org/z/xiDGam Update: Added -Wall to godbolt
    • kazinator 8y ago
      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?
      • tux3 8y ago
        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.
        • kazinator 8y ago
          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.
          • tux3 8y ago
            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 =]
• Taniwha 8y ago
  No, it picks 0 because that's required for the code path where the "if (ret) return ret;" doesn't return, for all the other code paths where it reaches the end of the subroutine ret is undefined so 0 is good for all possible paths to that point
  • tux3 8y ago
    Since I'm on a comment spree today: I think we mostly agree, actually, but note that the compiler doesn't have to pick 0 for the undefined value at all. It only does that because that's the most convenient value to pick. (And for the code path where it's deduced to be 0, well it's not really picking, it doesn't have a choice =] ) A compiler with less optimizations could chose to spill the "ret" variable and not pick a value at all, for example. It would just return whatever happens to be lying on the stack if you took the unitialized path.
    • Taniwha 8y ago
      yes I agree, and it's optimised out a stack location for ret so no stack goop to default to My point really is that there are two interesting paths here, one where it's undefined and one where it's 0 ... I guess my point is that 0 is more than just 'convenient' because it's required for one of those paths
      • tux3 8y ago
        Fair enough, I can see your point =]