Hello Jonny,
On 7/7/21 1:36 PM, Jonny Grant wrote:
On 06/07/2021 23:11, Florian Weimer wrote:
* Jonny Grant:
The reason it does not crash appears to be because of this warning
which removes the call to strlen due to the return not being
checked?
GCC uses the information that the argument to strlen cannot be null on
that particular path.
It's a shame GCC doesn't give a warning
It may be GCC is using '__builtin_strlen'
<string.h> marks the param as nonnull. However, I am surprised this does not trigger the GCC warning -Werror=nonnull
[[gnu::nunnull]], aka __attribute__((nonnull)), is only triggered by
compile time NULL. That is: passing NULL (either hardcoded or through a
macro)
If after optimizations the compiler realizes that an argument is NULL at
compile time, it may (but also may not) trigger the warning. However,
if you optimize too much, the expression may vanish, and the warning be
silenced again. The -fanalyzer option of GCC may (but also may not)
realize of more NULL cases.
I managed to trigger the following, but only with '-O0 -fanalyzer':
[[
<source>:17:20: error: use of NULL 's' where non-null expected [CWE-476]
[-Werror=analyzer-null-argument]
17 | int x = strlen (s);
| ~~~~~~~^~~
'void f(const char*)': events 1-2
|
| 16 | const char *s = NULL;
| | ^
| | |
| | (1) 's' is NULL
| 17 | int x = strlen(s);
| | ~~~~~~~~~~
| | |
| | (2) argument 1 ('s') NULL where non-null
expected
|
In file included from <source>:4:
/usr/include/string.h:385:15: note: argument 1 of 'size_t strlen(const
char*)' must be non-null
385 | extern size_t strlen (const char *__s)
| ^~~~~~
]]
It is undefined behavior to pass NULL to a nonnull parameter, and it is
a problem that the caller should check before the call. The compiler
may warn you of the most obvious ones, but in the end, the
responsibility is on the programmer.
See
<https://stackoverflow.com/questions/42035769/how-to-generate-warning-of-non-null-argument-for-my-custom-function>.
/* Return the length of S. */
extern size_t strlen (const char *__s)
__THROW __attribute_pure__ __nonnull ((1));
Perhaps that is just a macro that is not actually used......
It _is_ being used:
/usr/include$ grep -rn 'define __nonnull' -B3 -A1
x86_64-linux-gnu/sys/cdefs.h-290-/* The nonull function attribute allows
to mark pointer parameters which
x86_64-linux-gnu/sys/cdefs.h-291- must not be NULL. */
x86_64-linux-gnu/sys/cdefs.h-292-#if __GNUC_PREREQ (3,3)
x86_64-linux-gnu/sys/cdefs.h:293:# define __nonnull(params)
__attribute__ ((__nonnull__ params))
x86_64-linux-gnu/sys/cdefs.h-294-#else
x86_64-linux-gnu/sys/cdefs.h:295:# define __nonnull(params)
x86_64-linux-gnu/sys/cdefs.h-296-#endif
If I add another function -Werror=nonnull does give a warning
void test(const char * const p) __attribute__((nonnull));
https://godbolt.org/z/x37sbfWaG
<source>:15:9: error: argument 1 null where non-null expected [-Werror=nonnull]
15 | test(NULL);
strlen.c:11:3: warning: statement with no effect [-Wunused-value]
11 | strlen (str);
| ^~~~~~~~~~~~
https://godbolt.org/z/caoes5nGa
That site probably uses different library headers.
As posted, with Debian's GCC 8.3, I get this for the main function:
main:
xorl %eax, %eax
ret
In that case maybe https://man7.org/linux/man-pages/man3/strlen.3.html should have a "NOTES" section stating something similar to your wording ?
NOTES
The behavior of strlen(NULL) is depends on different things. It may cause a SEGV exception, or the compiler may optimize and remove the code path handling NULL.
I disagree with this. It is likely that the behavior is that, given the
current implementation of Linux/GCC/glibc. But it is undefined
behavior, and anything can happen. You should just try harder to avoid
it, and not rely on any possible outcome of it. GCC people may decide
tomorrow to change the behavior to do some more agresive optimizations,
and the documentation shouldn't preclude such a thing, as long as it's
legal according to the relevant standards, and sane.
Cheers,
Alex
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
