[PATCH] D48734: [Sema] Consider all format_arg attributes.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC336239: [Sema] Consider all format_arg attributes. (authored
by Meinersbur, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48734?vs=153643=154038#toc
Repository:
rC Clang
https://reviews.llvm.org/D48734
Files:
lib/Sema/SemaChecking.cpp
test/Sema/attr-format_arg.c
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,27 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data
arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data
arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND =
dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const auto *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,27 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+
[PATCH] D48734: [Sema] Consider all format_arg attributes.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! Repository: rC Clang https://reviews.llvm.org/D48734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48734: [Sema] Consider all format_arg attributes.
Meinersbur updated this revision to Diff 153643.
Meinersbur added a comment.
- Reinsert test for the change in this patch
Repository:
rC Clang
https://reviews.llvm.org/D48734
Files:
lib/Sema/SemaChecking.cpp
test/Sema/attr-format_arg.c
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,27 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data
arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data
arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND =
dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const auto *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,27 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}}
+
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst =
[PATCH] D48734: [Sema] Consider all format_arg attributes.
Meinersbur updated this revision to Diff 153642.
Meinersbur added a comment.
- Address Aaron's remarks
Repository:
rC Clang
https://reviews.llvm.org/D48734
Files:
lib/Sema/SemaChecking.cpp
test/Sema/attr-format_arg.c
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,23 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data
arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data
arguments}}
+
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND =
dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const auto *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,23 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}}
+
+ printf(h(
+"%d",
+"" // expected-warning {{format string is empty}}
+ ), 123);
+ printf(h(
+"", // expected-warning {{format string is empty}}
+"%d"
+ ), 123);
+ printf(h("%d", "%d"), 123);
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const auto *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const auto *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID ==
[PATCH] D48734: [Sema] Consider all format_arg attributes.
aaron.ballman added inline comments.
Comment at: lib/Sema/SemaChecking.cpp:5523
+ StringLiteralCheckType CommonResult;
+ for (const FormatArgAttr *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
You can use `const auto *FA` here instead; the type is spelled out in the
initializer.
Comment at: lib/Sema/SemaChecking.cpp:5536
+
+ if (const FunctionDecl *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
`const auto *`
Comment at: test/Sema/attr-format_arg.c:17
+
+ printf(h("", ""), 123); // expected-warning 2{{format string is empty}}
}
Can you add tests for the other permutations as well? It would also be useful
to put the string literals on their own line so that the expected-warning
designation matches the specific argument that's being diagnosed. e.g.,
```
h(
"%d",
""
)
h(
"",
"%d"
)
h(
"%d",
"%d"
)
```
Repository:
rC Clang
https://reviews.llvm.org/D48734
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48734: [Sema] Consider all format_arg attributes.
Meinersbur added inline comments. Comment at: lib/Sema/SemaChecking.cpp:5534 + if (!IsFirst) +return CommonResult; + Not sure what should be returned here; To minimize surprises, this returns what the current version would have returned. Repository: rC Clang https://reviews.llvm.org/D48734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48734: [Sema] Consider all format_arg attributes.
Meinersbur created this revision.
Meinersbur added reviewers: dlj, rsmith, Jean-Daniel.
If a function has multiple format_arg attributes, clang only considers the
first it finds (because AttributeLists are in reverse order, it is textual
last) and ignores all others.
Loop over all FormatArgAttr to print warnings for all declared format_arg
attributes.
For instance, GNU gettext's ngettext (select plural or singular version of
format string) has two __format_arg__ attributes.
Repository:
rC Clang
https://reviews.llvm.org/D48734
Files:
lib/Sema/SemaChecking.cpp
test/Sema/attr-format_arg.c
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,15 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data
arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data
arguments}}
+
+ printf(h("", ""), 123); // expected-warning 2{{format string is empty}}
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND =
dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const FormatArgAttr *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const FunctionDecl *FD = dyn_cast(ND)) {
unsigned BuiltinID = FD->getBuiltinID();
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
Index: test/Sema/attr-format_arg.c
===
--- test/Sema/attr-format_arg.c
+++ test/Sema/attr-format_arg.c
@@ -4,10 +4,15 @@
const char* f(const char *s) __attribute__((format_arg(1)));
+const char *h(const char *msg1, const char *msg2)
+__attribute__((__format_arg__(1))) __attribute__((__format_arg__(2)));
+
void g(const char *s) {
printf("%d", 123);
printf("%d %d", 123); // expected-warning{{more '%' conversions than data arguments}}
printf(f("%d"), 123);
printf(f("%d %d"), 123); // expected-warning{{more '%' conversions than data arguments}}
+
+ printf(h("", ""), 123); // expected-warning 2{{format string is empty}}
}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -5518,13 +5518,22 @@
case Stmt::CXXMemberCallExprClass: {
const CallExpr *CE = cast(E);
if (const NamedDecl *ND = dyn_cast_or_null(CE->getCalleeDecl())) {
- if (const FormatArgAttr *FA = ND->getAttr()) {
+ bool IsFirst = true;
+ StringLiteralCheckType CommonResult;
+ for (const FormatArgAttr *FA : ND->specific_attrs()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
-return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
- CheckedVarArgs, UncoveredArg, Offset);
- } else if (const FunctionDecl *FD = dyn_cast(ND)) {
+StringLiteralCheckType Result = checkFormatStringExpr(
+S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
+CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
+if (IsFirst) {
+ CommonResult = Result;
+ IsFirst = false;
+}
+ }
+ if (!IsFirst)
+return CommonResult;
+
+ if (const FunctionDecl *FD = dyn_cast(ND)) {
unsigned BuiltinID =
