llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 @llvm/pr-subscribers-clang Author: Benedek Kaibas (benedekaibas) <details> <summary>Changes</summary> Currently the `UseAfterLifetimeEnd` checker used `getString()` for constructing error message. However, `getString()` is a debug only stringification and should not be used for emitting reports to the users. That is why I have changed it to `getDescriptiveName()` and also implemented the `getRegionName`(#<!-- -->211552) function to return the region's descriptive name. The `getRegionName()` function got also moved to the modeling checker since both of the reporting checkers consume it (#<!-- -->211818). This PR also uses the `trackStoredValue()` for value tracking path notes, so the report points at where the value's source came from. --- Patch is 23.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/212158.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 (+25-24) - (modified) clang/test/Analysis/lifetime-bound.cpp (+63-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..f210195762770 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,4 @@ 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}} } + diff --git a/clang/test/Analysis/lifetime-bound.cpp b/clang/test/Analysis/lifetime-bound.cpp index 4920a3c928fb8..f9c3ddf524ac0 100644 --- a/clang/test/Analysis/lifetime-bound.cpp +++ b/clang/test/Analysis/lifetime-bound.cpp @@ -1,8 +1,7 @@ // 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 {}; void clang_analyzer_dumpLifetimeOriginsOf(int*); @@ -21,7 +20,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 +35,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 +49,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 +60,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 +73,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 +86,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 +101,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 +113,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 +126,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 +154,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 +191,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 with local variable 'x' returned}} - // expected-warning@-4 {{reference to stack memory associated with local variable 'y' returned}} + // expected-note@-2 {{Returning value bound to 'x' that will go out of scope}} + // expected-warning@-3 {{Returning value bound to 'y' that will go out of scope}} + // expected-note@-4 {{Returning value bound to 'y' that will go out of scope}} + // expected-warning@-5 {{reference to stack memory associated with local variable 'x' returned}} + // expected-warning@-6 {{reference to stack memory associated with local variable 'y' returned}} } // Return value bound to annotated parameters (no dangling sources). @@ -187,10 +209,11 @@ int &no_dangling_sources_ref(int &a [[clang::lifetimebound]], int &b [[clang::li // Return value bound to annotated parameters (one dangling source). int &one_dangling_source_ref(int &a [[clang::lifetimebound]]) { - int x = 1; + int x = 1; // expected-note {{'x' initialized here}... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/212158 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
