5 ms·

Ouch, this is annoying. But it's a bug. Sometimes those happen, and they get fixed. This is totally unlike the user hostility that results from playing gotcha

Ouch, this is annoying. But it's a bug. Sometimes those happen, and they get fixed. This is totally unlike the user hostility that results from playing gotcha with UB which has also resulted in removing null pointer checks. Those aren't bugs and don't get fixed.

Gibbon1 7y ago
This is more than annoying this is unacceptable and the people responsible need to be stopped.
- saagarjha 7y ago
  Relax, it's an honest bug in open source software and they're working on fixing it (edit: they've already fixed it in trunk). What more do you expect?
  - Gibbon1 7y ago
    I expect them to stop playing with fire with useless UB optimizations.
    - saagarjha 7y ago
      This bug has nothing to do with undefined behavior, as many of the other commenters (including myself) have pointed out.
      - temac 7y ago
        I'm curious why you think that? The origin is absolutely the usual exploitation of UB in the sense that the compiler supposes that code paths that are past one can be elided (because programmers never write bugs, that kind of interesting reasoning...) Except it has a bug (the irony :p ) in the logic that computes that, and actually elides more code paths than it should. That shows that their architecture to do that kind of dangerous transformation is fragile, and they are not able to guarantee that that kind of transformation is really restricted to buggy source code (which would even then still be a bad idea IMO, but that is another story).
        saagarjha 7y ago
        There's no undefined behavior, though. The code provided is well formed: it passes NULL to a function that checks for it, and the NULL check is being removed erroneously because GCC thinks it's not necessary. Here's an explanation of the bug: https://news.ycombinator.com/item?id=20822378 https://news.ycombinator.com/item?id=20822378
        temac 7y ago
        Of course there is no UB in the source code. That makes the problem worse, not less bad... Non-nullness is inferred from 'node->child = NULL;' which would be UB would node be NULL. At this point the compiler thinks it is superfluous to check whether 'node' is null and start to propagates that, except it messes up. It seems that it messes up extremely badly because even -fno-delete-null-pointer-checks does not seem to fix the issue :/
        saagarjha 7y ago
        > Non-nullness is inferred from 'node->child = NULL;' which would be UB would node be NULL. There’s also a dereference inside of walk, except it’s protected by a NULL check.
        temac 7y ago
        Yes I see that, I was not describing what the compiler should observe to work properly, I was describing the erroneous reasoning it was making prior to the fix. I should have written: non-nullness of 'node'. And TBH I'm starting to be more concerned by the fact that fno-omit-null-pointer-checks did not suppress that codegen bug than by its existence in the first place...
        saagarjha 7y ago
        > I was describing the erroneous reasoning it was making prior to the fix. I should have written: non-nullness of 'node'. I think you're misinterpreting the error in the compiler's reasoning: unless I am misunderstanding you, it doesn't seem to match the comment I linked earlier. The reason -fno-delete-null-pointer-checks did not help is because it only kicks in when you use a NULL pointer in a way that is undefined, but that's not what's occurring here.
      - rocqua 7y ago
        Only up to a point. This bug is a misfiring attempt at detecting undefined behavior to allow for optimizations. If we did not attempt to remove null pointer checks, that would have prevented this bug in GCC. Hence, the practice of optimizing based on UB is relevant here, even if this was a compiler bug, rather than a compiler optimizing based on UB.
        saagarjha 7y ago
        > This bug is a misfiring attempt at detecting undefined behavior to allow for optimizations. I don't think it is. It's the same as removing the conditional here: int foo = 1; if (foo) { // whatever } The compiler (incorrectly) deduced that the pointer was non-NULL and removed the check because it saw it as unnecessary.
        jcranmer 7y ago
        > This bug is a misfiring attempt at detecting undefined behavior to allow for optimizations. It's closer to a conditional constant propagation that was accidentally applied unconditionally. The code was guarded by an if statement that asserted that a variable did not have a particular value, which lets the compiler delete further checks that the variable does or does not have the value. No UB behavior in play, unless you're complaining about the UB of "variables can't be changed unless the programmer changes them directly or indirectly." If you pay attention to the change that introduced the bug--it was a change in the constant propagator that caused the bug.
a1369209993 7y ago
Those are bugs and the compiler developers' refusal to fix them is precisely the user hostility you are complaining about.
- Thorrez 7y ago
  Whether it's a bug or not depends on what the desired behavior is. People disagree on what the desired behavior, thus people disagree about whether they are bugs. If your desired behavior is to obey the spec and run as quickly as possible then they are not bugs. The user hostility could be argued to be in the spec.
- ogoffart 7y ago
  No they are not. When your program invoke UB, it is a bug in your program, and not in the compiler. The compiler author are helping you with things like -fsanitize=undefined. Or even lets you define some behavior which is normaly undefined with things like -fwrapv, -fno-strict-aliasing, -fno-delete-null-pointer-checks But you are asking the compiler to optimize your program, if you give an invalid program to optimize, you get an invalid result. There is no bug there.

Gibbon1 7y ago

This is more than annoying this is unacceptable and the people responsible need to be stopped.

saagarjha 7y ago
Relax, it's an honest bug in open source software and they're working on fixing it (edit: they've already fixed it in trunk). What more do you expect?
- Gibbon1 7y ago
  I expect them to stop playing with fire with useless UB optimizations.
  - saagarjha 7y ago
    This bug has nothing to do with undefined behavior, as many of the other commenters (including myself) have pointed out.
    - temac 7y ago
      I'm curious why you think that? The origin is absolutely the usual exploitation of UB in the sense that the compiler supposes that code paths that are past one can be elided (because programmers never write bugs, that kind of interesting reasoning...) Except it has a bug (the irony :p ) in the logic that computes that, and actually elides more code paths than it should. That shows that their architecture to do that kind of dangerous transformation is fragile, and they are not able to guarantee that that kind of transformation is really restricted to buggy source code (which would even then still be a bad idea IMO, but that is another story).
      - saagarjha 7y ago
        There's no undefined behavior, though. The code provided is well formed: it passes NULL to a function that checks for it, and the NULL check is being removed erroneously because GCC thinks it's not necessary. Here's an explanation of the bug: https://news.ycombinator.com/item?id=20822378 https://news.ycombinator.com/item?id=20822378
        temac 7y ago
        Of course there is no UB in the source code. That makes the problem worse, not less bad... Non-nullness is inferred from 'node->child = NULL;' which would be UB would node be NULL. At this point the compiler thinks it is superfluous to check whether 'node' is null and start to propagates that, except it messes up. It seems that it messes up extremely badly because even -fno-delete-null-pointer-checks does not seem to fix the issue :/
        saagarjha 7y ago
        > Non-nullness is inferred from 'node->child = NULL;' which would be UB would node be NULL. There’s also a dereference inside of walk, except it’s protected by a NULL check.
        temac 7y ago
        Yes I see that, I was not describing what the compiler should observe to work properly, I was describing the erroneous reasoning it was making prior to the fix. I should have written: non-nullness of 'node'. And TBH I'm starting to be more concerned by the fact that fno-omit-null-pointer-checks did not suppress that codegen bug than by its existence in the first place...
        saagarjha 7y ago
        > I was describing the erroneous reasoning it was making prior to the fix. I should have written: non-nullness of 'node'. I think you're misinterpreting the error in the compiler's reasoning: unless I am misunderstanding you, it doesn't seem to match the comment I linked earlier. The reason -fno-delete-null-pointer-checks did not help is because it only kicks in when you use a NULL pointer in a way that is undefined, but that's not what's occurring here.
    - rocqua 7y ago
      Only up to a point. This bug is a misfiring attempt at detecting undefined behavior to allow for optimizations. If we did not attempt to remove null pointer checks, that would have prevented this bug in GCC. Hence, the practice of optimizing based on UB is relevant here, even if this was a compiler bug, rather than a compiler optimizing based on UB.
      - saagarjha 7y ago
        > This bug is a misfiring attempt at detecting undefined behavior to allow for optimizations. I don't think it is. It's the same as removing the conditional here: int foo = 1; if (foo) { // whatever } The compiler (incorrectly) deduced that the pointer was non-NULL and removed the check because it saw it as unnecessary.
      - jcranmer 7y ago
        > This bug is a misfiring attempt at detecting undefined behavior to allow for optimizations. It's closer to a conditional constant propagation that was accidentally applied unconditionally. The code was guarded by an if statement that asserted that a variable did not have a particular value, which lets the compiler delete further checks that the variable does or does not have the value. No UB behavior in play, unless you're complaining about the UB of "variables can't be changed unless the programmer changes them directly or indirectly." If you pay attention to the change that introduced the bug--it was a change in the constant propagator that caused the bug.

a1369209993 7y ago

Those are bugs and the compiler developers' refusal to fix them is precisely the user hostility you are complaining about.

Thorrez 7y ago
Whether it's a bug or not depends on what the desired behavior is. People disagree on what the desired behavior, thus people disagree about whether they are bugs. If your desired behavior is to obey the spec and run as quickly as possible then they are not bugs. The user hostility could be argued to be in the spec.
ogoffart 7y ago
No they are not. When your program invoke UB, it is a bug in your program, and not in the compiler. The compiler author are helping you with things like -fsanitize=undefined. Or even lets you define some behavior which is normaly undefined with things like -fwrapv, -fno-strict-aliasing, -fno-delete-null-pointer-checks But you are asking the compiler to optimize your program, if you give an invalid program to optimize, you get an invalid result. There is no bug there.