llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Benedek Kaibas (benedekaibas) <details> <summary>Changes</summary> During the development of the lifetime checkers I had many important test cases saved locally and were never imported to the test files of the checkers. This PR adds those missing test cases to the test files. By adding these test cases it helps tracking the state of the lifetime checkers and what improvements they need in order to have better coverage and reduce the false positive rate. *AI policy*: During the development I have consulted with claude sonnet 5 to list me test case scenarios it thinks are useful and then reviewed its feedback and implemented/wrote those test cases by myself. Since some of the test cases are edge cases I wanted to make sure the implementation is on the right track. --- Patch is 27.30 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/212254.diff 6 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp (+10-15) - (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp (+8) - (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h (+4) - (modified) clang/lib/StaticAnalyzer/Checkers/UseAfterLifetimeEnd.cpp (+7-5) - (modified) clang/test/Analysis/dangling-ptr-deref.cpp (+43-24) - (modified) clang/test/Analysis/lifetime-bound.cpp (+142-31) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp b/clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp index 9b4b1056c8ee6..3daad5f131ae6 100644 --- a/clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp @@ -15,8 +15,8 @@ class DanglingPtrDeref : public Checker<check::Location, check::PostCall> { void checkLocation(SVal Loc, bool IsLoad, const Stmt *S, CheckerContext &C) const; void checkPostCall(const CallEvent &Call, CheckerContext &C) const; - void reportUseAfterScope(const MemRegion *Region, ExplodedNode *N, - CheckerContext &C) const; + void reportUseAfterScope(const MemRegion *Region, const Stmt *S, + ExplodedNode *N, CheckerContext &C) const; const BugType BugMsg{this, "ReportDanglingPtrDeref", "LifetimeBound"}; }; @@ -45,7 +45,7 @@ void DanglingPtrDeref::checkLocation(SVal Loc, bool IsLoad, const Stmt *S, if (const MemRegion *LocRegion = Loc.getAsRegion()) { if (lifetime_modeling::isDeallocated(State, LocRegion)) { if (ExplodedNode *N = C.generateNonFatalErrorNode(State)) - reportUseAfterScope(LocRegion, N, C); + reportUseAfterScope(LocRegion, S, N, C); } } } @@ -62,27 +62,20 @@ void DanglingPtrDeref::checkPostCall(const CallEvent &Call, if (const MemRegion *ArgRegion = Call.getArgSVal(Idx).getAsRegion()) if (lifetime_modeling::isDeallocated(State, ArgRegion)) if (ExplodedNode *N = C.generateNonFatalErrorNode()) - reportUseAfterScope(ArgRegion, N, C); + reportUseAfterScope(ArgRegion, Call.getArgExpr(Idx), N, C); } } -static std::string getRegionName(const MemRegion *Reg) { - // FIXME: Once the checker supports heap allocation, more region kinds - // should be handled to produce the correct descriptive name. - if (const std::string &RegName = Reg->getDescriptiveName(); !RegName.empty()) - return RegName; - llvm_unreachable("unhandled region"); -} - void DanglingPtrDeref::reportUseAfterScope(const MemRegion *Region, - ExplodedNode *N, + const Stmt *S, ExplodedNode *N, CheckerContext &C) const { auto BR = std::make_unique<PathSensitiveBugReport>( BugMsg, - (llvm::Twine("Use of ") + getRegionName(Region) + + (llvm::Twine("Use of ") + lifetime_modeling::getRegionName(Region) + " after its lifetime ended."), N); BR->addVisitor<DanglingPtrDerefBRVisitor>(Region); + bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *BR); C.emitReport(std::move(BR)); } @@ -107,7 +100,9 @@ DanglingPtrDerefBRVisitor::VisitNode(const ExplodedNode *N, S, BRC.getSourceManager(), N->getStackFrame()); return std::make_shared<PathDiagnosticEventPiece>( Pos, - (getRegionName(SourceRegion) + llvm::Twine(" is destroyed here")).str(), + (lifetime_modeling::getRegionName(SourceRegion) + + llvm::Twine(" is destroyed here")) + .str(), true); } diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp index 765a95c008053..df8a554e43157 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp @@ -87,6 +87,14 @@ static ProgramStateRef bindSource(ProgramStateRef State, SVal RetVal, return State; } +std::string lifetime_modeling::getRegionName(const MemRegion *Reg) { + // FIXME: Once the checker supports heap allocation, more region kinds + // should be handled to produce the correct descriptive name. + if (const std::string &RegName = Reg->getDescriptiveName(); !RegName.empty()) + return RegName; + llvm_unreachable("unhandled region"); +} + void LifetimeModeling::checkPostCall(const CallEvent &Call, CheckerContext &C) const { ProgramStateRef State = C.getState(); diff --git a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h index cea2799db4b1d..1e2ef8f7810ac 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h +++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.h @@ -15,6 +15,10 @@ getDanglingRegionsAfterReturn(SVal Source, ProgramStateRef State, /// Returns true if the underlying MemRegion is deallocated. bool isDeallocated(ProgramStateRef State, const MemRegion *Region); + +/// Returns the descriptive name of the memory region or a placeholder if a +/// descriptive name cannot be constructed for it. +std::string getRegionName(const MemRegion *Reg); } // namespace clang::ento::lifetime_modeling #endif // LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_LIFETIMEMODELING_H diff --git a/clang/lib/StaticAnalyzer/Checkers/UseAfterLifetimeEnd.cpp b/clang/lib/StaticAnalyzer/Checkers/UseAfterLifetimeEnd.cpp index 6a2933900c01f..0f320eb910930 100644 --- a/clang/lib/StaticAnalyzer/Checkers/UseAfterLifetimeEnd.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/UseAfterLifetimeEnd.cpp @@ -1,5 +1,6 @@ #include "LifetimeModeling.h" #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h" #include "clang/StaticAnalyzer/Core/Checker.h" using namespace clang; @@ -8,7 +9,7 @@ using namespace ento; namespace { class UseAfterLifetimeEnd : public Checker<check::EndFunction> { public: - void reportDanglingSource(const MemRegion *Source, ExplodedNode *N, + void reportDanglingSource(const MemRegion *Source, SVal Val, ExplodedNode *N, CheckerContext &C) const; void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const; const BugType BugMsg{this, "UseAfterLifetimeEnd", "LifetimeBound"}; @@ -38,18 +39,19 @@ void UseAfterLifetimeEnd::checkEndFunction(const ReturnStmt *RS, if (ExplodedNode *N = C.generateNonFatalErrorNode(State, C.getPredecessor())) { for (const MemRegion *R : RetValRegion) - reportDanglingSource(R, N, C); + reportDanglingSource(R, RetVal, N, C); } } void UseAfterLifetimeEnd::reportDanglingSource(const MemRegion *Source, - ExplodedNode *N, + SVal RetVal, ExplodedNode *N, CheckerContext &C) const { auto BR = std::make_unique<PathSensitiveBugReport>( BugMsg, - (llvm::Twine("Returning value bound to '") + Source->getString() + - "' that will go out of scope"), + (llvm::Twine("Returning value bound to ") + + lifetime_modeling::getRegionName(Source) + " that will go out of scope"), N); + bugreporter::trackStoredValue(RetVal, Source, *BR); C.emitReport(std::move(BR)); } diff --git a/clang/test/Analysis/dangling-ptr-deref.cpp b/clang/test/Analysis/dangling-ptr-deref.cpp index fa9dae41421bc..bdae67bb91539 100644 --- a/clang/test/Analysis/dangling-ptr-deref.cpp +++ b/clang/test/Analysis/dangling-ptr-deref.cpp @@ -4,8 +4,8 @@ void test_case_one() { int *ptr = nullptr; { - int num = 5; - ptr = # + int num = 5; // expected-note {{'num' initialized to 5}} + ptr = # // expected-note {{Value assigned to 'ptr'}} } // expected-note@-1 {{'num' is destroyed here}} *ptr = 6; @@ -17,10 +17,10 @@ void test_case_two() { int *ptr_one = nullptr; int *ptr_two = nullptr; { - int n = 1; - int m = 2; - ptr_one = &n; - ptr_two = &m; + int n = 1; // expected-note {{'n' initialized to 1}} + int m = 2; // expected-note {{'m' initialized to 2}} + ptr_one = &n; // expected-note {{Value assigned to 'ptr_one'}} + ptr_two = &m; // expected-note {{Value assigned to 'ptr_two'}} } // expected-note@-1 {{'n' is destroyed here}} // expected-note@-2 {{'m' is destroyed here}} @@ -45,7 +45,7 @@ void test_case_three() { void test_case_four() { int *ptr = nullptr; { - int num = 5; + int num = 5; // expected-note {{'num' initialized to 5}} ptr = # } // expected-note@-1 {{'num' is destroyed here}} @@ -75,8 +75,8 @@ void test_case_seven() { // expected-note@+3 {{Loop condition is true. Entering loop body}} // expected-note@+2 {{Assuming 'i' is >= 10}} // expected-note@+1 {{Loop condition is false. Execution continues on line}} - for (int i = 0; i < 10; ++i) { - ptr = &i; + for (int i = 0; i < 10; ++i) { // expected-note {{'i' initialized to 0}} + ptr = &i; // expected-note {{Value assigned to 'ptr'}} escape(ptr); } // expected-note@-1 {{'i' is destroyed here}} @@ -88,8 +88,8 @@ void test_case_seven() { void passing_dangling_ptr_to_opaque_func() { int *ptr = nullptr; { - int num = 5; - ptr = # + int num = 5; // expected-note {{'num' initialized to 5}} + ptr = # // expected-note {{Value assigned to 'ptr'}} } // expected-note@-1 {{'num' is destroyed here}} escape(ptr); @@ -104,7 +104,7 @@ int deref_param(int *p) { return *p; } void inlined_callee_single_report() { int *ptr = nullptr; { - int num = 5; + int num = 5; // expected-note {{'num' initialized to 5}} ptr = # } // expected-note@-1 {{'num' is destroyed here}} @@ -120,7 +120,7 @@ struct MyStruct { int x; }; struct Inner { int x; }; -struct Outer { struct Inner inner; }; +struct Outer { Inner inner; }; struct A { struct B { int x; }; @@ -131,7 +131,7 @@ struct A { char member_subregion_dangling_deref() { const char *p = nullptr; { - MyBuffer tmp_buffer = {}; + MyBuffer tmp_buffer = {}; // expected-note {{Initializing to 0}} p = tmp_buffer.buffer; } // expected-note@-1 {{'tmp_buffer.buffer[0]' is destroyed here}} @@ -147,7 +147,7 @@ void passing_dangling_to_call() { const char *p = nullptr; { MyBuffer tmp_buffer = {}; - p = tmp_buffer.buffer; + p = tmp_buffer.buffer; // expected-note {{Value assigned to 'p'}} } // expected-note@-1 {{'tmp_buffer.buffer[0]' is destroyed here}} opaque(p); @@ -178,8 +178,8 @@ char member_subregion_alive_deref_pp() { void arr_elem_subreg_dangling_deref() { int *ptr = nullptr; { - int local_arr[4]; - ptr = &local_arr[1]; + int local_arr[4]; // expected-note {{'local_arr' declared without an initial value}} + ptr = &local_arr[1]; // expected-note {{Value assigned to 'ptr'}} } // expected-note@-1 {{'local_arr[1]' is destroyed here}} *ptr = 7; @@ -190,7 +190,7 @@ void arr_elem_subreg_dangling_deref() { char member_array_elem_dangling_deref() { const char *p = nullptr; { - MyBuffer tmp_buffer = {}; + MyBuffer tmp_buffer = {}; // expected-note {{Initializing to 0}} p = tmp_buffer.buffer + 3; } // expected-note@-1 {{'tmp_buffer.buffer[3]' is destroyed here}} @@ -202,7 +202,7 @@ char member_array_elem_dangling_deref() { char member_array_out_of_bounds_dangling_deref() { const char *p = nullptr; { - MyBuffer tmp_buffer = {}; + MyBuffer tmp_buffer = {}; // expected-note {{Initializing to 0}} p = tmp_buffer.buffer + 10; } // expected-note@-1 {{'tmp_buffer.buffer[10]' is destroyed here}} @@ -214,7 +214,7 @@ char member_array_out_of_bounds_dangling_deref() { int struct_field_dangling_deref() { int *p = nullptr; { - MyStruct s = {}; + MyStruct s = {}; // expected-note {{'s.x' initialized to 0}} p = &s.x; } // expected-note@-1 {{'s.x' is destroyed here}} @@ -226,7 +226,7 @@ int struct_field_dangling_deref() { int struct_array_element_dangling_deref() { int *p = nullptr; { - MyStruct arr[4] = {}; + MyStruct arr[4] = {}; // expected-note {{field 'x' initialized to 0}} p = &arr[2].x; } // expected-note@-1 {{'arr[2].x' is destroyed here}} @@ -238,7 +238,7 @@ int struct_array_element_dangling_deref() { int nested_field_dangling_deref() { int *p = nullptr; { - Outer o = {}; + Outer o = {}; // expected-note {{'o.inner.x' initialized to 0}} p = &o.inner.x; } // expected-note@-1 {{'o.inner.x' is destroyed here}} @@ -250,7 +250,7 @@ int nested_field_dangling_deref() { int nested_type_field_dangling_deref() { int *p = nullptr; { - A a = {}; + A a = {}; // expected-note {{'a.b.x' initialized to 0}} p = &a.b.x; } // expected-note@-1 {{'a.b.x' is destroyed here}} @@ -262,7 +262,7 @@ int nested_type_field_dangling_deref() { char member_subregion_dangling_deref_increment() { const char *p = nullptr; { - MyBuffer tmp_buffer = {}; + MyBuffer tmp_buffer = {}; // expected-note {{Initializing to 0}} p = tmp_buffer.buffer; } // expected-note@-1 {{'tmp_buffer.buffer[1]' is destroyed here}} @@ -271,3 +271,22 @@ char member_subregion_dangling_deref_increment() { // expected-warning@-1 {{Use of 'tmp_buffer.buffer[1]' after its lifetime ended}} // expected-note@-2 {{Use of 'tmp_buffer.buffer[1]' after its lifetime ended}} } + +void chain() { + int *ptr = nullptr; + { + int local = 5; + int *a = &local; // expected-note {{'a' initialized here}} + int *b = a; + int *c = b; + ptr = c; + } + *ptr = 6; + // expected-warning@-1 {{Use of 'local' after its lifetime ended}} + // expected-note@-2 {{Use of 'local' after its lifetime ended}} + // expected-note@-9 {{'local' initialized to 5}} + // expected-note@-8 {{'b' initialized to the value of 'a'}} + // expected-note@-8 {{'c' initialized to the value of 'b'}} + // expected-note@-8 {{The value of 'c' is assigned to 'ptr'}} + // expected-note@-8 {{'local' is destroyed here}} +} diff --git a/clang/test/Analysis/lifetime-bound.cpp b/clang/test/Analysis/lifetime-bound.cpp index 4920a3c928fb8..170f8e989e054 100644 --- a/clang/test/Analysis/lifetime-bound.cpp +++ b/clang/test/Analysis/lifetime-bound.cpp @@ -1,10 +1,34 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UseAfterLifetimeEnd,debug.DebugLifetimeModeling \ -// RUN: -analyzer-config cfg-lifetime=true -verify %s +// RUN: -analyzer-config cfg-lifetime=true -analyzer-output=text -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UseAfterLifetimeEnd,debug.DebugLifetimeModeling \ -// RUN: -analyzer-config c++-container-inlining=false -analyzer-config cfg-lifetime=true -verify %s - +// RUN: -analyzer-config c++-container-inlining=false -analyzer-config cfg-lifetime=true -analyzer-output=text -verify %s struct A {}; +struct Pair { + int a; + int b; +}; + +struct Base { + int x; +}; + +struct Derived : Base { + int y; +}; + +struct Inner { + int val; +}; + +struct Outer { + Inner inner; +}; + +struct Buffer { + int arr[4]; +}; + void clang_analyzer_dumpLifetimeOriginsOf(int*); void clang_analyzer_dumpLifetimeOriginsOf(int&); void clang_analyzer_dumpLifetimeOriginsOf(A*); @@ -21,7 +45,9 @@ void caller() { int v = 0; X obj; int &r = obj.choose(v); - clang_analyzer_dumpLifetimeOriginsOf(r); // expected-warning {{Origin &v bound to v}} + clang_analyzer_dumpLifetimeOriginsOf(r); + // expected-warning@-1 {{Origin &v bound to v}} + // expected-note@-2 {{Origin &v bound to v}} } // Obj ref type function return annotated case. @@ -34,7 +60,9 @@ void caller_two() { // Return statement is annotated case. Y y; A &f = y.getA(); - clang_analyzer_dumpLifetimeOriginsOf(f); // expected-warning {{Origin &y.a bound to y}} + clang_analyzer_dumpLifetimeOriginsOf(f); + // expected-warning@-1 {{Origin &y.a bound to y}} + // expected-note@-2 {{Origin &y.a bound to y}} } // Obj ptr type function return annotated case. @@ -46,7 +74,9 @@ struct Z { void caller_three() { Z z; A *func = z.getA(); - clang_analyzer_dumpLifetimeOriginsOf(func); // expected-warning {{Origin &z.a bound to z}} + clang_analyzer_dumpLifetimeOriginsOf(func); + // expected-warning@-1 {{Origin &z.a bound to z}} + // expected-note@-2 {{Origin &z.a bound to z}} } // Free function with annotated param and ref return. @@ -55,7 +85,9 @@ int &foo(int &num [[clang::lifetimebound]]) { return num; } void caller_four() { int num = 5; int &s = foo(num); - clang_analyzer_dumpLifetimeOriginsOf(s); // expected-warning {{Origin &num bound to num}} + clang_analyzer_dumpLifetimeOriginsOf(s); + // expected-warning@-1 {{Origin &num bound to num}} + // expected-note@-2 {{Origin &num bound to num}} } // Free function with annotated param and ptr return. @@ -66,7 +98,9 @@ void caller_five() { int *n_ptr = &n; int *s = boo(n_ptr); - clang_analyzer_dumpLifetimeOriginsOf(s); // expected-warning {{Origin &n bound to n}} + clang_analyzer_dumpLifetimeOriginsOf(s); + // expected-warning@-1 {{Origin &n bound to n}} + // expected-note@-2 {{Origin &n bound to n}} } // Free function with both annotated and non-annotated parameters. @@ -77,12 +111,12 @@ void caller_six() { int odd = 55; int &s = fn(even, odd); - clang_analyzer_dumpLifetimeOriginsOf(s); // expected-warning {{Origin &odd bound to odd}} + clang_analyzer_dumpLifetimeOriginsOf(s); + // expected-warning@-1 {{Origin &odd bound to odd}} + // expected-note@-2 {{Origin &odd bound to odd}} } - - -// These are the cases when the result of function calls are SymbolRefs. +// Test cases for testing when the result of function calls are SymbolRefs. // Function returns ptr and has an annotated parameter. int *foo(int *n [[clang::lifetimebound]]); @@ -92,7 +126,9 @@ void caller_seven() { int *y_ptr = &y; auto *bind = foo(y_ptr); - clang_analyzer_dumpLifetimeOriginsOf(bind); // expected-warning-re {{Origin &SymRegion{{.*}} bound to y}} + clang_analyzer_dumpLifetimeOriginsOf(bind); + // expected-warning-re@-1 {{Origin &SymRegion{{.*}} bound to y}} + // expected-note-re@-2 {{Origin &SymRegion{{.*}} bound to y}} } // Function returns a reference and has an annotated parameter. @@ -102,7 +138,9 @@ void caller_eight() { int f = 15; auto &bind = func(f); - clang_analyzer_dumpLifetimeOriginsOf(bind); // expected-warning-re {{Origin &SymRegion{{.*}} bound to f}} + clang_analyzer_dumpLifetimeOriginsOf(bind); + // expected-warning-re@-1 {{Origin &SymRegion{{.*}} bound to f}} + // expected-note-re@-2 {{Origin &SymRegion{{.*}} bound to f}} } // Function returns a reference and has two annotated parameters. @@ -113,7 +151,9 @@ void caller_nine() { int second_num = 2; int &numbers = f(first_num, second_num); - clang_analyzer_dumpLifetimeOriginsOf(numbers); // expected-warning-re {{Origin &SymRegion{{.*}} bound to first_num, second_num}} + clang_analyzer_dumpLifetimeOriginsOf(numbers); + // expected-warning-re@-1 {{Origin &SymRegion{{.*}} bound to first_num, second_num}} + // expected-note-re@-2 {{Origin &SymRegion{{.*}} bound to first_num, second_num}} } struct View { @@ -139,16 +179,19 @@ int *test_func(int *p [[clang::lifetimebound]]); int *direct_return() { - int i = 5; + int i = 5; //expected-note {{'i' initialized here}} return test_func(&i); // expected-warning@-1 {{Returning value bound to 'i' that will go out of scope}} // expected-warning@-2 {{address of stack memory associated with local variable 'i' returned}} + // expected-note@-3 {{Returning value bound to 'i' that will go out of scope}} } int *variable_return() { - int y = 5; + int y = 5; // expected-note {{'y' initialized here}} int *p = test_func(&y); - return p; // expected-warning {{Returning value bound to 'y' that will go out of scope}} + return p; + // expected-warning@-1 {{Returning value bound to 'y' that will go out of scope}} + // expected-note@-2 {{Returning value bound to 'y' that will go out of scope}} } int *borrow_from_caller(int *b [[clang::lifetimebound]]) { @@ -173,11 +216,15 @@ int &multi_param_test_ref(int &a [[clang::lifetimebound]], int &b [[clang::lifet // Return value bound to annotated parameters (two dangling sources). int &dangling_sources_ref() { int x = 1, y = 2; + // expected-note@-1 {{'x' initialized here}} + // expected-note@-2 {{'y' initialized here}} return multi_param_test_ref(x, y); // expected-warning@-1 {{Returning value bound to 'x' that will go out of scope}} - // expected-warning@-2 {{Returning value bound to 'y' that will go out of scope}} - // expected-warning@-3 {{reference to stack memory associated ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/212254 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
