[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669 Xi Ruoyao changed: What|Removed |Added Severity|normal |enhancement --- Comment #7 from Xi Ruoyao --- (In reply to Zeb Figura from comment #6) > It is my impression that gcc is interested in avoiding false positives for > its warnings. Correct, but we are also interested in avoiding false negatives. Without extra information provided by something like __builtin_unreachable, any change decreasing false positives will increase false negatives (unless the false positive is completely stupid: for the simplified test case I think the false positive not completely stupid, but maybe it is completely stupid for your original program). > It is also my impression that -Wnonnull is not *supposed* to emit warnings > for cases where, from the compiler's point of view, NULL might be passed, > but some high-level invariant prevents this. Compare -Wmaybe-uninitialized, > where the documentation clearly specifies otherwise. Maybe we can separate -Wnonnull into -Wmaybe-nonnull and -Wnonnull, or just make -Wnonnull not to emit warnings for conditional paths and tell users expecting a nonnull warning in conditional paths to use the analyzer (it's very supposed to warn even in conditional paths) instead. > If both of these impressions are incorrect, this bug report can be closed as > WONTFIX. I'll keep it open but make it an enhancement.
[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669 --- Comment #6 from Zeb Figura --- It is my impression that gcc is interested in avoiding false positives for its warnings. This isn't to say that there aren't some number of false positives in existence, but it is my impression that gcc is interested in reducing that number. It is also my impression that -Wnonnull is not *supposed* to emit warnings for cases where, from the compiler's point of view, NULL might be passed, but some high-level invariant prevents this. Compare -Wmaybe-uninitialized, where the documentation clearly specifies otherwise. If both of these impressions are incorrect, this bug report can be closed as WONTFIX. (In reply to Xi Ruoyao from comment #5) > And you can tell the compiler some fact about the semantics of the Windoge > API functions if you really need -Werror=nonnull (though I cannot see any > reason you must use -Werror here): If it makes a difference, please feel free to pretend I said -Wnonnull, rather than -Werror=nonnull. It was merely a debugging aid, meant to help me try to narrow down the conditions causing this error.
[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669
--- Comment #5 from Xi Ruoyao ---
And you can tell the compiler some fact about the semantics of the Windoge API
functions if you really need -Werror=nonnull (though I cannot see any reason
you must use -Werror here):
int GetSystemDirectory16(char *path, int count)
{
if (GetWindowsDirectoryA()) {
/* For blah blah blah... reason
path will never be NULL if GetWindowsDirectoryA returns true */
if (!path)
__builtin_unreachable();
__builtin_strcpy(path, "foo");
}
return 0;
}
[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669 --- Comment #4 from Xi Ruoyao --- (In reply to Zeb Figura from comment #3) > (In reply to Xi Ruoyao from comment #2) > > (In reply to Xi Ruoyao from comment #1) > > > The warning given for the reduced test case is correct because it does not > > > make sense. It should be just rewritten as > > > > I mean, the code does not make sense. > > > > And the warning is given exactly because GCC is optimizing the strcpy call > > to unreachable. > > If GetWindowsDirectoryA() was idempotent, and GetSystemDirectory16() had no > other users, that might be true, but as it is I don't think so. > > The pattern both of those functions call is, much like snprintf(), you pass > a buffer and a size, and if the size is 0 then they'll return the size that > would have been written if there was a large enough buffer. In that case the > buffer can be NULL. In trying to reduce the test case down to the minimal > possible complexity I obscured that fact, but regardless I don't think the > reduced testcase is nonsensical. Then that's why -Wnonnull is a warning, not an error. Using -Werror is just telling the compiler "you can reject valid code", anyway.
[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669 --- Comment #3 from Zeb Figura --- (In reply to Xi Ruoyao from comment #2) > (In reply to Xi Ruoyao from comment #1) > > The warning given for the reduced test case is correct because it does not > > make sense. It should be just rewritten as > > I mean, the code does not make sense. > > And the warning is given exactly because GCC is optimizing the strcpy call > to unreachable. If GetWindowsDirectoryA() was idempotent, and GetSystemDirectory16() had no other users, that might be true, but as it is I don't think so. The pattern both of those functions call is, much like snprintf(), you pass a buffer and a size, and if the size is 0 then they'll return the size that would have been written if there was a large enough buffer. In that case the buffer can be NULL. In trying to reduce the test case down to the minimal possible complexity I obscured that fact, but regardless I don't think the reduced testcase is nonsensical.
[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669 --- Comment #2 from Xi Ruoyao --- (In reply to Xi Ruoyao from comment #1) > The warning given for the reduced test case is correct because it does not > make sense. It should be just rewritten as I mean, the code does not make sense. And the warning is given exactly because GCC is optimizing the strcpy call to unreachable.
[Bug middle-end/111669] bogus -Wnonnull in conditionally executed code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111669
Xi Ruoyao changed:
What|Removed |Added
CC||xry111 at gcc dot gnu.org
--- Comment #1 from Xi Ruoyao ---
The warning given for the reduced test case is correct because it does not make
sense. It should be just rewritten as
int GetSystemDirectory16(char *path, int count)
{
if (GetWindowsDirectoryA())
__builtin_unreachable();
return 0;
}
because in the case if GetWindowsDirectoryA() returns non-zero, the code always
invoke undefined behavior.
I'm not sure about the original file.c, but AFAIK GCC developers are keeping to
say "hey, don't use -Werror".
