diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -319,6 +319,8 @@ `Issue 58302 `_ `Issue 58753 `_ `Issue 59100 `_ +- Fix issue using __attribute__((format)) on non-variadic functions that expect + more than one formatted argument. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3191,6 +3191,18 @@ fmt(fmt, "hello", 123); // warning: format string is not a string literal } +When using the format attribute on a variadic function, the first data parameter +_must_ be the index of the ellipsis in the parameter list. Clang will generate +a diagnostic otherwise, as it wouldn't be possible to forward that argument list +to `printf`-family functions. For instance, this is an error: + +.. code-block:: c + + __attribute__((__format__(__printf__, 1, 2))) + void fmt(const char *s, int b, ...); + // ^ error: format attribute parameter 3 is out of bounds + // (must be __printf__, 1, 3) + Using the ``format`` attribute on a non-variadic function emits a GCC compatibility diagnostic. }]; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3891,27 +3891,38 @@ if (!checkUInt32Argument(S, AL, FirstArgExpr, FirstArg, 3)) return; - // check if the function is variadic if the 3rd argument non-zero + // FirstArg == 0 is is always valid. if (FirstArg != 0) { - if (isFunctionOrMethodVariadic(D)) - ++NumArgs; // +1 for ... - else - S.Diag(D->getLocation(), diag::warn_gcc_requires_variadic_function) << AL; - } - - // strftime requires FirstArg to be 0 because it doesn't read from any - // variable the input is just the current time + the format string. - if (Kind == StrftimeFormat) { - if (FirstArg != 0) { + if (Kind == StrftimeFormat) { + // If the kind is strftime, FirstArg must be 0 because strftime does not + // use any variadic arguments. S.Diag(AL.getLoc(), diag::err_format_strftime_third_parameter) - << FirstArgExpr->getSourceRange(); - return; + << FirstArgExpr->getSourceRange() + << FixItHint::CreateReplacement(FirstArgExpr->getSourceRange(), "0"); + return; + } else if (isFunctionOrMethodVariadic(D)) { + // Else, if the function is variadic, then FirstArg must be 0 or the + // "position" of the ... parameter. It's unusual to use 0 with variadic + // functions, so the fixit proposes the latter. + if (FirstArg != NumArgs + 1) { + S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) + << AL << 3 << FirstArgExpr->getSourceRange() + << FixItHint::CreateReplacement(FirstArgExpr->getSourceRange(), + std::to_string(NumArgs + 1)); + return; + } + } else { + // Inescapable GCC compatibility diagnostic. + S.Diag(D->getLocation(), diag::warn_gcc_requires_variadic_function) << AL; + if (FirstArg <= Idx) { + // Else, the function is not variadic, and FirstArg must be 0 or any + // parameter after the format parameter. We don't offer a fixit because + // there are too many possible good values. + S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) + << AL << 3 << FirstArgExpr->getSourceRange(); + return; + } } - // if 0 it disables parameter checking (to use with e.g. va_list) - } else if (FirstArg != 0 && FirstArg != NumArgs) { - S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) - << AL << 3 << FirstArgExpr->getSourceRange(); - return; } FormatAttr *NewAttr = S.mergeFormatAttr(D, AL, II, Idx, FirstArg); diff --git a/clang/test/FixIt/attr-format.c b/clang/test/FixIt/attr-format.c new file mode 100644 --- /dev/null +++ b/clang/test/FixIt/attr-format.c @@ -0,0 +1,9 @@ +// RUN: not %clang_cc1 %s -fsyntax-only -fdiagnostics-parseable-fixits 2>&1 | FileCheck %s + +// CHECK: fix-it:"{{.*}}attr-format.c":{4:36-4:37}:"0" +__attribute__((format(strftime, 1, 1))) +void my_strftime(const char *fmt); + +// CHECK: fix-it:"{{.*}}attr-format.c":{8:34-8:36}:"2" +__attribute__((format(printf, 1, 10))) +void my_strftime(const char *fmt, ...); diff --git a/clang/test/Sema/attr-format.c b/clang/test/Sema/attr-format.c --- a/clang/test/Sema/attr-format.c +++ b/clang/test/Sema/attr-format.c @@ -7,10 +7,14 @@ void c(const char *a, ...) __attribute__((format(printf, 0, 2))); // expected-error {{'format' attribute parameter 2 is out of bounds}} void d(const char *a, int c) __attribute__((format(printf, 1, 2))); // expected-warning {{GCC requires a function with the 'format' attribute to be variadic}} void e(char *str, int c, ...) __attribute__((format(printf, 2, 3))); // expected-error {{format argument not a string type}} +void f(int a, const char *b, ...) __attribute__((format(printf, 2, 1))); // expected-error {{'format' attribute parameter 3 is out of bounds}} +void g(int a, const char *b, ...) __attribute__((format(printf, 2, 2))); // expected-error {{'format' attribute parameter 3 is out of bounds}} +void h(int a, const char *b, ...) __attribute__((format(printf, 2, 3))); // no-error +void i(const char *a, int b, ...) __attribute__((format(printf, 1, 2))); // expected-error {{'format' attribute parameter 3 is out of bounds}} typedef const char *xpto; -void f(xpto c, va_list list) __attribute__((format(printf, 1, 0))); // no-error -void g(xpto c) __attribute__((format(printf, 1, 0))); // no-error +void j(xpto c, va_list list) __attribute__((format(printf, 1, 0))); // no-error +void k(xpto c) __attribute__((format(printf, 1, 0))); // no-error void y(char *str) __attribute__((format(strftime, 1, 0))); // no-error void z(char *str, int c, ...) __attribute__((format(strftime, 1, 2))); // expected-error {{strftime format attribute requires 3rd parameter to be 0}} @@ -94,3 +98,9 @@ forward_fixed(fmt, i); a(fmt, i); } + +__attribute__((format(printf, 1, 2))) void forward_fixed_2(const char *fmt, int i, int j) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}} + forward_fixed_2(fmt, i, j); + a(fmt, i); +} +