(I assume this goes into the category 'web', I couldn't find one for 'documentation')... The documentation for the 'nonnull' attribute (section 5.27) currently says: "The compiler may also choose to make optimizations based on the knowledge that certain function arguments will not be null". This is somewhat misleading in that it doesn't quite convey the effects that may result from incorrect use of nonnull, and in particular the fact that while analysis and issuing of warnings on nonnull is performed by the front-end, use of nonnull for code-generation is done by the back-end. As a result there can be cases where no warning is issued because the analysis required to reveal this would have to be done by the back-end, but the back-end optimiser still changes the code that it generates under the assumption that the pointer is never null. Obviously this can be excused by saying that it's the programmer's fault for applying the attribute incorrectly, but if its primary use is as a code- diagnosis tool then the programmers may not be aware of the sometimes drastic code-generation side-effects. To make developers aware of this issue it'd be useful to amend the docs to append to the above sentence the additional text: "Note that the use of 'nonnull' to generate warnings and to generate code are performed by different passes of the compiler. The optimiser may completely remove sections of code (for example checks for a pointer being null) if it encounters a parameter with the 'nonnull' attribute set. This attribute should therefore be used with care".
This is actually a lot more serious than just a documentation issue (I've changed it from a doc issue to a gcc issue) in that the use of this annotation by the optimizer will break correctly-functioning code by silently removing any checks for NULL pointers. To summarise the original issue: There are cases where no warning about use of a NULL pointer is issued because the analysis required to reveal this would have to be done by the back-end, but the back-end optimiser still changes the code that it generates under the assumption that the pointer is never null. In other words use of this annotation to warn about NULL pointers results in code that segfaults if the pointer ever does happen to be NULL, even though the compiler doesn't (reliably) warn about use of a NULL pointer in the first place. Because of this it's just too dangerous to use, if the compiler can't reliably warn about use of NULL pointers then it shouldn't then use this unreliable information to change the behaviour of generated code. The problem is compounded by the way the attribute is used, for __unused__ the attribute is placed in front of the variable that it affects but for __nonnull__ it's necessary to tediously hand-count parameters and then provide a list of index values for the attribute. When annotating a function with several pointers, some of which can be null and some of which can't, it's very easy to lose track and supply an off-by-one index, which gcc uses to remove NULL pointer checks resulting in segfaults in apparently-correct code. Is there any reason why __nonnull__ can't work the same way as __unused__?
No this is not a non documentation bug as non-null attribute says that the argument has to be non null to work correctly. So if you pass a NULL argument to the function, it will crash. So optimizing out the check for NULLness is correct really as the function is already expecting nonNULL.
It's more than just a documentation bug, two different portions of gcc are interpreting this attribute in completely different ways, and the interaction between the two breaks otherwise perfectly valid code. The intended use of the nonnull attribute is to warn, at compile time, against the inadvertent use of NULL pointers where they should be nonnull. By overloading it to also change the way code generation works gcc is making it far too dangerous to use and more or less destroying its usefulness for its original application - a single error in counting parameters or applying the annotation and your otherwise fully correct code suddenly breaks. In fact gcc seems to be doing the opposite of what ISO WG 14 is proposing for this attribute, which was to add extra checking to make sure the attribute isn't NULL. gcc instead *removes* extra checking to make sure the attribute isn't NULL. As it stands, gcc's interpretation of nonnull is unsafe at any speed, it doesn't reliably warn about NULL pointers, but it does reliably break your code when used.
The GCC attribute is an extension. What it is about is saying if it is NULL then the program/function will crash and is not expecting a NULL argument. So if you mark a function argument as non NULL, then the optimizer can optimize it based on that fact. Even though the C standards committee is proposing something different, that will be handled differently from the GCC attribute extension anyways. So again there is no changed needed really to the GCC extension as it is defined as one that will crash if supplied with a NULL argument. Think printf with %s where it is supplied with a NULL argument or puts with a NULL argument.
>Think printf with %s where it is supplied with a NULL argument or puts >with a NULL argument. Sure, I understand how that part of it works, but gcc doesn't just use it for that, it applies two often mutually exclusive interpretations to the same attribute. Warning about inadvertent use of NULL is useful for developers, so there's a temptation to annotate code with it to warn at compile time of errors (and that seems to be the intended use of stdc_nonnull). However the opposite interpretation of the attribute is that if you do inadvertently pass a NULL pointer to annotated code, gcc may not warn about it but will cause your code to crash. The single attribute shouldn't be used to perform two mutually exclusive (and in one case, dangerous) actions, it's like storing rat poison in a fruit juice bottle...
>gcc doesn't just use it for that, it applies two often mutually exclusive interpretations to the same > attribute. They are no mutually exclusive at all. Think of it is this way. The developer of the API says that it must be non-NULL so when the developer of the API then tests for NULLness he is either being stupid or really just thinking that the user will not use it correctly but since the warning is there, there is no reason for the function itself to test for NULLness which is why GCC optimizes away the check.
>The developer of the API says that it must be non-NULL so when the developer >of the API then tests for NULLness he is either being stupid or really just >thinking that the user will not use it correctly but since the warning is >there But the warning *isnt't* there, gcc doesn't reliably warn about incorrect use of a NULL pointer. If gcc could reliably detect use of a NULL pointer and warn about it, and *then* remove the NULL pointer checks that'd be fine. At the moment gcc doesn't warn about NULL pointer use but does remove code that would catch inadvertent NULL pointer use, and that's my complaint with the way the attribute works - it's what James Reason (who specialises in safety engineering) would call a latent pathogen.
This works as designed (and documented). Whether the design is the best possible one is another question.
Subject: Re: Use of the 'nonnull' attribute breaks code On Tue, 10 Jun 2008, pgut001 at cs dot auckland dot ac dot nz wrote: > fully correct code suddenly breaks. In fact gcc seems to be doing the opposite > of what ISO WG 14 is proposing for this attribute, which was to add extra > checking to make sure the attribute isn't NULL. gcc instead *removes* extra > checking to make sure the attribute isn't NULL. If WG14 is proposing incompatible attribute semantics, that's ignoring the C1X Charter (N1250) - we agreed at the London meeting that one of the main problems with C99 and reasons for it not being widely implemented, that should be avoided for C1x, was invention and being incompatible with existing practice. (Some subsequent minutes suggest that the idea has since been adopted that C++0x can invent something and then C1x can take C++0x as existing practice for C; not a good idea in my view.) 13. Unlike for C9X, the consensus at the London meeting was that there should be no invention, without exception. Only those features that have a history and are in common use by a commercial implementation should be considered. Also there must be care to standardize these features in a way that would make the Standard and the commercial implementation compatible. Do you have a reference to the incompatible proposal? I sent N1259 a while back warning about incompatibilities in syntax; perhaps a paper is needed on incompatible semantics as well.
>13. Unlike for C9X, the consensus at the London meeting was that there should >be no invention, without exception. Only those features that have a history >and are in common use by a commercial implementation should be considered. >Also there must be care to standardize these features in a way that would >make the Standard and the commercial implementation compatible. Well they've done exactly that, it's perfectly compatible with VC++ (via PREfast), which is a widely-used commercial implementation :-). >Do you have a reference to the incompatible proposal? I sent N1259 a while >back warning about incompatibilities in syntax; perhaps a paper is needed on >incompatible semantics as well. I'm using N1273, which seems to be the only published statement on the use of attributes. For stdc_nonnull it describes the proposed semantics as: To be discussed[[the implementation may/it is implementation defined add assertions to ensure the runtime values of the variables are not null pointers] OTOH the gcc interpretation of this seems to be: To be discussed[[the implementation may/it is implementation defined *remove* assertions that ensure the runtime values of the variables are not null pointers] If the current gcc behaviour is regarded as correct and appropriate then maybe the docs need to be updated (which is why I originally reported this as a doc. bug, I wasn't sure which of the two was definitive). Currently the docs say something to the effect of: The compiler WILL issue warnings and MAY change its code generation when it sees this attribute. when in fact what it really does is: The compiler WILL change its code generation and MAY issue warnings when it sees this attribute. So the proposed change would be to substitute, in place of the current ... 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. the new text: ... causes the compiler to make optimizations based on the knowledge that certain function arguments will not be null. The compiler may also choose 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. This more accurately reflects the actual behaviour of the compiler.