llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-static-analyzer-1 Author: Benedek Kaibas (benedekaibas) <details> <summary>Changes</summary> Improve dangling value tracking in the `DanglingPtrDeref` checker by adding `trackExpressionValue`. The report with this change now tracks the dangling value and shows where the value originated from. Currently the checker only points at the destruction and use sites which does not explain the full picture for the user. --- Full diff: https://github.com/llvm/llvm-project/pull/211818.diff 3 Files Affected: - (modified) clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp (+17-9) - (modified) clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp (+1-1) - (modified) clang/test/Analysis/dangling-ptr-deref.cpp (+111-9) ``````````diff diff --git a/clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp b/clang/lib/StaticAnalyzer/Checkers/DanglingPtrDeref.cpp index ff2087e1db933..16550bb7007e1 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,19 +62,28 @@ 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; + return "the 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 '") + Region->getString() + - "' after its lifetime ended."), + (llvm::Twine("Use of ") + getRegionName(Region) + + " after its lifetime ended."), N); BR->addVisitor<DanglingPtrDerefBRVisitor>(Region); + bugreporter::trackExpressionValue(N, bugreporter::getDerefExpr(S), *BR); C.emitReport(std::move(BR)); } @@ -99,8 +108,7 @@ DanglingPtrDerefBRVisitor::VisitNode(const ExplodedNode *N, S, BRC.getSourceManager(), N->getStackFrame()); return std::make_shared<PathDiagnosticEventPiece>( Pos, - (llvm::Twine("'") + SourceRegion->getString() + "' is destroyed here") - .str(), + (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 2b6f5dae4243f..ef0b1cb264b18 100644 --- a/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/LifetimeModeling.cpp @@ -73,7 +73,7 @@ std::vector<const MemRegion *> lifetime_modeling::getDanglingRegionsAfterReturn( bool lifetime_modeling::isDeallocated(ProgramStateRef State, const MemRegion *Region) { - return State->contains<DeallocatedSourceSet>(Region); + return State->contains<DeallocatedSourceSet>(Region->getBaseRegion()); } static ProgramStateRef bindSource(ProgramStateRef State, SVal RetVal, diff --git a/clang/test/Analysis/dangling-ptr-deref.cpp b/clang/test/Analysis/dangling-ptr-deref.cpp index e661eff9ccbb6..890ab279a54aa 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}} @@ -112,3 +112,105 @@ void inlined_callee_single_report() { // expected-note@-1 {{Calling 'deref_param'}} (void)r; } + +struct MyBuffer { + char buffer[8]; +}; +struct MyStruct { int x; }; +struct Inner { int x; }; +struct Outer { struct Inner inner; }; + +char member_subregion_dangling_deref() { + const char *p = nullptr; + { + struct MyBuffer tmp_buffer = {}; + p = tmp_buffer.buffer; + } + // expected-note@-1 {{'tmp_buffer.buffer[0]' is destroyed here}} + return *p; + // expected-warning@-1 {{Use of 'tmp_buffer.buffer[0]' after its lifetime ended}} + // expected-note@-2 {{Use of 'tmp_buffer.buffer[0]' after its lifetime ended}} +} + +void opaque(const char *); + +void passing_dangling_to_call() { + const char *p = nullptr; + { + struct MyBuffer tmp_buffer = {}; + p = tmp_buffer.buffer; + } + // expected-note@-1 {{'tmp_buffer.buffer[0]' is destroyed here}} + opaque(p); + // expected-warning@-1 {{Use of 'tmp_buffer.buffer[0]' after its lifetime ended}} + // expected-note@-2 {{Use of 'tmp_buffer.buffer[0]' after its lifetime ended}} +} + +char member_subregion_alive_deref() { + { + struct MyBuffer tmp_buffer = {}; + const char *p = tmp_buffer.buffer; + opaque(p); // no-warning + return *p; // no-warning + } +} + +void arr_elem_subreg_dangling_deref() { + int *ptr = nullptr; + { + int local_arr[5]; + ptr = &local_arr[1]; + } + // expected-note@-1 {{'local_arr[1]' is destroyed here}} + *ptr = 7; + // expected-warning@-1 {{Use of 'local_arr[1]' after its lifetime ended}} + // expected-note@-2 {{Use of 'local_arr[1]' after its lifetime ended}} +} + +char member_array_elem__dangling_deref() { + const char *p = nullptr; + { + struct MyBuffer tmp_buffer = {}; + p = tmp_buffer.buffer + 3; + } + // expected-note@-1 {{'tmp_buffer.buffer[3]' is destroyed here}} + return *p; + // expected-warning@-1 {{Use of 'tmp_buffer.buffer[3]' after its lifetime ended}} + // expected-note@-2 {{Use of 'tmp_buffer.buffer[3]' after its lifetime ended}} +} + +int struct_field_dangling_deref() { + int *p = nullptr; + { + struct MyStruct s = {}; + p = &s.x; + } + // expected-note@-1 {{'s.x' is destroyed here}} + return *p; + // expected-warning@-1 {{Use of 's.x' after its lifetime ended}} + // expected-note@-2 {{Use of 's.x' after its lifetime ended}} +} + +int struct_array_element_dangling_deref() { + int *p = nullptr; + { + struct MyStruct arr[4] = {}; + p = &arr[2].x; + } + // expected-note@-1 {{'arr[2].x' is destroyed here}} + return *p; + // expected-warning@-1 {{Use of 'arr[2].x' after its lifetime ended}} + // expected-note@-2 {{Use of 'arr[2].x' after its lifetime ended}} +} + +int nested_field_dangling_deref() { + int *p = nullptr; + { + struct Outer o = {}; + p = &o.inner.x; + } + // expected-note@-1 {{'o.inner.x' is destroyed here}} + return *p; + // expected-warning@-1 {{Use of 'o.inner.x' after its lifetime ended}} + // expected-note@-2 {{Use of 'o.inner.x' after its lifetime ended}} +} `````````` </details> https://github.com/llvm/llvm-project/pull/211818 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
