[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

Arthur O'Dwyer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 29 15:52:15 PDT 2021


Quuxplusone added a comment.

In D112579#3097360 <https://reviews.llvm.org/D112579#3097360>, @aaron.ballman wrote:

> [...] Having the types in the signatures changes something I think might be pretty fundamental to the way the format string checker works -- it tries to figure out types *after default argument promotions* because those promotions are a lossy operation. However, when the types are specified in the signature, those default argument promotions no longer happen. The type passed for a `%f` may actually be a `float` rather than promoted to a `double`, the type for a `char` may actually be `char` rather than `int`, etc.

True, and I think you're right that this change needs some really exhaustive tests. First, notice that the point of `format(printf)` is still //ultimately// to pass the arguments to some `printf`-like varargs function that //will// do the default argument promotions. So if our argument type is `float`, well, in the non-pathological cases, //eventually// it //should// become a `double` (when it is finally passed to `printf`, possibly several levels deeper in the function call stack). But second, consider situations like

  void myprintf(const char *fmt, int n) __attribute__((format(printf, 1, 2)));  // N.B.: int, not unsigned long
  int main() {
      myprintf("%lu", 3uL);  // this should error
      myprintf("%d", 3uL);  // this should not error
  }



> In terms of the specific cases allowed, I think I am happier about variadic templates than I am about fixed-signature functions. [...] For a fixed-signature function, I'm not certain I see what the value add is -- the only valid format strings such a function could accept would have to be fixed to the function signature anyway.

Or, in some situations (like logging), the format string could be some weird amalgam of the signature and something else, right?

  void mydebug(const char *fmt, int line) { printf(fmt, "test.cpp", line); }
  int main() {
      mydebug("%s:%d", 42);  // printf("%s:%d", 42) would not be correct; but the way mydebug ultimately uses printf, this is actually safe and intentional
  }

When we receive a `va_list` or a varargs `...`, this scenario doesn't arise (AFAIK), because there is no way to manipulate or insert-more-arguments-into a `va_list` after the fact.

Initially I thought this PR was a slam-dunk, but thinking about all the pathological-//but-possible// corner cases here does make me more skeptical now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112579/new/

https://reviews.llvm.org/D112579



More information about the cfe-commits mailing list