Re: [PATCH] stdlib: avoid canonicalize_file_name contradiction

[Jim Meyering <jim@meyering.net>]

On Sun, Jan 5, 2020 at 12:47 PM Bruno Haible <bruno@clisp.org> wrote:
>
> Jim Meyering wrote:
> > > 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.
> >
> > It is useful indeed.
>
> OK, then let's preserve it.
>
> > Thanks for working on that. However, it did not help, because at least
> > on Fedora 30, we're using the system declaration
>
> In this case, since the glibc headers (in particular <sys/cdefs.h>) do not
> give us the means to influence what __nonnull expands to, we have no choice
> than to disable the test when the function comes from the system.
>
> We do try to check the system functions just like we check the gnulib functions;
> this strategy has led us to find numerous libc bugs on various systems. But
> here, there's no way. Since canonicalize_file_name and ptsname_r are glibc
> inventions, not POSIX functions, there is no specification that tells what
> should happen if someone passes a NULL pointer to them; therefore the
> __nonnull declaration and the effect that it has (from GCC), namely execute
> arbitrary code, cannot be formally disputed. They can be disputed from the
> point of view of practical usefulness, though.
>
> I've pushed this. I hope it fixes the problem.
>
>
> 2020-01-05  Bruno Haible  <bruno@clisp.org>
>
>         tests: Avoid GCC over-optimization caused by _GL_ARG_NONNULL attributes.
>         Reported by Jim Meyering in
>         <https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00040.html>.
>         * lib/stdlib.in.h (GNULIB_defined_canonicalize_file_name): New macro.
>         (GNULIB_defined_ptsname_r): New macro.
>         * tests/test-canonicalize.c (_GL_ARG_NONNULL): Define to empty.
>         (main): Disable the NULL argument test if canonicalize_file_name does
>         not come from gnulib.
>         * tests/test-canonicalize-lgpl.c (_GL_ARG_NONNULL): Define to empty.
>         (main): Disable the NULL argument test if canonicalize_file_name does
>         not come from gnulib.
>         * tests/test-ptsname_r.c (_GL_ARG_NONNULL): Define to empty.
>         (test_errors): Disable the NULL argument test if ptsname_r does not come
>         from gnulib.

Thanks for those fixes. With those and the three
multithread-test-disabling changes, I confirm that all of GNU sed's
tests pass when built using the latest from gcc master (GCC10-pre).