This is GCC Bugzilla
This is GCC Bugzilla Version 2.20+
View Bug Activity | Format For Printing | Clone This Bug
when a coder writes (erroneously) such a code: char *m_strrtrim(char *s) __attribute__((nonull(1)); char *m_strrtrim(char *s) { int len = s ? strlen(s) : 0; while (len > 1 && isspace((unsigned char)s[len - 1])) len--; return s + len; } Then gcc uses the __attribute__((nonnull(1)) — which again is a programming mistake — to optimize the check of s beeing NULL or not. That is very correct from a compiling point of view, but it generated segfaults in my code, that I had a very hard time to find, because of it beeing in the header file rather than in the implementation where I looked for it (as the backtrace pointed me in that function). I suppose that gcc do the optimization because it knows that 's' is non NULL, though it should make a distinction between s beeing non NULL because it knows so (e.g. because s is a local buffer) or because it comes from a programmer assertion. When it's the latter, it should warn about any trivial test, like it does when you test if an unsigned int is greater or equal to 0 for example. What I mean is that: __attribute__((nonull(1))) void foo(char *s) { if (!s) { if (!s) { // do sth; } } } here, the first test on s SHOULD NOT be optimized silently, because at this point s is marked as beeing NONNNUL thanks to a /programmer/ assertion, not constant folding. I don't know for the second though, maybe it's worth to warn, maybe not.
nonnull attribute to the function says the function's argument is non null at the time we enter the function so assuming that is correct. Also this is documented this way: nonnull (arg-index, ...) The nonnull attribute specifies that some function parameters should be non-null pointers. For instance, the declaration: extern void * my_memcpy (void *dest, const void *src, size_t len) __attribute__((nonnull (1, 2))); causes the compiler to check that, in calls to my_memcpy, arguments dest and src are non-null. If the compiler determines that a null pointer is passed in an argument slot marked as non-null, and the -Wnonnull option is enabled, a warning is issued. The compiler may also choose to make optimizations based on the knowledge that certain function arguments will not be null. Witeness the last sentence.
Please, I'm not telling the behaviour is crazy, it's indeed correct. I'm just asking for a smallish warning that I may be shooting myself in the foot when I do sth like my 'foo' function from the bug report. When you do : size_t i; // ... if (i >= 0) { ... if (i < 0) { ... gcc issues a warning to tell me that I'm doing a test that will always be true (or always be false). I'd just like to have the same when I've specified that a parameter is nonnull and that I nontheless tries to see if it's NULL or not. You're not answering to what I ask, I've read the documentation, and I've already acknowledged that gcc optimizations in presence of the nonnull attribute are legitimate.
(In reply to comment #2) > Please, I'm not telling the behaviour is crazy, it's indeed correct. > > I'm just asking for a smallish warning that I may be shooting myself in the > foot when I do sth like my 'foo' function from the bug report. > > When you do : > > size_t i; > > // ... > > if (i >= 0) { ... > if (i < 0) { ... And this optimization happens way after we are done parsing so ... I don't think a warning is the right thing in this case because the developer should have read the documention and it is harder to give a warning in this case really as this optimization is done currently only in the VRP so we don't know if it is from *a = 1; if (!a) ... of just from nonnull.
Andrew, please don't close enhancement requests WONTFIX because you think they're too hard to implement. There is no harm in leaving them open in case someone decides that it's not too hard and they're going to code it. Also, please don't insult our users by implying they didn't read the documentation, especially when the immediately previous message demonstrates beyond question that they did. Actually, just quit insulting the users. OK?
The main issue I have emitting a warning here is that it will produce a bunch of false positives for an example: static int f(int *a) { if (a) return *a; return -1; } int g(int *a, int c) __attribute__((nonnull(1) )); int g(int *a, int c) { if (c) return f(a); return -1; } int h(int *a, int c) { if (c) return f(a); return -1; } Or even f is a define: #define f(a) ({int *b_ = a; int t = -1; if(b_) t = *b_; t;}) Also we don't want to warn for: int g(int *a, int c) __attribute__((nonnull(1) )); int *x; int g(int *a, int c) { if (!c) a = x if (a) return *a; return c; } This shows how very fragile this warning will have to be to be able to not to warn in those cases. The unsigned warning is very fragile when it comes to macros (and templates) also but it is not as fragile as this warning as it is not flow sensative.
Subject: Re: __attribute__((nonull(...))) and silent optimizations Well, it's just like 'may be used uninitialized' false positives, isn't it? The warning shouldn't issue from the VRP pass, we should have some kind of data-flow-dependent warning pass that operates really, really early (though still on language-independent IL) so that we can have nice predictable rules about what it will and will not catch... Anyway that's how I'd do it. Which I am not volunteering to do, mind.
Subject: Re: __attribute__((nonull(...))) and silent optimizations Also: my main concern here is not the technical details of the feature but my dislike for your tendency to blow off bug reports that you think are bogus and insult the users who reported them. That's not the public face the project should present.
*** This bug has been marked as a duplicate of 17308 ***