[PATCH] D30157: [analyzer] Improve valist check
This revision was automatically updated to reflect the committed changes.
Closed by commit rL297153: [analyzer] Improve valist checks and move it out
from alpha state. (authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D30157?vs=90676&id=90855#toc
Repository:
rL LLVM
https://reviews.llvm.org/D30157
Files:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
cfe/trunk/test/Analysis/valist-uninitialized-no-undef.c
cfe/trunk/test/Analysis/valist-uninitialized.c
cfe/trunk/test/Analysis/valist-unterminated.c
Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
===
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -45,7 +45,6 @@
def CplusplusOptIn : Package<"cplusplus">, InPackage;
def Valist : Package<"valist">;
-def ValistAlpha : Package<"valist">, InPackage, Hidden;
def DeadCode : Package<"deadcode">;
def DeadCodeAlpha : Package<"deadcode">, InPackage, Hidden;
@@ -291,7 +290,7 @@
// Valist checkers.
//===--===//
-let ParentPackage = ValistAlpha in {
+let ParentPackage = Valist in {
def UninitializedChecker : Checker<"Uninitialized">,
HelpText<"Check for usages of uninitialized (or already released) va_lists.">,
@@ -305,7 +304,7 @@
HelpText<"Check for va_lists which are copied onto itself.">,
DescFile<"ValistChecker.cpp">;
-} // end : "alpha.valist"
+} // end : "valist"
//===--===//
// Deadcode checkers.
Index: cfe/trunk/test/Analysis/valist-unterminated.c
===
--- cfe/trunk/test/Analysis/valist-unterminated.c
+++ cfe/trunk/test/Analysis/valist-unterminated.c
@@ -1,49 +1,56 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple hexagon-unknown-linux -analyzer-checker=core,valist.Unterminated,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,valist.Unterminated,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
void f1(int fst, ...) {
va_list va;
va_start(va, fst); // expected-note{{Initialized va_list}}
- return; // expected-warning{{Initialized va_list 'va' is leaked}} expected-note{{Initialized va_list 'va' is leaked}}
+ return; // expected-warning{{Initialized va_list 'va' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va' is leaked}}
}
void f2(int fst, ...) {
va_list va;
va_start(va, fst); // expected-note{{Initialized va_list}}
va_end(va); // expected-note{{Ended va_list}}
va_start(va, fst); // expected-note{{Initialized va_list}}
-} // expected-warning{{Initialized va_list 'va' is leaked}} expected-note{{Initialized va_list 'va' is leaked}}}
+} // expected-warning{{Initialized va_list 'va' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va' is leaked}}
void f3(int fst, ...) {
va_list va, va2;
va_start(va, fst);
va_copy(va2, va); // expected-note{{Initialized va_list}}
- va_end(va); // expected-warning{{Initialized va_list 'va2' is leaked}} expected-note{{Initialized va_list 'va2' is leaked}}
+ va_end(va); // expected-warning{{Initialized va_list 'va2' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va2' is leaked}}
}
void f4(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
- return; // expected-warning{{Initialized va_list is leaked}} expected-note{{Initialized va_list is leaked}}
+ return; // expected-warning{{Initialized va_list is leaked}}
+ // expected-note@-1{{Initialized va_list is leaked}}
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}}
+ // expected-note@-1{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
(void)va_arg(*fst, int);
//FIXME: this should NOT cause a warning
- va_end(*fst); // expected-warning{{Initialized va_list is leaked}} expected-note{{Initialized va_list is leaked}}
+ va_end(*fst); // expected-warning{{Initialized va_list is leaked}}
+ // expected-note@-1{{Initialized va_list is leaked}}
}
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
va_start(*y,fst); // expected-note{{Initialized va_list}}
-} // expected-warning{{Initialized va_list 'x' is leaked}} expected-note{{Initia
[PATCH] D30157: [analyzer] Improve valist check
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. It's great to see things move out of alpha and finally become actually accessible to the majority of users. Thanks! https://reviews.llvm.org/D30157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun added a comment. In https://reviews.llvm.org/D30157#689374, @danielmarjamaki wrote: > I am running this checker right now on various projects. Here are currently > seen results.. https://drive.google.com/open?id=0BykPmWrCOxt2STZMOXZ5OGlwM3c > > Feel free to look at it and see if there are FPs or TPs. Thank you for running this check on those projects! I did not do a deep review of the findings but some of the results I checked are true positives. Also, the number of results are not scary, none of the projects generated tons of results. https://reviews.llvm.org/D30157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun updated this revision to Diff 90676.
xazax.hun marked an inline comment as done.
xazax.hun added a comment.
- Improve the readability of test files.
- Rebased on latest ToT.
https://reviews.llvm.org/D30157
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
Index: test/Analysis/valist-unterminated.c
===
--- test/Analysis/valist-unterminated.c
+++ test/Analysis/valist-unterminated.c
@@ -1,49 +1,56 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple hexagon-unknown-linux -analyzer-checker=core,valist.Unterminated,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu -analyzer-checker=core,valist.Unterminated,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
void f1(int fst, ...) {
va_list va;
va_start(va, fst); // expected-note{{Initialized va_list}}
- return; // expected-warning{{Initialized va_list 'va' is leaked}} expected-note{{Initialized va_list 'va' is leaked}}
+ return; // expected-warning{{Initialized va_list 'va' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va' is leaked}}
}
void f2(int fst, ...) {
va_list va;
va_start(va, fst); // expected-note{{Initialized va_list}}
va_end(va); // expected-note{{Ended va_list}}
va_start(va, fst); // expected-note{{Initialized va_list}}
-} // expected-warning{{Initialized va_list 'va' is leaked}} expected-note{{Initialized va_list 'va' is leaked}}}
+} // expected-warning{{Initialized va_list 'va' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va' is leaked}}
void f3(int fst, ...) {
va_list va, va2;
va_start(va, fst);
va_copy(va2, va); // expected-note{{Initialized va_list}}
- va_end(va); // expected-warning{{Initialized va_list 'va2' is leaked}} expected-note{{Initialized va_list 'va2' is leaked}}
+ va_end(va); // expected-warning{{Initialized va_list 'va2' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va2' is leaked}}
}
void f4(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
- return; // expected-warning{{Initialized va_list is leaked}} expected-note{{Initialized va_list is leaked}}
+ return; // expected-warning{{Initialized va_list is leaked}}
+ // expected-note@-1{{Initialized va_list is leaked}}
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}}
+ // expected-note@-1{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
(void)va_arg(*fst, int);
//FIXME: this should NOT cause a warning
- va_end(*fst); // expected-warning{{Initialized va_list is leaked}} expected-note{{Initialized va_list is leaked}}
+ va_end(*fst); // expected-warning{{Initialized va_list is leaked}}
+ // expected-note@-1{{Initialized va_list is leaked}}
}
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
va_start(*y,fst); // expected-note{{Initialized va_list}}
-} // expected-warning{{Initialized va_list 'x' is leaked}} expected-note{{Initialized va_list 'x' is leaked}}
+} // expected-warning{{Initialized va_list 'x' is leaked}}
+ // expected-note@-1{{Initialized va_list 'x' is leaked}}
void f8(int *fst, ...) {
va_list x;
@@ -54,9 +61,12 @@
void reinit(int *fst, ...) {
va_list va;
- va_start(va, fst); // expected-note{{Initialized va_list}} expected-note{{Initialized va_list}}
- va_start(va, fst); // expected-warning{{Initialized va_list 'va' is initialized again}} expected-note{{Initialized va_list 'va' is initialized again}}
-} // expected-warning{{Initialized va_list 'va' is leaked}} expected-note{{Initialized va_list 'va' is leaked}}
+ va_start(va, fst); // expected-note{{Initialized va_list}}
+ // expected-note@-1{{Initialized va_list}}
+ va_start(va, fst); // expected-warning{{Initialized va_list 'va' is initialized again}}
+ // expected-note@-1{{Initialized va_list 'va' is initialized again}}
+} // expected-warning{{Initialized va_list 'va' is leaked}}
+ // expected-note@-1{{Initialized va_list 'va' is leaked}}
void reinitOk(int *fst, ...) {
va_list va;
@@ -69,20 +79,23 @@
void copyself(int fst, ...) {
va_list va;
va_start(va, fst); // expected-note{{Initialized va_list}}
- va_copy(va, va); // expected-warning{{va_list 'va' is copied onto itself}} expected-note{{va_list 'va' is copie
[PATCH] D30157: [analyzer] Improve valist check
zaks.anna added inline comments.
Comment at: test/Analysis/valist-uninitialized-no-undef.c:25
+ va_list va;
+ vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf'
is called with an uninitialized va_list argument}} expected-note{{Function
'vprintf' is called with an uninitialized va_list argument}}
expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is
false}}
+}
Please, split the long "expected" lines into multiple lines - one per note. It
will improve readability in non-wrapping editors. Thanks!
https://reviews.llvm.org/D30157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
danielmarjamaki added a comment. I am running this checker right now on various projects. Here are currently seen results.. https://drive.google.com/open?id=0BykPmWrCOxt2STZMOXZ5OGlwM3c Feel free to look at it and see if there are FPs or TPs. https://reviews.llvm.org/D30157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun updated this revision to Diff 89857.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.
- Move the check out of alpha.
https://reviews.llvm.org/D30157
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
Index: test/Analysis/valist-unterminated.c
===
--- test/Analysis/valist-unterminated.c
+++ test/Analysis/valist-unterminated.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,valist.Unterminated,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,valist.Unterminated,valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -28,9 +29,8 @@
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}} expected-note{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
Index: test/Analysis/valist-uninitialized.c
===
--- test/Analysis/valist-uninitialized.c
+++ test/Analysis/valist-uninitialized.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,valist.Uninitialized,valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -38,13 +39,6 @@
va_end(fst);
} // no-warning
-//FIXME: this should not cause a warning
-void f6(va_list *fst, ...) {
- va_start(*fst, fst);
- (void)va_arg(*fst, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
- va_end(*fst);
-}
-
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
@@ -141,38 +135,31 @@
(void)va_arg(arg, int);
} //no-warning
-// This is the same function as the previous one, but it is called in call_uses_arg2(),
-// and the warning is generated during the analysis of call_uses_arg2().
-void inlined_uses_arg(va_list arg) {
- (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
-}
-
-void call_inlined_uses_arg(int fst, ...) {
- va_list va;
- inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}}
-}
-
void call_vprintf_ok(int isstring, ...) {
va_list va;
va_start(va, isstring);
vprintf(isstring ? "%s" : "%d", va);
va_end(va);
} //no-warning
-void call_vprintf_bad(int isstring, ...) {
+void call_some_other_func(int n, ...) {
va_list va;
- vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}}
-}
+ some_library_function(n, va);
+} //no-warning
-void call_vsprintf_bad(char *buffer, ...) {
- va_list va;
- va_start(va, buffer); // expected-note{{Initialized va_list}}
- va_end(va); // expected-note{{Ended va_list}}
- vsprintf(buffer, "%s %d %d %lf %03d", va); //expected-warning{{Function 'vsprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vsprintf' is called with an uninitialized va_list argument}}
+void inlined_uses_arg_good(va_list arg) {
+ (void)va_arg(arg, int);
}
-void call_some_other_func(int n, ...) {
+void call_inlined_uses_arg_good(int fst, ...) {
va_list va;
- some_library_function(n, va);
-} //no-warning
+ va_start(va, fst);
+ inlined_uses_arg_good(va);
+ va_end(va);
+}
+void va_copy_test(va_list arg) {
+ va_list dst;
+ va_copy(dst, arg);
+ va_end(dst);
+}
Index: test/Analysis/valist-uninitialized-no-undef.c
===
--- /dev/null
+++ t
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun updated this revision to Diff 89780.
xazax.hun added a comment.
- Minor style improvement.
https://reviews.llvm.org/D30157
Files:
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
Index: test/Analysis/valist-unterminated.c
===
--- test/Analysis/valist-unterminated.c
+++ test/Analysis/valist-unterminated.c
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -28,9 +29,8 @@
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}} expected-note{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
Index: test/Analysis/valist-uninitialized.c
===
--- test/Analysis/valist-uninitialized.c
+++ test/Analysis/valist-uninitialized.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -38,13 +39,6 @@
va_end(fst);
} // no-warning
-//FIXME: this should not cause a warning
-void f6(va_list *fst, ...) {
- va_start(*fst, fst);
- (void)va_arg(*fst, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
- va_end(*fst);
-}
-
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
@@ -141,38 +135,31 @@
(void)va_arg(arg, int);
} //no-warning
-// This is the same function as the previous one, but it is called in call_uses_arg2(),
-// and the warning is generated during the analysis of call_uses_arg2().
-void inlined_uses_arg(va_list arg) {
- (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
-}
-
-void call_inlined_uses_arg(int fst, ...) {
- va_list va;
- inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}}
-}
-
void call_vprintf_ok(int isstring, ...) {
va_list va;
va_start(va, isstring);
vprintf(isstring ? "%s" : "%d", va);
va_end(va);
} //no-warning
-void call_vprintf_bad(int isstring, ...) {
+void call_some_other_func(int n, ...) {
va_list va;
- vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}}
-}
+ some_library_function(n, va);
+} //no-warning
-void call_vsprintf_bad(char *buffer, ...) {
- va_list va;
- va_start(va, buffer); // expected-note{{Initialized va_list}}
- va_end(va); // expected-note{{Ended va_list}}
- vsprintf(buffer, "%s %d %d %lf %03d", va); //expected-warning{{Function 'vsprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vsprintf' is called with an uninitialized va_list argument}}
+void inlined_uses_arg_good(va_list arg) {
+ (void)va_arg(arg, int);
}
-void call_some_other_func(int n, ...) {
+void call_inlined_uses_arg_good(int fst, ...) {
va_list va;
- some_library_function(n, va);
-} //no-warning
+ va_start(va, fst);
+ inlined_uses_arg_good(va);
+ va_end(va);
+}
+void va_copy_test(va_list arg) {
+ va_list dst;
+ va_copy(dst, arg);
+ va_end(dst);
+}
Index: test/Analysis/valist-uninitialized-no-undef.c
===
--- /dev/null
+++ test/Analysis/valist-uninitialized-no-undef.c
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -veri
[PATCH] D30157: [analyzer] Improve valist check
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:189 + const auto *EReg = dyn_cast_or_null(Reg); + return EReg && VaListModelledAsArray ? EReg->getSuperRegion() : Reg; +} I would personally recommend parentheses around EReg and VaListModelledAsArray to highlight the precedence. https://reviews.llvm.org/D30157 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun added inline comments.
Comment at: test/Analysis/valist-uninitialized-no-undef.c:19
+ // FIXME: There should be no warning for this.
+ (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an
uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized
va_list}}
+ va_end(*fst);
xazax.hun wrote:
> NoQ wrote:
> > As the patch tries to handle symbolic va_list regions, i wonder what's so
> > particularly hard about this false positive (apart from its being obviously
> > rare, by the way did you actually see such code?).
> What is strange, this case does work with the hexagon AST variant.
Also, I did not see such code inproduction yet
https://reviews.llvm.org/D30157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun updated this revision to Diff 89187.
xazax.hun marked 3 inline comments as done.
xazax.hun added a comment.
- Fixed a comment.
https://reviews.llvm.org/D30157
Files:
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
Index: test/Analysis/valist-unterminated.c
===
--- test/Analysis/valist-unterminated.c
+++ test/Analysis/valist-unterminated.c
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -28,9 +29,8 @@
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}} expected-note{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
Index: test/Analysis/valist-uninitialized.c
===
--- test/Analysis/valist-uninitialized.c
+++ test/Analysis/valist-uninitialized.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -38,13 +39,6 @@
va_end(fst);
} // no-warning
-//FIXME: this should not cause a warning
-void f6(va_list *fst, ...) {
- va_start(*fst, fst);
- (void)va_arg(*fst, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
- va_end(*fst);
-}
-
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
@@ -141,38 +135,31 @@
(void)va_arg(arg, int);
} //no-warning
-// This is the same function as the previous one, but it is called in call_uses_arg2(),
-// and the warning is generated during the analysis of call_uses_arg2().
-void inlined_uses_arg(va_list arg) {
- (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
-}
-
-void call_inlined_uses_arg(int fst, ...) {
- va_list va;
- inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}}
-}
-
void call_vprintf_ok(int isstring, ...) {
va_list va;
va_start(va, isstring);
vprintf(isstring ? "%s" : "%d", va);
va_end(va);
} //no-warning
-void call_vprintf_bad(int isstring, ...) {
+void call_some_other_func(int n, ...) {
va_list va;
- vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}}
-}
+ some_library_function(n, va);
+} //no-warning
-void call_vsprintf_bad(char *buffer, ...) {
- va_list va;
- va_start(va, buffer); // expected-note{{Initialized va_list}}
- va_end(va); // expected-note{{Ended va_list}}
- vsprintf(buffer, "%s %d %d %lf %03d", va); //expected-warning{{Function 'vsprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vsprintf' is called with an uninitialized va_list argument}}
+void inlined_uses_arg_good(va_list arg) {
+ (void)va_arg(arg, int);
}
-void call_some_other_func(int n, ...) {
+void call_inlined_uses_arg_good(int fst, ...) {
va_list va;
- some_library_function(n, va);
-} //no-warning
+ va_start(va, fst);
+ inlined_uses_arg_good(va);
+ va_end(va);
+}
+void va_copy_test(va_list arg) {
+ va_list dst;
+ va_copy(dst, arg);
+ va_end(dst);
+}
Index: test/Analysis/valist-uninitialized-no-undef.c
===
--- /dev/null
+++ test/Analysis/valist-uninitialized-no-undef.c
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-outp
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:178
+VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay;
+ const MemRegion *Reg = SV.getAsRegion();
+ if (const auto *DeclReg = Reg->getAs()) {
NoQ wrote:
> I suspect that UnknownVal should also be handled before that, otherwise we'd
> have null dereference on the next line.
Indeed.
Comment at: test/Analysis/valist-uninitialized-no-undef.c:5
+
+// This is the same function as the previous one, but it is called in
call_inlined_uses_arg(),
+// and the warning is generated during the analysis of call_inlined_uses_arg().
NoQ wrote:
> Hmm, where's the previous one?
Tha calling function is after this one.
Comment at: test/Analysis/valist-uninitialized-no-undef.c:19
+ // FIXME: There should be no warning for this.
+ (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an
uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized
va_list}}
+ va_end(*fst);
NoQ wrote:
> As the patch tries to handle symbolic va_list regions, i wonder what's so
> particularly hard about this false positive (apart from its being obviously
> rare, by the way did you actually see such code?).
What is strange, this case does work with the hexagon AST variant.
https://reviews.llvm.org/D30157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun updated this revision to Diff 89185.
xazax.hun added a comment.
- Address some review comments.
- Add some additional tests.
- Fixed some false positives (checking for symbolic values for va_copy and more
robust detection of which valist model is used by the platform)
- I have run the checker on https://github.com/rathena/rathena, there were no
false positives or crashes using the current state. It is a 170KLOC C project.
https://reviews.llvm.org/D30157
Files:
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
Index: test/Analysis/valist-unterminated.c
===
--- test/Analysis/valist-unterminated.c
+++ test/Analysis/valist-unterminated.c
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -28,9 +29,8 @@
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}} expected-note{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
Index: test/Analysis/valist-uninitialized.c
===
--- test/Analysis/valist-uninitialized.c
+++ test/Analysis/valist-uninitialized.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -38,13 +39,6 @@
va_end(fst);
} // no-warning
-//FIXME: this should not cause a warning
-void f6(va_list *fst, ...) {
- va_start(*fst, fst);
- (void)va_arg(*fst, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
- va_end(*fst);
-}
-
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
@@ -141,38 +135,31 @@
(void)va_arg(arg, int);
} //no-warning
-// This is the same function as the previous one, but it is called in call_uses_arg2(),
-// and the warning is generated during the analysis of call_uses_arg2().
-void inlined_uses_arg(va_list arg) {
- (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
-}
-
-void call_inlined_uses_arg(int fst, ...) {
- va_list va;
- inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}}
-}
-
void call_vprintf_ok(int isstring, ...) {
va_list va;
va_start(va, isstring);
vprintf(isstring ? "%s" : "%d", va);
va_end(va);
} //no-warning
-void call_vprintf_bad(int isstring, ...) {
+void call_some_other_func(int n, ...) {
va_list va;
- vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}}
-}
+ some_library_function(n, va);
+} //no-warning
-void call_vsprintf_bad(char *buffer, ...) {
- va_list va;
- va_start(va, buffer); // expected-note{{Initialized va_list}}
- va_end(va); // expected-note{{Ended va_list}}
- vsprintf(buffer, "%s %d %d %lf %03d", va); //expected-warning{{Function 'vsprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vsprintf' is called with an uninitialized va_list argument}}
+void inlined_uses_arg_good(va_list arg) {
+ (void)va_arg(arg, int);
}
-void call_some_other_func(int n, ...) {
+void call_inlined_uses_arg_good(int fst, ...) {
va_list va;
- some_library_function(n, va);
-} //no-warning
+ va_start(va, fst);
+ inlined_uses_arg_good(va);
+ va_end(va);
+}
+void va_copy_test(va_list arg) {
+ va_list dst;
+ va_copy(dst, arg);
+ va_end(dst);
+}
Index: test/Analysis/valist-uninitialized-no-u
[PATCH] D30157: [analyzer] Improve valist check
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/ValistChecker.cpp:178
+VaListModelledAsArray = Cast->getCastKind() == CK_ArrayToPointerDecay;
+ const MemRegion *Reg = SV.getAsRegion();
+ if (const auto *DeclReg = Reg->getAs()) {
I suspect that UnknownVal should also be handled before that, otherwise we'd
have null dereference on the next line.
Comment at: test/Analysis/valist-uninitialized-no-undef.c:5
+
+// This is the same function as the previous one, but it is called in
call_inlined_uses_arg(),
+// and the warning is generated during the analysis of call_inlined_uses_arg().
Hmm, where's the previous one?
Comment at: test/Analysis/valist-uninitialized-no-undef.c:19
+ // FIXME: There should be no warning for this.
+ (void)va_arg(*fst, int); // expected-warning{{va_arg() is called on an
uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized
va_list}}
+ va_end(*fst);
As the patch tries to handle symbolic va_list regions, i wonder what's so
particularly hard about this false positive (apart from its being obviously
rare, by the way did you actually see such code?).
https://reviews.llvm.org/D30157
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30157: [analyzer] Improve valist check
xazax.hun created this revision.
This patch makes the valist check more robust to the different kind of ASTs
that are generated on different platforms:
Generated on x86_64-pc-linux-gnu:
|-TypedefDecl 0x1d07a40 <> implicit
__builtin_ms_va_list 'char *'
| `-PointerType 0x1d07a00 'char *'
| `-BuiltinType 0x1d071b0 'char'
|-TypedefDecl 0x1d07d08 <> implicit referenced
__builtin_va_list 'struct __va_list_tag [1]'
| `-ConstantArrayType 0x1d07cb0 'struct __va_list_tag [1]' 1
| `-RecordType 0x1d07b20 'struct __va_list_tag'
| `-Record 0x1d07a90 '__va_list_tag'
Generated on hexagon-unknown-linux:
-TypedefDecl 0x6c47e0 <> implicit referenced
__builtin_va_list 'char *'
| `-PointerType 0x6c47a0 'char *'
| `-BuiltinType 0x6c4020 'char'
|-TypedefDecl 0x6c4860
col:27 referenced va_list '__builtin_va_list':'char *'
| `-TypedefType 0x6c4830 '__builtin_va_list' sugar
| |-Typedef 0x6c47e0 '__builtin_va_list'
| `-PointerType 0x6c47a0 'char *'
| `-BuiltinType 0x6c4020 'char'
This patch also manages to fix one of the FIXMEs in the tests.
Note that, some tests are only there for x86_64-pc-linux-gnu. The reason is
that, the analyzer manages to notice the uninitializedness of va_list on
hexagon-unknown-linux, so it generates a sink node before this check could be
triggered.
https://reviews.llvm.org/D30157
Files:
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
test/Analysis/valist-uninitialized-no-undef.c
test/Analysis/valist-uninitialized.c
test/Analysis/valist-unterminated.c
Index: test/Analysis/valist-unterminated.c
===
--- test/Analysis/valist-unterminated.c
+++ test/Analysis/valist-unterminated.c
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Unterminated,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -28,9 +29,8 @@
}
void f5(va_list fst, ...) {
- va_start(fst, fst);
- //FIXME: this should cause a warning
-} // no-warning
+ va_start(fst, fst); // expected-note{{Initialized va_list}}
+} // expected-warning{{Initialized va_list}} expected-note{{Initialized va_list}}
void f6(va_list *fst, ...) {
va_start(*fst, fst); // expected-note{{Initialized va_list}}
Index: test/Analysis/valist-uninitialized.c
===
--- test/Analysis/valist-uninitialized.c
+++ test/Analysis/valist-uninitialized.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple hexagon-unknown-linux -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,alpha.valist.Uninitialized,alpha.valist.CopyToSelf -analyzer-disable-checker=core.CallAndMessage -analyzer-output=text -analyzer-store=region -verify %s
#include "Inputs/system-header-simulator-for-valist.h"
@@ -38,13 +39,6 @@
va_end(fst);
} // no-warning
-//FIXME: this should not cause a warning
-void f6(va_list *fst, ...) {
- va_start(*fst, fst);
- (void)va_arg(*fst, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
- va_end(*fst);
-}
-
void f7(int *fst, ...) {
va_list x;
va_list *y = &x;
@@ -141,36 +135,13 @@
(void)va_arg(arg, int);
} //no-warning
-// This is the same function as the previous one, but it is called in call_uses_arg2(),
-// and the warning is generated during the analysis of call_uses_arg2().
-void inlined_uses_arg(va_list arg) {
- (void)va_arg(arg, int); //expected-warning{{va_arg() is called on an uninitialized va_list}} expected-note{{va_arg() is called on an uninitialized va_list}}
-}
-
-void call_inlined_uses_arg(int fst, ...) {
- va_list va;
- inlined_uses_arg(va); // expected-note{{Calling 'inlined_uses_arg'}}
-}
-
void call_vprintf_ok(int isstring, ...) {
va_list va;
va_start(va, isstring);
vprintf(isstring ? "%s" : "%d", va);
va_end(va);
} //no-warning
-void call_vprintf_bad(int isstring, ...) {
- va_list va;
- vprintf(isstring ? "%s" : "%d", va); //expected-warning{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Function 'vprintf' is called with an uninitialized va_list argument}} expected-note{{Assuming 'isstring' is 0}} expected-note{{'?' condition is false}}
-}
-
-void call_vsprintf_b
