fcloutier updated this revision to Diff 479658.
fcloutier added a comment.
Fix new fixit test for Windows, where directory separators are backslashes
instead of forward slashes.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137603/new/
https://reviews.llvm.org/D137603
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/AttrDocs.td
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/FixIt/attr-format.c
clang/test/Sema/attr-format.c
Index: clang/test/Sema/attr-format.c
===================================================================
--- clang/test/Sema/attr-format.c
+++ 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);
+}
+
Index: clang/test/FixIt/attr-format.c
===================================================================
--- /dev/null
+++ 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, ...);
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3890,27 +3890,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);
Index: clang/include/clang/Basic/AttrDocs.td
===================================================================
--- clang/include/clang/Basic/AttrDocs.td
+++ 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.
}];
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -316,6 +316,8 @@
`Issue 58302 <https://github.com/llvm/llvm-project/issues/58302>`_
`Issue 58753 <https://github.com/llvm/llvm-project/issues/58753>`_
`Issue 59100 <https://github.com/llvm/llvm-project/issues/59100>`_
+- Fix issue using __attribute__((format)) on non-variadic functions that expect
+ more than one formatted argument.
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
