Author: Yitzhak Mandelbaum Date: 2022-12-22T14:42:17Z New Revision: 38404df9d879483784a7024b2b4a366388d6d476
URL: https://github.com/llvm/llvm-project/commit/38404df9d879483784a7024b2b4a366388d6d476 DIFF: https://github.com/llvm/llvm-project/commit/38404df9d879483784a7024b2b4a366388d6d476.diff LOG: [clang][dataflow] Fix bug in handling of `return` statements. The handling of return statements, added in support of context-sensitive analysis, has a bug relating to functions that return reference types. Specifically, interpretation of such functions can result in a crash from a bad cast. This patch fixes the bug and guards all of that code with the context-sensitive option, since there's no reason to execute at all when context-sensitive analysis is off. Differential Revision: https://reviews.llvm.org/D140430 Added: Modified: clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 43d004697799..336de81b0653 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -429,6 +429,9 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { } void VisitReturnStmt(const ReturnStmt *S) { + if (!Options.ContextSensitiveOpts) + return; + auto *Ret = S->getRetValue(); if (Ret == nullptr) return; @@ -443,6 +446,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { auto *Loc = Env.getReturnStorageLocation(); assert(Loc != nullptr); + // FIXME: Support reference-type returns. + if (Loc->getType()->isReferenceType()) + return; + // FIXME: Model NRVO. Env.setValue(*Loc, *Val); } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 299837aee928..b5e10b24fffb 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -4018,6 +4018,35 @@ TEST(TransferTest, ContextSensitiveOptionDisabled) { {TransferOptions{/*.ContextSensitiveOpts=*/std::nullopt}}); } +// This test is a regression test, based on a real crash. +TEST(TransferTest, ContextSensitiveReturnReferenceFromNonReferenceLvalue) { + // This code exercises an unusual code path. If we return an lvalue directly, + // the code will catch that it's an l-value based on the `Value`'s kind. If we + // pass through a dummy function, the framework won't populate a value at + // all. In contrast, this code results in a (fresh) value, but it is not + // `ReferenceValue`. This test verifies that we catch this case as well. + std::string Code = R"( + class S {}; + S& target(bool b, S &s) { + return b ? s : s; + // [[p]] + } + )"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto *Loc = Env.getReturnStorageLocation(); + ASSERT_THAT(Loc, NotNull()); + + EXPECT_THAT(Env.getValue(*Loc), IsNull()); + }, + {TransferOptions{ContextSensitiveOptions{}}}); +} + TEST(TransferTest, ContextSensitiveDepthZero) { std::string Code = R"( bool GiveBool(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
