Format warnings ignored by -Wformat

[Jon Flatley]

Hello,

Thank you for both of your detailed responses!

On Tue, Feb 26, 2019 at 10:46 AM Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>
> On Tue, Feb 26, 2019 at 02:26:12PM +0000, Jonathan Wakely wrote:
> > On Mon, 25 Feb 2019 at 23:32, Segher Boessenkool
> > <segher@kernel.crashing.org> wrote:
> > > On Mon, Feb 25, 2019 at 11:18:31AM -0800, Jon Flatley wrote:
> > > > I was hoping I could reach out and gain some insight as to why GCC
> > > > ignores these scenarios where Clang produces warnings. This would be
> > > > very helpful as we move to enable -Wformat for both GCC and Clang in
> > > > the upstream kernel, which is an issue we're tracking here:
> > > > https://github.com/ClangBuiltLinux/linux/issues/378
> > >
> > > You cannot pass a short int to a varargs function like printf, it is
> > > always promoted to int.  As the C standard says for the h flag character:
> > >
> > > h
> > >     Specifies that a following d, i, o, u, x, or X conversion specifier
> > >     applies to a short int or unsigned short int argument (the argument
> > >     will have been promoted according to the integer promotions, but
> > >     its value shall be converted to short int or unsigned short int
> > >     before printing); or that a following n conversion specifier applies
> > >     to a pointer to a short int argument.

The way I interpret this is that the %hd specifier applies to either a
short int or unsigned short int as an argument regardless of how the
value is promoted. The statement about the integer promotions doesn't
sound like this is saying %hd also applies to just "int". Expecting an
"int" value for a %hd specifier doesn't feel right to me.

> >
> > A bit later it says:
> >
> > If any argument is
> > not the correct type for the corresponding conversion specification,
> > the behavior is
> > undefined.
> >
> > The "conversion specification" is %hd so includes both the h length
> > modifier and the d conversion specifier, and it's not clear to me
> > whether "the correct type" means the type as modified by the length
> > modifier or not, i.e. whether %hd
>
> It means "int" here, afaics.  This is about whether the same types args
> are passed to the printf function as it is trying to read.

IIUC, you're saying the type should be "int" from the perspective of
what printf receives. Given the language of the "h" specifier it
sounds to me like it makes the "correct type" a short int, not an int.
Otherwise the meaning of %hd is "anything that promotes to an int is
fine, but it will be converted to a short" when it sounds like it
should be "a short is expected here despite integer promotions".

What's more is compiling
    printf("%hd");
produces the warning
    warning: format ‘%hd’ expects a matching ‘int’ argument
but I would expect this to read
    warning: format ‘%hd’ expects a matching ‘short int’ argument
given the language for the "h" specifier reads
    applies to a short int or unsigned short int argument
much like how the language for the "l" specifier reads
    applies to a long int or unsigned long int argument
and compiling
    printf("%ld");
produces the warning
    warning: format ‘%ld’ expects a matching ‘long int’ argument

>
> > > Assuming the integer started out as short int before passing it to the
> > > printf function is something GCC does not do;
> >
> > But GCC doesn't need to assume anything when it can see exactly what's
> > being passed to printf. It can see that an int really is used there,
> > not just something promoted to int.
>
> Maybe I didn't say it clearly, sorry.  If GCC wants to warn about passing
> a plain int to %hd it needs to assume this is "bad" somehow (even although
> the C standard says exactly how this should behave).

It's very likely there's something I'm missing in the C standard and
this behavior is appropriate. It just seems to go against intuition.

If this is the correct behavior, does that mean the Clang warnings in
this situation are inappropriate and I should be discussing or
potentially filing a bug with the Clang community?

Thanks again for your help,
Jon

>
> > > it will warn on a lot of
> > > perfectly valid code.
> >
> > But also misses warnings for code with unintended (maybe even
> > undefined) behaviour:
> >
> > #include <stdio.h>
> > #include <limits.h>
> >
> > int main()
> > {
> >   int i = SHRT_MAX + 1;
> >   printf("%hd %hd\n", i, SHRT_MAX + 1);
> > }
>
> Conversion to a shorter integer type can be implementation-defined, not
> undefined.  GCC has
>
>      For conversion to a type of width N, the value is reduced modulo
>      2^N to be within range of the type; no signal is raised.
>
> (but it is a bit shady what printf does exactly, it's not part of GCC;
> but we can probably assume it is compiled by GCC itself).
>
> > Inside printf the int values are converted to short, which is either
> > an overflow (so undefined) or, if the conversion is done via unsigned
> > types, at least a change in value that is probably unwanted.
>
> It's implementation-defined, see 6.3.1.3/3.
>
> > The compiler should be able to warn that SHRT_MAX+1 will not be
> > printed correctly.
>
> That depends what you call "correct".  As I said, there will be lots of
> false positives with such a warning, as OP already showed.
>
>
> Segher