diff --git a/clang/lib/Analysis/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety.cpp index c2e6dd74d0758..3c9ac9c48b2e5 100644 --- a/clang/lib/Analysis/LifetimeSafety.cpp +++ b/clang/lib/Analysis/LifetimeSafety.cpp @@ -445,7 +445,6 @@ class FactGenerator : public ConstStmtVisitor { void VisitImplicitCastExpr(const ImplicitCastExpr *ICE) { if (!hasOrigin(ICE->getType())) return; - Visit(ICE->getSubExpr()); // An ImplicitCastExpr node itself gets an origin, which flows from the // origin of its sub-expression (after stripping its own parens/casts). // TODO: Consider if this is actually useful in practice. Alternatively, we diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 0b94b1044f072..1b66d83df5171 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -2905,6 +2905,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( AC.getCFGBuildOptions().AddCXXNewAllocator = false; AC.getCFGBuildOptions().AddCXXDefaultInitExprInCtors = true; + bool EnableLifetimeSafetyAnalysis = S.getLangOpts().EnableLifetimeSafety; + // Force that certain expressions appear as CFGElements in the CFG. This // is used to speed up various analyses. // FIXME: This isn't the right factoring. This is here for initial @@ -2912,11 +2914,10 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( // expect to always be CFGElements and then fill in the BuildOptions // appropriately. This is essentially a layering violation. if (P.enableCheckUnreachable || P.enableThreadSafetyAnalysis || - P.enableConsumedAnalysis) { + P.enableConsumedAnalysis || EnableLifetimeSafetyAnalysis) { // Unreachable code analysis and thread safety require a linearized CFG. AC.getCFGBuildOptions().setAllAlwaysAdd(); - } - else { + } else { AC.getCFGBuildOptions() .setAlwaysAdd(Stmt::BinaryOperatorClass) .setAlwaysAdd(Stmt::CompoundAssignOperatorClass) @@ -2927,7 +2928,6 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings( .setAlwaysAdd(Stmt::UnaryOperatorClass); } - bool EnableLifetimeSafetyAnalysis = S.getLangOpts().EnableLifetimeSafety; // Install the logical handler. std::optional LEH; if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) { diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp index bcde9adf25ca5..bc7e0127e5d4c 100644 --- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp +++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp @@ -293,3 +293,22 @@ void pointer_indirection() { int *q = *pp; // CHECK: AssignOrigin (Dest: {{[0-9]+}} (Decl: q), Src: {{[0-9]+}} (Expr: ImplicitCastExpr)) } + +// CHECK-LABEL: Function: ternary_operator +// FIXME: Propagate origins across ConditionalOperator. +void ternary_operator() { + int a, b; + int *p; + p = (a > b) ? &a : &b; + // CHECK: Block B2: + // CHECK: Issue (LoanID: [[L_A:[0-9]+]], ToOrigin: [[O_ADDR_A:[0-9]+]] (Expr: UnaryOperator)) + // CHECK: End of Block + + // CHECK: Block B3: + // CHECK: Issue (LoanID: [[L_B:[0-9]+]], ToOrigin: [[O_ADDR_A:[0-9]+]] (Expr: UnaryOperator)) + // CHECK: End of Block + + // CHECK: Block B1: + // CHECK: AssignOrigin (Dest: [[O_P:[0-9]+]] (Decl: p), Src: {{[0-9]+}} (Expr: ConditionalOperator)) + // CHECK: End of Block +} diff --git a/clang/unittests/Analysis/LifetimeSafetyTest.cpp b/clang/unittests/Analysis/LifetimeSafetyTest.cpp index c8d88b4ea2277..13e5832d70050 100644 --- a/clang/unittests/Analysis/LifetimeSafetyTest.cpp +++ b/clang/unittests/Analysis/LifetimeSafetyTest.cpp @@ -11,6 +11,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Testing/TestAST.h" #include "llvm/ADT/StringMap.h" +#include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include @@ -20,6 +21,7 @@ namespace clang::lifetimes::internal { namespace { using namespace ast_matchers; +using ::testing::SizeIs; using ::testing::UnorderedElementsAreArray; // A helper class to run the full lifetime analysis on a piece of code @@ -96,21 +98,18 @@ class LifetimeTestHelper { return OID; } - std::optional getLoanForVar(llvm::StringRef VarName) { + std::vector getLoansForVar(llvm::StringRef VarName) { auto *VD = findDecl(VarName); - if (!VD) - return std::nullopt; + if (!VD) { + ADD_FAILURE() << "No VarDecl found for '" << VarName << "'"; + return {}; + } std::vector LID = Analysis.getLoanIDForVar(VD); if (LID.empty()) { ADD_FAILURE() << "Loan for '" << VarName << "' not found."; - return std::nullopt; - } - // TODO: Support retrieving more than one loans to a var. - if (LID.size() > 1) { - ADD_FAILURE() << "More than 1 loans found for '" << VarName; - return std::nullopt; + return {}; } - return LID[0]; + return LID; } std::optional getLoansAtPoint(OriginID OID, @@ -121,13 +120,12 @@ class LifetimeTestHelper { return Analysis.getLoansAtPoint(OID, PP); } - std::optional> + std::optional> getExpiredLoansAtPoint(llvm::StringRef Annotation) { ProgramPoint PP = Runner.getProgramPoint(Annotation); if (!PP) return std::nullopt; - auto Expired = Analysis.getExpiredLoansAtPoint(PP); - return llvm::DenseSet{Expired.begin(), Expired.end()}; + return Analysis.getExpiredLoansAtPoint(PP); } private: @@ -197,12 +195,13 @@ MATCHER_P2(HasLoansToImpl, LoanVars, Annotation, "") { std::vector ExpectedLoans; for (const auto &LoanVar : LoanVars) { - std::optional ExpectedLIDOpt = Info.Helper.getLoanForVar(LoanVar); - if (!ExpectedLIDOpt) { + std::vector ExpectedLIDs = Info.Helper.getLoansForVar(LoanVar); + if (ExpectedLIDs.empty()) { *result_listener << "could not find loan for var '" << LoanVar << "'"; return false; } - ExpectedLoans.push_back(*ExpectedLIDOpt); + ExpectedLoans.insert(ExpectedLoans.end(), ExpectedLIDs.begin(), + ExpectedLIDs.end()); } return ExplainMatchResult(UnorderedElementsAreArray(ExpectedLoans), @@ -221,17 +220,17 @@ MATCHER_P(AreExpiredAt, Annotation, "") { << Annotation << "'"; return false; } - std::vector ActualExpiredLoans(ActualExpiredSetOpt->begin(), - ActualExpiredSetOpt->end()); + std::vector ActualExpiredLoans = *ActualExpiredSetOpt; std::vector ExpectedExpiredLoans; for (const auto &VarName : Info.LoanVars) { - auto LoanIDOpt = Helper.getLoanForVar(VarName); - if (!LoanIDOpt) { + auto LoanIDs = Helper.getLoansForVar(VarName); + if (LoanIDs.empty()) { *result_listener << "could not find a loan for variable '" << VarName << "'"; return false; } - ExpectedExpiredLoans.push_back(*LoanIDOpt); + ExpectedExpiredLoans.insert(ExpectedExpiredLoans.end(), LoanIDs.begin(), + LoanIDs.end()); } return ExplainMatchResult(UnorderedElementsAreArray(ExpectedExpiredLoans), ActualExpiredLoans, result_listener); @@ -730,5 +729,17 @@ TEST_F(LifetimeAnalysisTest, ReassignedPointerThenOriginalExpires) { EXPECT_THAT(LoansTo({"s1", "s2"}), AreExpiredAt("p_after_s1_expires")); } +TEST_F(LifetimeAnalysisTest, NoDuplicateLoansForImplicitCastToConst) { + SetupTest(R"( + void target() { + MyObj a; + const MyObj* p = &a; + const MyObj* q = &a; + POINT(at_end); + } + )"); + EXPECT_THAT(Helper->getLoansForVar("a"), SizeIs(2)); +} + } // anonymous namespace } // namespace clang::lifetimes::internal