[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 Matthieu Brucher changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|WORKSFORME |--- --- Comment #20 from Matthieu Brucher --- Reopening, seriously!
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #19 from Matthieu Brucher --- That was my original comment... Thanks for quoting me... The issue is that the warning doesn't tell me what you told me, that "this" is never equal to nullptr in that context. This is what the warning should be (and as such, yes, it is a bug). This change in behavior (this is always different from nullptr) was introduced in gcc 6. Any version before would still check that the condition is true, so GCC behaves better now, handling better the case than before, but it still needs to educate people that what they are doing is wrong.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #17 from Matthieu Brucher --- Not everyone runs this sanitizer, and not everyone has unit tests that can find this error (the application I woked on that had this has numerous unit tests, but this was not tested because the guy that wrote them forgot about this undefined behavior or something similar). It was discovered by chance on a product using it and tracked down. Once again, a warning about conditions that are always true or false would capture i, I don't know if gcc has it and if it would warn properly.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #15 from Matthieu Brucher --- Thanks, finally something relevant, yes it's a bug that can be easilly checked by gcc and that GCC can easilly warn about. (lowering the bar here). Yes, the user should chase the bug because they screwep on a monumental level. But why can't gcc warn that the user is about to do something stupid or tell them that the comparison is always true? Of course, if there is a warning level that already does that, my comment is moot (although making it a higher level warning may help).
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 Matthieu Brucher changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|INVALID |--- --- Comment #12 from Matthieu Brucher --- Reopening, either this == nullptr is always false and then debug mode should be changed accordingly, or add a warning. Some consistency is required here.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #11 from Matthieu Brucher --- Oh, and if this is always false, why isn't it the case in debug mode? It is then a bug according to what you said.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #10 from Matthieu Brucher --- If it is always false, what impeds you for warning as such? Because numerous people are checking this (and I agree that they shouldn't), as proven by my 2 links.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928
--- Comment #8 from Matthieu Brucher ---
In a header:
class Foo
{
public:
void bar();
};
Inthe corresponding source:
#include
void Foo::bar()
{
if(this)
{
std::cout << "Pointer is not null";
}
else
{
std::cout << "Pointer is null, this should not happen, undefined behavior";
}
}
in main.cpp
int main(int argc, char** argv)
{
Foo foo;
foo.bar();
Foo* foo2 = nullptr;
foo2->bar();
return 0;
}
In debug mode, you get once the Pointer is not null and then the other display,
in optimized mode, you get twice the first one.
What is fundamentally wrong is stated in the standard. It is an undefined
behavior to call a method from a null pointer. So testing this inside a method
is an undefined behavior: https://www.viva64.com/en/w/V668/ or even better
https://www.viva64.com/en/b/0226/ since at the time it is clear that gcc didn't
use this perfectly fine optimization.
Once again, GCC acts on this undefined behavior by setting the comparison to
false in optmized mode, which is perfectly fine. The problem is doing so
_silently_, as I said in my original message. Especially since now GCC acts on
this in a non-classic way.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #6 from Matthieu Brucher --- I never said that the test alone should be banned. Please read the original message first. I said that if(!this) in the context of a method gives "unexpected" behavior (according to the standard and the difference in behavior of GCC between debug and optimized mode) and thus should give an error or at least a warning, and this is easy to catch. At least make it a warning, like clang does when it detects that a path is always true or false, some kind of way for the user to _know_ that they messed up. The warning is not complicated to do as that's what the optimizer deduces given the constraints. And yes, there is something fundamentally wrong with comparing this to nullptr according to the C++ standard and the contract GCC has in its optimizer. Or give a reason why it is valid and where the standard says it supercedes the undefined behavior and the thread discussion on why GCC added the optimization constraint. And in that case, you should probably remove the optimization as well to be consistent.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #4 from Matthieu Brucher --- I would agree if the debug and optimized versions had the same behavior. They do not. As such there should be a huge warning about the undefined behavior. And no, it's not valid code, it's an undefined behavior, you can do whatever you want with it. Outputting an error is a better solution, even adding a warning is. Silently optimizing an undefined behavior away is not.
[Bug c++/81928] if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 --- Comment #2 from Matthieu Brucher --- No, I think the optimization is more than correct. The warning doesn't warn of the actual problem, if you check against nullptr, you won't even get a warning. Comparing this to anything from Null to nullptr should be an error, not be silently passed. Some people still think that it is valid because it worked in the past and because it's a bad anti pattern. So yes, it should be rejected because it's only point is to compare the pointer to nullptr, which is not sane. And it must be rejected because gcc in optimized mode considers this to be always not nullptr, and doesn't even warn that the expression is thus constant. The user doesn't know that he screwed big time.
[Bug c++/81928] New: if(!this) optimization leads to possible errors without warnings
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81928 Bug ID: 81928 Summary: if(!this) optimization leads to possible errors without warnings Product: gcc Version: 7.1.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c++ Assignee: unassigned at gcc dot gnu.org Reporter: matthieu.brucher at gmail dot com Target Milestone: --- The constructs if(!this) in a method raises a warning about comparison with NULL, as it should. We agree that this is an undefined behavior at its worst. The issue is that the behavior of this condition is not the same between debug and optimized where the call is considered at compile time. Even if there is an existing warning on this undefined behavior, this should be an error (or a warning no matter what) as there is a different behavior between debug and optimized (even if they both make sense).
