[cfe-dev] Relaxing format specifier checks

[JF Bastien via cfe-dev]

Thanks for writing this up!

> On May 11, 2018, at 4:26 PM, 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 <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.
>  
> 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. 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. 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.
>  
> What are people's thoughts here? Are there preferences for relaxing -Wformat and adding a new option for strict format warnings

I much prefer this approach. We know that our portability warnings are a best-effort thing, and it would be best for developers to opt-in to them. I don’t thing -Wformat-portability should be on by default unless we also do things as suggested in D42933 (where we know that size and alignment of NSInteger / NSUInteger always match that of size_t / ssize_t, so there’s no actual portability issue).


> vs. keeping -Wformat as-is and adding a new option for relaxed format warnings?

I don’t like this approach. I think having -Wformat-relaxed / -Wformat / -Wformat-pedantic is confusing.


> 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.

+1


> Thanks,
> Shoaib
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180511/3fbb22b3/attachment.html>