On 7/30/21 9:25 AM, Richard Biener wrote:
> On Wed, 28 Jul 2021, Tom de Vries wrote:
>
>> Hi,
>>
>> Improve nonnull attribute documentation in a number of ways:
>>
>> Reorganize discussion of effects into:
>> - effects for calls to functions with nonnull-marked parameters, and
>> - effects for function definitions with nonnull-marked parameters.
>> This makes it clear that -fno-delete-null-pointer-checks has no effect for
>> optimizations based on nonnull-marked parameters in function definitions
>> (see PR100404).
>>
>> Mention -Wnonnull-compare.
>>
>> Mention workaround from PR100404 comment 7.
>>
>> The workaround can be used for this scenario. Say we have a test.c:
>> ...
>> #include <stdlib.h>
>>
>> extern int isnull (char *ptr) __attribute__ ((nonnull));
>> int isnull (char *ptr)
>> {
>> if (ptr == 0)
>> return 1;
>> return 0;
>> }
>>
>> int
>> main (void)
>> {
>> char *ptr = NULL;
>> if (isnull (ptr)) __builtin_abort ();
>> return 0;
>> }
>> ...
>>
>> The test-case contains a mistake: ptr == NULL, and we want to detect the
>> mistake using an abort:
>> ...
>> $ gcc test.c
>> $ ./a.out
>> Aborted (core dumped)
>> ...
>>
>> At -O2 however, the mistake is not detected:
>> ...
>> $ gcc test.c -O2
>> $ ./a.out
>> ...
>> which is what -Wnonnull-compare (not show here) warns about.
>>
>> The easiest way to fix this is by dropping the nonnull attribute. But that
>> also disables -Wnonnull, which would detect something like:
>> ...
>> if (isnull (NULL)) __builtin_abort ();
>> ...
>> at compile time.
>>
>> Using this workaround:
>> ...
>> int isnull (char *ptr)
>> {
>> + asm ("" : "+r"(ptr));
>> if (ptr == 0)
>> return 1;
>> return 0;
>> }
>> ...
>> we still manage to detect the problem at runtime with -O2:
>> ...
>> $ ~/gcc_versions/devel/install/bin/gcc test.c -O2
>> $ ./a.out
>> Aborted (core dumped)
>> ...
>> while keeping the possibility to detect "isnull (NULL)" at compile time.
>>
>> OK for trunk?
>
> I think it's an improvement over the current situation but the
> inline-assembler suggestion to "fix" definition side optimizations
> are IMHO a hint at that we need a better solution here.
Agreed.
[ If you want I can resubmit without that bit, if it's not acceptable. I
merely included it because it's the only solution I found advertised (in
bugzilla). ]
> Splitting
> the attribute into a caller and a calle side one for example,
> or making -fno-delete-null-pointer-checks do what it suggests.
>
I think the problem is that in fact there are two attributes:
- assume_nonnull
- verify_nonnull (or, want_nonnull or some such)
which got conflated in the nonnull attribute. [ Which still could be
fine if you got an -fnonnull=assume/verify to switch between the two
interpretations. ]
Anyway, so more concretely my idea is that this should generate a -Wnonnull:
...
extern void foo (void *ptr) __attribute__((verify_nonnull (1)));
void bar (void) {
foo (nullptr);
}
...
and the same with assume_nonnull (after all, the assumption is violated).
And this assert could be optimized away (with -Wnonnull-compare):
...
extern void foo (void *ptr) __attribute__((assume_nonnull (1)));
void foo (void *ptr) {
assert (ptr != nullptr);
}
...
while the assert shouldn't be optimized away with verify_nonnull.
And likewise, this if could be optimized away by
fdelete-null-pointer-checks:
...
extern void foo (void *ptr) __attribute__((assume_nonnull (1)));
void bar (void *ptr) {
foo (ptr);
if (ptr != nullptr)
{ ... }
}
...
while the if shouldn't be optimized away with verify_nonnull.
I think this way of splitting up the functionality conforms to the
principle of least surprise and does not require thinking about caller /
callee distinction, nor does it require extension of
-fno-delete-null-pointer-checks.
Thanks,
- Tom
> And as suggested elsewhere the effect of -fno-delete-null-pointer-checks
> making objects at NULL address valid should be a target hook based
> on the address-space with the default implementation considering
> only the default address-space having no objects at NULL.
>
