Re: [PATCH] stdlib: avoid canonicalize_file_name contradiction

[Bruno Haible]

Hi Jim,

> stdlib: avoid canonicalize_file_name contradiction
> * lib/stdlib.in.h (canonicalize_file_name): Remove the nonnull
> attribute from its declaration.

I'm not sure this is right. The patch removes a diagnostic (from GCC and
possibly other static analyzers) when some code is, by mistake, passing
NULL to canonicalize_file_name.

The question is: Is passing NULL to canonicalize_file_name valid? If not,
then the nonnull attribute should stay.

On one hand, in glibc's stdlib.h we have:

extern char *canonicalize_file_name (const char *__name)
     __THROW __nonnull ((1)) __wur;

On the other hand, in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93156#c3
you say "Note that at least some versions of canonicalize_file_name *can*
take a NULL argument". What are there versions? Even if Cygwin and/or
Solaris 11 have a function of the same name which allows passing NULL,
portable code should not pass NULL since that would not work on glibc
systems. Therefore the diagnostic is useful.

> tests/test-canonicalize-lgpl.c
> passes null_ptr () to it, which (via this contradiction) would
> provoke a segfault from GCC 10. See a small reproducer and
> discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93156

I would prefer to modify the tests, to avoid GCC's over-optimization.
There are two ways to do that:

  * Use 'volatile', as shown by Andrew Pinski in
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93156#c2
    I am not in favour of this, because the semantics of 'volatile'
    changes over time. Currently there is a proposed change in
    http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2244.htm#dr_476 .

  * The usual technique for this, that we already use in
      lib/canonicalize-lgpl.c
      lib/getaddrinfo.c
      lib/getdelim.c
      lib/glob.c
      lib/random_r.c
      lib/setenv.c
      lib/tsearch.c
      lib/unsetenv.c
    , is to add a line
      #define _GL_ARG_NONNULL(params)
    before '#include <config.h>'.

Does the attached patch fix the problem for you?

Bruno

0001-tests-Avoid-GCC-over-optimization-caused-by-_GL_ARG_.patch
Description: Text Data