[cfe-dev] Relaxing format specifier checks

Wed May 16 05:33:07 PDT 2018

Another +1 to that.

For NSInteger, one thing that projects can do until the SDK either makes
NSInteger and ssize_t consistent or offers format strings for NSInteger
instead of recommending casting is to add something like this and use it:

// The size of NSInteger and NSUInteger varies between 32-bit and 64-bit
// architectures and Apple does not provides standard format macros and
// recommends casting. This has many drawbacks, so instead define macros
// for formatting those types.
#if defined(OS_MACOSX)
#if defined(ARCH_CPU_64_BITS)
#if !defined(PRIdNS)
#define PRIdNS "ld"
#endif
#if !defined(PRIuNS)
#define PRIuNS "lu"
#endif
#if !defined(PRIxNS)
#define PRIxNS "lx"
#endif
#else  // defined(ARCH_CPU_64_BITS)
#if !defined(PRIdNS)
#define PRIdNS "d"
#endif
#if !defined(PRIuNS)
#define PRIuNS "u"
#endif
#if !defined(PRIxNS)
#define PRIxNS "x"
#endif
#endif
#endif  // defined(OS_MACOSX)

On Wed, May 16, 2018 at 7:49 AM, Aaron Ballman via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> I'm in the same camp as Hans and Joerg -- I would prefer to keep the
> existing check functionality and provide a more relaxed mode as an
> alternative if one is necessary (for the same reasons they bring up).
>
> ~Aaron
>
> On Wed, May 16, 2018 at 7:28 AM, Hans Wennborg via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> > 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
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> 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/20180516/77ee1683/attachment.html>