diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index cdde849b0e026..f8407adcf657e 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -1666,6 +1666,12 @@ std::unique_ptr CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) { assert(Succ == &cfg->getExit()); Block = nullptr; // the EXIT block is empty. Create all other blocks lazily. + // Add parameters to the initial scope to handle their dtos and lifetime ends. + LocalScope *paramScope = nullptr; + if (const auto *FD = dyn_cast_or_null(D)) + for (ParmVarDecl *PD : FD->parameters()) + paramScope = addLocalScopeForVarDecl(PD, paramScope); + if (BuildOpts.AddImplicitDtors) if (const CXXDestructorDecl *DD = dyn_cast_or_null(D)) addImplicitDtorsForDestructor(DD); @@ -2246,6 +2252,11 @@ LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD, if (!VD->hasLocalStorage()) return Scope; + // Reference parameters are aliases to objects that live elsewhere, so they + // don't require automatic destruction or lifetime tracking. + if (isa(VD) && VD->getType()->isReferenceType()) + return Scope; + if (!BuildOpts.AddLifetime && !BuildOpts.AddScopes && !needsAutomaticDestruction(VD)) { assert(BuildOpts.AddImplicitDtors); @@ -5616,8 +5627,15 @@ class StmtPrinterHelper : public PrinterHelper { bool handleDecl(const Decl *D, raw_ostream &OS) { DeclMapTy::iterator I = DeclMap.find(D); - if (I == DeclMap.end()) + if (I == DeclMap.end()) { + // ParmVarDecls are not declared in the CFG itself, so they do not appear + // in DeclMap. + if (auto *PVD = dyn_cast_or_null(D)) { + OS << "[Parm: " << PVD->getNameAsString() << "]"; + return true; + } return false; + } if (currentBlock >= 0 && I->second.first == (unsigned) currentBlock && I->second.second == currStmt) { diff --git a/clang/test/Analysis/lifetime-cfg-output.cpp b/clang/test/Analysis/lifetime-cfg-output.cpp index 0a75c5bcc0bcc..36b36eddc440c 100644 --- a/clang/test/Analysis/lifetime-cfg-output.cpp +++ b/clang/test/Analysis/lifetime-cfg-output.cpp @@ -935,3 +935,31 @@ int backpatched_goto() { goto label; i++; } + +// CHECK: [B2 (ENTRY)] +// CHECK-NEXT: Succs (1): B1 +// CHECK: [B1] +// CHECK-NEXT: 1: a +// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, LValueToRValue, int) +// CHECK-NEXT: 3: b +// CHECK-NEXT: 4: [B1.3] (ImplicitCastExpr, LValueToRValue, int) +// CHECK-NEXT: 5: [B1.2] + [B1.4] +// CHECK-NEXT: 6: c +// CHECK-NEXT: 7: [B1.6] (ImplicitCastExpr, LValueToRValue, int) +// CHECK-NEXT: 8: [B1.5] + [B1.7] +// CHECK-NEXT: 9: int res = a + b + c; +// CHECK-NEXT: 10: res +// CHECK-NEXT: 11: [B1.10] (ImplicitCastExpr, LValueToRValue, int) +// CHECK-NEXT: 12: return [B1.11]; +// CHECK-NEXT: 13: [B1.9] (Lifetime ends) +// CHECK-NEXT: 14: [Parm: c] (Lifetime ends) +// CHECK-NEXT: 15: [Parm: b] (Lifetime ends) +// CHECK-NEXT: 16: [Parm: a] (Lifetime ends) +// CHECK-NEXT: Preds (1): B2 +// CHECK-NEXT: Succs (1): B0 +// CHECK: [B0 (EXIT)] +// CHECK-NEXT: Preds (1): B1 +int test_param_scope_end_order(int a, int b, int c) { + int res = a + b + c; + return res; +} diff --git a/clang/test/Analysis/scopes-cfg-output.cpp b/clang/test/Analysis/scopes-cfg-output.cpp index 6ed6f3638f75b..9c75492c33a42 100644 --- a/clang/test/Analysis/scopes-cfg-output.cpp +++ b/clang/test/Analysis/scopes-cfg-output.cpp @@ -1437,12 +1437,14 @@ void test_cleanup_functions() { // CHECK-NEXT: 4: return; // CHECK-NEXT: 5: CleanupFunction (cleanup_int) // CHECK-NEXT: 6: CFGScopeEnd(i) +// CHECK-NEXT: 7: CFGScopeEnd(m) // CHECK-NEXT: Preds (1): B3 // CHECK-NEXT: Succs (1): B0 // CHECK: [B2] // CHECK-NEXT: 1: return; // CHECK-NEXT: 2: CleanupFunction (cleanup_int) // CHECK-NEXT: 3: CFGScopeEnd(i) +// CHECK-NEXT: 4: CFGScopeEnd(m) // CHECK-NEXT: Preds (1): B3 // CHECK-NEXT: Succs (1): B0 // CHECK: [B3] diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp index e80a05860389c..1191469e23df1 100644 --- a/clang/test/Sema/warn-lifetime-safety.cpp +++ b/clang/test/Sema/warn-lifetime-safety.cpp @@ -529,14 +529,14 @@ TriviallyDestructedClass* trivial_class_uar () { return ptr; // expected-note {{returned here}} } -// FIXME: No lifetime warning for this as no expire facts are generated for parameters const int& return_parameter(int a) { - return a; + return a; // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} } -// FIXME: No lifetime warning for this as no expire facts are generated for parameters int* return_pointer_to_parameter(int a) { - return &a; + return &a; // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} } const int& return_reference_to_parameter(int a) @@ -788,9 +788,52 @@ const MyObj& lifetimebound_return_ref_to_local() { // expected-note@-1 {{returned here}} } -// FIXME: Fails to diagnose UAR when a reference to a by-value param escapes via the return value. -View lifetimebound_return_of_by_value_param(MyObj stack_param) { - return Identity(stack_param); +View lifetimebound_return_by_value_param(MyObj stack_param) { + return Identity(stack_param); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +View lifetimebound_return_by_value_multiple_param(int cond, MyObj a, MyObj b, MyObj c) { + if (cond == 1) + return Identity(a); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} + if (cond == 2) + return Identity(b); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} + return Identity(c); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} + +template +View lifetimebound_return_by_value_param_template(T t) { + return Identity(t); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} +} +void use_lifetimebound_return_by_value_param_template() { + lifetimebound_return_by_value_param_template(MyObj{}); // expected-note {{in instantiation of}} +} + +void lambda_uar_param() { + auto lambda = [](MyObj stack_param) { + return Identity(stack_param); // expected-warning {{address of stack memory is returned later}} + // expected-note@-1 {{returned here}} + }; + lambda(MyObj{}); +} + +// FIXME: This should be detected. We see correct destructors but origin flow breaks somewhere. +namespace VariadicTemplatedParamsUAR{ + +template +View Max(Args... args [[clang::lifetimebound]]); + +template +View lifetimebound_return_of_variadic_param(Args... args) { + return Max(args...); +} +void test_variadic() { + lifetimebound_return_of_variadic_param(MyObj{1}, MyObj{2}, MyObj{3}); +} } // FIXME: Fails to diagnose UAF when a reference to a by-value param escapes via an out-param. diff --git a/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp b/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp index 88630119ba8a1..609255437fe82 100644 --- a/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp @@ -149,9 +149,18 @@ recordState(Elements=8, Branches=2, Joins=1) enterElement(return b ? p : q;) transfer() recordState(Elements=9, Branches=2, Joins=1) +enterElement([Parm: q] (Lifetime ends)) +transfer() +recordState(Elements=10, Branches=2, Joins=1) +enterElement([Parm: p] (Lifetime ends)) +transfer() +recordState(Elements=11, Branches=2, Joins=1) +enterElement([Parm: b] (Lifetime ends)) +transfer() +recordState(Elements=12, Branches=2, Joins=1) enterBlock(0, false) -recordState(Elements=9, Branches=2, Joins=1) +recordState(Elements=12, Branches=2, Joins=1) endAnalysis() )");