[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 6 12:05:07 PDT 2018


aaron.ballman added subscribers: hans, joerg.
aaron.ballman added a comment.

In https://reviews.llvm.org/D47290#1115352, @jfb wrote:

> Hopefully this makes sense? I'm not sure I summarize my context very well. I'm happy to talk about it in-person next week too.


I appreciate the in-person conversation, it helped me to understand your goals better.

That said, I still see this as changing the portability aspects of -Wformat (because we'd be changing the behavior based on the target, but this is perhaps different from the portability concerns @rjmccall raised) and I don't see where to get off that train. For instance, do we also silence -Wformat when targeting Windows with %ld and %d because long and int have the same size and alignment requirements for x86 and x86-64 (at least, I don't know about other Windows targets) and that is effectively baked into the platform requirements? If we don't, how is that situation different than the one motivating your changes?

As a user, I very much appreciate that -Wformat gives me the same diagnostics no matter what platform I target, so these changes make me a bit uncomfortable -- those warnings are not useless to me. You (rightly) asked for a concrete explanation of why. My reason is: the user's code here is UB and there is a long history demonstrating that "it's UB, but it works and does what I want!" suddenly changing into "it's not what I want" silently, perhaps many years down the line, because the optimizer suddenly got smarter and it results in exploitable security vulnerabilities. However, I'm no longer as scared that the optimizer is going to start making type-based decisions based on this format specifier string. I can imagine the optimizer making value-based decisions based on a format specifier string (like seeing a value matches a %s specifier and assuming the value cannot be null), but that's not this. However, I have had users for which UB is verboten. Period. Full stop. I doubt these users care at all about Apple's platform, so *this* change isn't likely to impact them, but the *precedent* from this change certainly could. However, perhaps making this pedantic as in this patch will be reasonable (I believe these folks already pass -pedantic when they realize that's a thing). I have to do my homework on that (and can't do that homework this week).


Repository:
  rC Clang

https://reviews.llvm.org/D47290





More information about the cfe-commits mailing list