[master] b0af060fb simple fix for fedora/gcc-10.0.1: -Werror=format-overflow, by some reason hit on s390x

[Dridi Boukelmoune]

On Wed, Feb 12, 2020 at 2:47 PM Nils Goroll <nils.goroll at uplex.de> wrote:
>
> >> -                   cmd, i, retval, r);
> >> +                   cmd != NULL ? cmd : "NULL", i, retval, r);
> >
> > We shouldn't blindly take this change, we should instead ask the GCC
> > or Fedora folks (or their intersection) why this is happening. We are
> > hiding a potential bug under the rug here.
>
> The gcc error from https://github.com/varnishcache/varnish-cache/issues/3216 is:
>
> inlined from 'varnish_launch' at vtc_varnish.c:500:6:
> vtc_varnish.c:121:3: error: '%s' directive argument is null
> [-Werror=format-overflow=]
>
> So in my mind the warning is legit, but can only happen with inling.
>
> Can you explain which potential bug you are suspecting?

This looks like GCC tripping on its own feet: it wants to inline a
variant of this function where cmd is known to be null because (I
suspect) a large-enough portion of the function can be eliminated,
enough to shrink below the inlining threshold on s390x. So far so
good, then it encounters a function with a format __attribute__, but
it can't tell whether it might overflow its destination (the answer is
no, it can't) and yet it insists on complaining about a potential
destination overflow because the argument to a %s format is known
to be null.

As long as the format __attribute__ can't tell the difference between
a printf-like or an sn?printf-like function it shouldn't trigger spurious
overflow checks.

The printf documentation says that the argument to a %s format is a
pointer to an array of char, but doesn't say explicitly that this
pointer must not be null.

This is at best off topic and otherwise irrelevant since on the target
system glibc will print "(null)" for that argument.

I can accept an undefined behavior claim better than Wformat-overflow
but if we acknowledge that then we need to fix a lot of format arguments
to ensure non-null %s directives. To me this should have been escalated
to GCC or Fedora instead, that's Ingvar's job as a package maintainer
for Fedora: something I would have pointed out if it hadn't been merged
so fast.

Dridi