From 11b8f2321286705460f75f9b0d3a48e5c84fb791 Mon Sep 17 00:00:00 2001 From: George Karpenkov Date: Wed, 29 Aug 2018 22:48:50 +0000 Subject: [PATCH] [analyzer] Improve tracing for uninitialized struct fields rdar://13729267 Differential Revision: https://reviews.llvm.org/D51323 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@340986 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/CallAndMessageChecker.cpp | 2 + test/Analysis/uninit-vals-ps-region.m | 93 ---------- .../{uninit-vals-ps.c => uninit-vals.c} | 58 +++++- test/Analysis/uninit-vals.cpp | 3 +- test/Analysis/uninit-vals.m | 173 ++++++++++++++++-- 5 files changed, 211 insertions(+), 118 deletions(-) delete mode 100644 test/Analysis/uninit-vals-ps-region.m rename test/Analysis/{uninit-vals-ps.c => uninit-vals.c} (60%) diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp index e540f8a3df..1f9bdeb059 100644 --- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp +++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp @@ -304,6 +304,8 @@ bool CallAndMessageChecker::PreVisitProcessArg(CheckerContext &C, auto R = llvm::make_unique(*BT, os.str(), N); R->addRange(ArgRange); + if (ArgEx) + bugreporter::trackNullOrUndefValue(N, ArgEx, *R); // FIXME: enhance track back for uninitialized value for arbitrary // memregions C.emitReport(std::move(R)); diff --git a/test/Analysis/uninit-vals-ps-region.m b/test/Analysis/uninit-vals-ps-region.m deleted file mode 100644 index 18010f240d..0000000000 --- a/test/Analysis/uninit-vals-ps-region.m +++ /dev/null @@ -1,93 +0,0 @@ -// RUN: %clang_analyze_cc1 -analyzer-store=region -analyzer-checker=core -verify %s - -struct s { - int data; -}; - -struct s global; - -void g(int); - -void f4() { - int a; - if (global.data == 0) - a = 3; - if (global.data == 0) // When the true branch is feasible 'a = 3'. - g(a); // no-warning -} - - -// Test uninitialized value due to part of the structure being uninitialized. -struct TestUninit { int x; int y; }; -struct TestUninit test_uninit_aux(); -void test_unit_aux2(int); -void test_uninit_pos() { - struct TestUninit v1 = { 0, 0 }; - struct TestUninit v2 = test_uninit_aux(); - int z; - v1.y = z; // expected-warning{{Assigned value is garbage or undefined}} - test_unit_aux2(v2.x + v1.y); -} -void test_uninit_pos_2() { - struct TestUninit v1 = { 0, 0 }; - struct TestUninit v2; - test_unit_aux2(v2.x + v1.y); // expected-warning{{The left operand of '+' is a garbage value}} -} -void test_uninit_pos_3() { - struct TestUninit v1 = { 0, 0 }; - struct TestUninit v2; - test_unit_aux2(v1.y + v2.x); // expected-warning{{The right operand of '+' is a garbage value}} -} - -void test_uninit_neg() { - struct TestUninit v1 = { 0, 0 }; - struct TestUninit v2 = test_uninit_aux(); - test_unit_aux2(v2.x + v1.y); -} - -extern void test_uninit_struct_arg_aux(struct TestUninit arg); -void test_uninit_struct_arg() { - struct TestUninit x; - test_uninit_struct_arg_aux(x); // expected-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}} -} - -@interface Foo -- (void) passVal:(struct TestUninit)arg; -@end -void testFoo(Foo *o) { - struct TestUninit x; - [o passVal:x]; // expected-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}} -} - -// Test case from . That shows an uninitialized value -// being used in the LHS of a compound assignment. -void rdar_7780304() { - typedef struct s_r7780304 { int x; } s_r7780304; - s_r7780304 b; - b.x |= 1; // expected-warning{{The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage}} -} - - -// The flip side of PR10163 -- float arrays that are actually uninitialized -// (The main test is in uninit-vals.m) -void test_PR10163(float); -void PR10163 (void) { - float x[2]; - test_PR10163(x[1]); // expected-warning{{uninitialized value}} -} - -struct MyStr { - int x; - int y; -}; -void swap(struct MyStr *To, struct MyStr *From) { - // This is not really a swap but close enough for our test. - To->x = From->x; - To->y = From->y; // no warning -} -int test_undefined_member_assignment_in_swap(struct MyStr *s2) { - struct MyStr s1; - s1.x = 5; - swap(s2, &s1); - return s2->y; // expected-warning{{Undefined or garbage value returned to caller}} -} diff --git a/test/Analysis/uninit-vals-ps.c b/test/Analysis/uninit-vals.c similarity index 60% rename from test/Analysis/uninit-vals-ps.c rename to test/Analysis/uninit-vals.c index ee25d9d9a6..d24b458747 100644 --- a/test/Analysis/uninit-vals-ps.c +++ b/test/Analysis/uninit-vals.c @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-store=region -fblocks -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core -fblocks -verify -analyzer-output=text %s struct FPRec { void (*my_func)(int * x); @@ -13,30 +13,34 @@ int f1_a(struct FPRec* foo) { } int f1_b() { - int x; + int x; // expected-note{{'x' declared without an initial value}} return bar(x)+1; // expected-warning{{1st function call argument is an uninitialized value}} + // expected-note@-1{{1st function call argument is an uninitialized value}} } int f2() { - int x; + int x; // expected-note{{'x' declared without an initial value}} if (x+1) // expected-warning{{The left operand of '+' is a garbage value}} + // expected-note@-1{{The left operand of '+' is a garbage value}} return 1; return 2; } int f2_b() { - int x; + int x; // expected-note{{'x' declared without an initial value}} return ((1+x)+2+((x))) + 1 ? 1 : 2; // expected-warning{{The right operand of '+' is a garbage value}} + // expected-note@-1{{The right operand of '+' is a garbage value}} } int f3(void) { - int i; + int i; // expected-note{{'i' declared without an initial value}} int *p = &i; if (*p > 0) // expected-warning{{The left operand of '>' is a garbage value}} + // expected-note@-1{{The left operand of '>' is a garbage value}} return 0; else return 1; @@ -59,14 +63,17 @@ int f5(void) { void f6(int x) { int a[20]; - if (x == 25) {} + if (x == 25) {} // expected-note{{Assuming 'x' is equal to 25}} + // expected-note@-1{{Taking true branch}} if (a[x] == 123) {} // expected-warning{{The left operand of '==' is a garbage value due to array index out of bounds}} + // expected-note@-1{{The left operand of '==' is a garbage value due to array index out of bounds}} } int ret_uninit() { - int i; + int i; // expected-note{{'i' declared without an initial value}} int *p = &i; return *p; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} } // @@ -141,6 +148,9 @@ typedef void (*RetVoidFuncType)(); int test_radar12278788_FP() { RetVoidFuncType f = foo_radar12278788_fp; return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}} + //expected-note@-1 {{Undefined or garbage value returned to caller}} + //expected-note@-2 {{Calling 'foo_radar12278788_fp'}} + //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}} } void rdar13665798() { @@ -150,8 +160,40 @@ void rdar13665798() { ^void() { return foo_radar12278788(); // no-warning }(); - ^int() { + ^int() { // expected-note{{Calling anonymous block}} RetVoidFuncType f = foo_radar12278788_fp; return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}} + //expected-note@-1 {{Undefined or garbage value returned to caller}} + //expected-note@-2 {{Calling 'foo_radar12278788_fp'}} + //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}} }(); } + +struct Point { + int x, y; +}; + +struct Point getHalfPoint() { + struct Point p; + p.x = 0; + return p; +} + +void use(struct Point p); + +void testUseHalfPoint() { + struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}} + // expected-note@-1{{Returning from 'getHalfPoint'}} + // expected-note@-2{{'p' initialized here}} + use(p); // expected-warning{{uninitialized}} + // expected-note@-1{{uninitialized}} +} + +void testUseHalfPoint2() { + struct Point p; + p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}} + // expected-note@-1{{Returning from 'getHalfPoint'}} + // expected-note@-2{{Value assigned to 'p'}} + use(p); // expected-warning{{uninitialized}} + // expected-note@-1{{uninitialized}} +} diff --git a/test/Analysis/uninit-vals.cpp b/test/Analysis/uninit-vals.cpp index 5ab1348ae5..6ba56f0c4e 100644 --- a/test/Analysis/uninit-vals.cpp +++ b/test/Analysis/uninit-vals.cpp @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core.builtin -verify -DCHECK_FOR_CRASH %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify -analyzer-output=text %s #ifdef CHECK_FOR_CRASH // expected-no-diagnostics @@ -28,6 +28,7 @@ void foo() { c1.b.a = c2->b.a; #else c1.b.a = c2->b.a; // expected-warning{{1st function call argument is an uninitialized value}} + // expected-note@-1{{1st function call argument is an uninitialized value}} #endif } } diff --git a/test/Analysis/uninit-vals.m b/test/Analysis/uninit-vals.m index 7c18dee976..f97af1a663 100644 --- a/test/Analysis/uninit-vals.m +++ b/test/Analysis/uninit-vals.m @@ -1,4 +1,4 @@ -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -analyzer-store=region -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-output=text -verify %s typedef unsigned int NSUInteger; typedef __typeof__(sizeof(int)) size_t; @@ -9,6 +9,112 @@ void free(void *); void clang_analyzer_eval(int); +struct s { + int data; +}; + +struct s global; + +void g(int); + +void f4() { + int a; + if (global.data == 0) + a = 3; + if (global.data == 0) // When the true branch is feasible 'a = 3'. + g(a); // no-warning +} + + +// Test uninitialized value due to part of the structure being uninitialized. +struct TestUninit { int x; int y; }; +struct TestUninit test_uninit_aux(); +void test_unit_aux2(int); +void test_uninit_pos() { + struct TestUninit v1 = { 0, 0 }; + struct TestUninit v2 = test_uninit_aux(); + int z; // expected-note{{'z' declared without an initial value}} + v1.y = z; // expected-warning{{Assigned value is garbage or undefined}} + // expected-note@-1{{Assigned value is garbage or undefined}} + test_unit_aux2(v2.x + v1.y); +} +void test_uninit_pos_2() { + struct TestUninit v1 = { 0, 0 }; + struct TestUninit v2; + test_unit_aux2(v2.x + v1.y); // expected-warning{{The left operand of '+' is a garbage value}} + // expected-note@-1{{The left operand of '+' is a garbage value}} +} +void test_uninit_pos_3() { + struct TestUninit v1 = { 0, 0 }; + struct TestUninit v2; + test_unit_aux2(v1.y + v2.x); // expected-warning{{The right operand of '+' is a garbage value}} + // expected-note@-1{{The right operand of '+' is a garbage value}} +} + +void test_uninit_neg() { + struct TestUninit v1 = { 0, 0 }; + struct TestUninit v2 = test_uninit_aux(); + test_unit_aux2(v2.x + v1.y); +} + +extern void test_uninit_struct_arg_aux(struct TestUninit arg); +void test_uninit_struct_arg() { + struct TestUninit x; // expected-note{{'x' initialized here}} + test_uninit_struct_arg_aux(x); // expected-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}} + // expected-note@-1{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}} +} + +@interface Foo +- (void) passVal:(struct TestUninit)arg; +@end +void testFoo(Foo *o) { + struct TestUninit x; // expected-note{{'x' initialized here}} + [o passVal:x]; // expected-warning{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}} + // expected-note@-1{{Passed-by-value struct argument contains uninitialized data (e.g., field: 'x')}} +} + +// Test case from . That shows an uninitialized value +// being used in the LHS of a compound assignment. +void rdar_7780304() { + typedef struct s_r7780304 { int x; } s_r7780304; + s_r7780304 b; + b.x |= 1; // expected-warning{{The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage}} + // expected-note@-1{{The left expression of the compound assignment is an uninitialized value. The computed value will also be garbage}} +} + + +// The flip side of PR10163 -- float arrays that are actually uninitialized +void test_PR10163(float); +void PR10163 (void) { + float x[2]; + test_PR10163(x[1]); // expected-warning{{uninitialized value}} + // expected-note@-1{{1st function call argument is an uninitialized value}} +} + +// PR10163 -- don't warn for default-initialized float arrays. +void PR10163_default_initialized_arrays(void) { + float x[2] = {0}; + test_PR10163(x[1]); // no-warning +} + +struct MyStr { + int x; + int y; +}; +void swap(struct MyStr *To, struct MyStr *From) { + // This is not really a swap but close enough for our test. + To->x = From->x; + To->y = From->y; // expected-note{{Uninitialized value stored to field 'y'}} +} +int test_undefined_member_assignment_in_swap(struct MyStr *s2) { + struct MyStr s1; + s1.x = 5; + swap(s2, &s1); // expected-note{{Calling 'swap'}} + // expected-note@-1{{Returning from 'swap'}} + return s2->y; // expected-warning{{Undefined or garbage value returned to caller}} + // expected-note@-1{{Undefined or garbage value returned to caller}} +} + @interface A - (NSUInteger)foo; @end @@ -31,13 +137,6 @@ NSUInteger f8(A* x){ } -// PR10163 -- don't warn for default-initialized float arrays. -// (An additional test is in uninit-vals-ps-region.m) -void test_PR10163(float); -void PR10163 (void) { - float x[2] = {0}; - test_PR10163(x[1]); // no-warning -} typedef struct { @@ -62,15 +161,17 @@ void PR14765_test() { Circle *testObj = calloc(sizeof(Circle), 1); clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} testObj->origin = makePoint(0.0, 0.0); - if (testObj->size > 0) { ; } // warning occurs here + if (testObj->size > 0) { ; } // expected-note{{Taking false branch}} // FIXME: Assigning to 'testObj->origin' kills the default binding for the // whole region, meaning that we've forgotten that testObj->size should also // default to 0. Tracked by . // This should be TRUE. clang_analyzer_eval(testObj->size == 0); // expected-warning{{UNKNOWN}} + // expected-note@-1{{UNKNOWN}} free(testObj); } @@ -78,9 +179,11 @@ void PR14765_test() { void PR14765_argument(Circle *testObj) { int oldSize = testObj->size; clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} testObj->origin = makePoint(0.0, 0.0); clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} } @@ -106,21 +209,32 @@ void PR14765_test_int() { IntCircle *testObj = calloc(sizeof(IntCircle), 1); clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.x == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.z == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint(1, 2); - if (testObj->size > 0) { ; } // warning occurs here + if (testObj->size > 0) { ; } // expected-note{{Taking false branch}} + // expected-note@-1{{Taking false branch}} + // expected-note@-2{{Taking false branch}} + // expected-note@-3{{Taking false branch}} // FIXME: Assigning to 'testObj->origin' kills the default binding for the // whole region, meaning that we've forgotten that testObj->size should also // default to 0. Tracked by . // This should be TRUE. clang_analyzer_eval(testObj->size == 0); // expected-warning{{UNKNOWN}} + // expected-note@-1{{UNKNOWN}} clang_analyzer_eval(testObj->origin.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.z == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} free(testObj); } @@ -128,12 +242,17 @@ void PR14765_test_int() { void PR14765_argument_int(IntCircle *testObj) { int oldSize = testObj->size; clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint(1, 2); clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.z == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} } @@ -173,6 +292,7 @@ void testSmallStructsCopiedPerField() { extern void useInt(int); useInt(b.x); // no-warning useInt(b.y); // expected-warning{{uninitialized}} + // expected-note@-1{{uninitialized}} } void testLargeStructsNotCopiedPerField() { @@ -189,15 +309,23 @@ void testSmallStructInLargerStruct() { IntCircle2D *testObj = calloc(sizeof(IntCircle2D), 1); clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.x == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint2D(1, 2); - if (testObj->size > 0) { ; } // warning occurs here + if (testObj->size > 0) { ; } // expected-note{{Taking false branch}} + // expected-note@-1{{Taking false branch}} + // expected-note@-2{{Taking false branch}} clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} free(testObj); } @@ -205,11 +333,15 @@ void testSmallStructInLargerStruct() { void testCopySmallStructIntoArgument(IntCircle2D *testObj) { int oldSize = testObj->size; clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint2D(1, 2); clang_analyzer_eval(testObj->size == oldSize); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(testObj->origin.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} } void testSmallStructBitfields() { @@ -223,7 +355,9 @@ void testSmallStructBitfields() { b = a; clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} } void testSmallStructBitfieldsFirstUndef() { @@ -236,7 +370,9 @@ void testSmallStructBitfieldsFirstUndef() { b = a; clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(b.x == 1); // expected-warning{{garbage}} + // expected-note@-1{{garbage}} } void testSmallStructBitfieldsSecondUndef() { @@ -249,7 +385,9 @@ void testSmallStructBitfieldsSecondUndef() { b = a; clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} clang_analyzer_eval(b.y == 2); // expected-warning{{garbage}} + // expected-note@-1{{garbage}} } void testSmallStructBitfieldsFirstUnnamed() { @@ -260,11 +398,13 @@ void testSmallStructBitfieldsFirstUnnamed() { a.y = 2; - b = a; + b = a; // expected-note{{Value assigned to 'c'}} clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} - b = c; + b = c; // expected-note{{Uninitialized value stored to 'b.y'}} clang_analyzer_eval(b.y == 2); // expected-warning{{garbage}} + // expected-note@-1{{garbage}} } void testSmallStructBitfieldsSecondUnnamed() { @@ -275,10 +415,11 @@ void testSmallStructBitfieldsSecondUnnamed() { a.x = 1; - b = a; + b = a; // expected-note{{Value assigned to 'c'}} clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}} + // expected-note@-1{{TRUE}} - b = c; + b = c; // expected-note{{Uninitialized value stored to 'b.x'}} clang_analyzer_eval(b.x == 1); // expected-warning{{garbage}} + // expected-note@-1{{garbage}} } - -- 2.40.0