[cfe-dev] Relaxing format specifier checks

[Hans Wennborg via cfe-dev]

Sorry for the late reply. I didn't see the thread until now.

On Sat, May 12, 2018 at 1:26 AM, Shoaib Meenai via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
> Hi all,
>
>
>
> Currently, clang's format specifier check warnings (-Wformat) check for
> exact type matches between the specifier and the corresponding argument. I'm
> proposing adding an option to relax these warnings such that it doesn't warn
> when there's a type mismatch but the types of the specifier and the argument
> have the same size and alignment (and are both integral). For an example, on
> an LLP64 system, attempting to print a long with "%d" would not warn, but
> attempting to print a long long with "%d" would still warn. We could either
> do this relaxation for -Wformat and add a new option for the stricter
> warnings, or keep -Wformat as is and add a new option for the relaxed
> warnings.
>
>
>
> The motivation is that during the 6.0 timeframe, clang gained format
> specifier checking for %zd (r308067) and %tu (r314470), which made -Wformat
> fire in a lot of additional cases. A particularly common case is programs
> using %zd to print NSInteger. This will be warning-free on 64-bit targets,
> where both ssize_t and NSInteger are long, but warn on 32-bit targets, where
> ssize_t is still long but NSInteger is int. This additional warning
> motivated https://reviews.llvm.org/D42933, which attempted to special case
> %z and NSInteger; the diff's comments have a bunch of additional discussion
> about this issue. The conclusion there was that, although having mismatched
> specifiers of this nature is definitely technically incorrect, there
> shouldn't actually be any issues in practice when the size and alignment of
> the specifier and the argument match. Ideally people would update their
> codebases to use proper specifiers, but that can be impractical for large
> codebases, and it would be really nice to have a way to restrict the warning
> to the cases which will actually cause problems in practice.

It sounds like the problem is really that NSInteger and size_t are
typedeffed to different types on 32-bit. Would it be possible to get
that fixed instead?

> Note that -Wformat isn't meant to be a general portability checker, since it
> only ever raises warnings when there's a type mismatch *on the current
> target*, not if there's any possibility of a mismatch.

It's (mostly) not trying to be clever; it's just checking what the
standard requires. Printing an int with %ld is wrong according to the
standard even if int and long are the same size on the target.

> For example, you
> could be printing out a size_t using %lu, and you wouldn't get warnings if
> your compilation target used unsigned long as the underlying type for
> size_t, even though that underlying type could differ across targets.

Right, because doing this kind of portability checks would be hard:
the compiler can't know if the source used unsigned long and was lucky
that it matched size_t, or if unsigned long had been carefully
selected to match.

> This
> proposal does mean we would lose out on some additional portability
> warnings; continuing the previous example, we would now not get warnings for
> printing out a size_t using %lu even for a compilation target where size_t
> was actually an unsigned int, assuming that the target had the same size and
> alignment for int and long (as most 32-bit platforms do). However, given
> that -Wformat was never meant to be a general portability checker, and that
> there would still be an option to get the fully strict warnings, I don't
> think this is too much of an issue in practice.

I prefer to think of them as correctness warnings rather than
portability warnings. I'm not buying the argument that because it
doesn't catch all portability issues, it shouldn't warn about
incorrect code.

> What are people's thoughts here? Are there preferences for relaxing -Wformat
> and adding a new option for strict format warnings vs. keeping -Wformat
> as-is and adding a new option for relaxed format warnings? Of course, we
> should also ensure that the optimizer doesn't do anything surprising when
> there's a type mismatch between the specifier and the argument but both
> types have the same size and alignment (i.e., the case where the relaxed
> format warning won't fire), both now and in the future.

I would prefer not to relax -Wformat by default. I think what it's
doing is simple and correct, and a large amount of code compiles with
it.

I'm not opposed to adding a -Wformat-relaxed option if there's a lot
of demand for it.

TL;DR: What Joerg said.

Thanks,
Hans