fcloutier created this revision.
fcloutier added reviewers: ahatanak, dcoughlin, aaron.ballman.
fcloutier added a project: clang.
Herald added a project: All.
fcloutier requested review of this revision.
Herald added a subscriber: cfe-commits.
The initial implementation <https://reviews.llvm.org/D112579> of
`__attribute__((format))` on non-variadic functions accidentally only accepted
//one// data argument. This worked:
__attribute__((format(printf, 1, 2)))
void f(const char *, int);
but this didn't:
__attribute__((format(printf, 1, 2)))
void f(const char *, int, int);
This is due to an oversight in changing the way diagnostics are emitted for
attribute((format)), and to a coincidence in the handling of the variadic case.
Test cases only covered the case that worked by coincidence.
Before the previous change, using `__attribute__((format))` on a non-variadic
function at all was an error and clang bailed out. After that change, it only
generates a GCC compatibility warning. However, as execution falls through, it
hits a second diagnostic when the first data argument is neither 0 nor the last
parameter of the function.
This change updates that check to allow any parameter after the format string
to be the first data argument when the function is non-variadic. When the
function is variadic, it still needs to be the index of the `...` "parameter".
Attribute documentation is updated to reflect the change and new tests are
added to verify that it works with //two// data parameters.
rdar://102069446
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137603
Files:
clang/include/clang/Basic/AttrDocs.td
clang/lib/Sema/SemaDeclAttr.cpp
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/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3890,27 +3890,33 @@
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;
+ } else if (isFunctionOrMethodVariadic(D)) {
+ // Else, if the function is variadic, then FirstArg must be 0 or the "position"
+ // of the ... parameter.
+ if (FirstArg != NumArgs + 1) {
+ S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds)
+ << AL << 3 << FirstArgExpr->getSourceRange();
+ 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.
+ 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.
}];
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
