https://github.com/bazuzi updated
https://github.com/llvm/llvm-project/pull/157535
>From 4779770aab880315d359005a5cacd4c4976d8649 Mon Sep 17 00:00:00 2001
From: Samira Bakon <baz...@google.com>
Date: Mon, 8 Sep 2025 15:08:52 -0400
Subject: [PATCH 1/3] Revert "Revert "[clang][dataflow] Transfer more cast
expressions." (#157148)"
This reverts commit 4c29a600fa34d0c1cabf4ffcf081f2a00b09fddd.
---
.../Analysis/FlowSensitive/StorageLocation.h | 11 +
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 71 ++++-
.../Analysis/FlowSensitive/TransferTest.cpp | 280 +++++++++++++++++-
3 files changed, 349 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
index 8fcc6a44027a0..534b9a017d8f0 100644
--- a/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
+++ b/clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
@@ -17,6 +17,7 @@
#include "clang/AST/Decl.h"
#include "clang/AST/Type.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Debug.h"
#include <cassert>
@@ -152,6 +153,11 @@ class RecordStorageLocation final : public StorageLocation
{
return {SyntheticFields.begin(), SyntheticFields.end()};
}
+ /// Add a synthetic field, if none by that name is already present.
+ void addSyntheticField(llvm::StringRef Name, StorageLocation &Loc) {
+ SyntheticFields.insert({Name, &Loc});
+ }
+
/// Changes the child storage location for a field `D` of reference type.
/// All other fields cannot change their storage location and always retain
/// the storage location passed to the `RecordStorageLocation` constructor.
@@ -164,6 +170,11 @@ class RecordStorageLocation final : public StorageLocation
{
Children[&D] = Loc;
}
+ /// Add a child storage location for a field `D`, if not already present.
+ void addChild(const ValueDecl &D, StorageLocation *Loc) {
+ Children.insert({&D, Loc});
+ }
+
llvm::iterator_range<FieldToLoc::const_iterator> children() const {
return {Children.begin(), Children.end()};
}
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 86a816e2e406c..23a6de45e18b1 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -20,14 +20,17 @@
#include "clang/AST/OperationKinds.h"
#include "clang/AST/Stmt.h"
#include "clang/AST/StmtVisitor.h"
+#include "clang/AST/Type.h"
#include "clang/Analysis/FlowSensitive/ASTOps.h"
#include "clang/Analysis/FlowSensitive/AdornedCFG.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
#include "clang/Analysis/FlowSensitive/RecordOps.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/Builtins.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Basic/OperatorKinds.h"
#include "llvm/Support/Casting.h"
#include <assert.h>
@@ -287,7 +290,7 @@ class TransferVisitor : public
ConstStmtVisitor<TransferVisitor> {
}
}
- void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
+ void VisitCastExpr(const CastExpr *S) {
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
@@ -317,6 +320,60 @@ class TransferVisitor : public
ConstStmtVisitor<TransferVisitor> {
break;
}
+ case CK_BaseToDerived: {
+ // This is a cast of (single-layer) pointer or reference to a record
type.
+ // We should now model the fields for the derived type.
+
+ // Get the RecordStorageLocation for the record object underneath.
+ RecordStorageLocation *Loc = nullptr;
+ if (S->getType()->isPointerType()) {
+ auto *PV = Env.get<PointerValue>(*SubExpr);
+ assert(PV != nullptr);
+ if (PV == nullptr)
+ break;
+ Loc = cast<RecordStorageLocation>(&PV->getPointeeLoc());
+ } else {
+ assert(S->getType()->isRecordType());
+ if (SubExpr->isGLValue()) {
+ Loc = Env.get<RecordStorageLocation>(*SubExpr);
+ } else {
+ Loc = &Env.getResultObjectLocation(*SubExpr);
+ }
+ }
+ if (!Loc) {
+ // Nowhere to add children or propagate from, so we're done.
+ break;
+ }
+
+ // Get the derived record type underneath the reference or pointer.
+ QualType Derived = S->getType().getNonReferenceType();
+ if (Derived->isPointerType()) {
+ Derived = Derived->getPointeeType();
+ }
+
+ // Add children to the storage location for fields (including synthetic
+ // fields) of the derived type and initialize their values.
+ for (const FieldDecl *Field :
+ Env.getDataflowAnalysisContext().getModeledFields(Derived)) {
+ assert(Field != nullptr);
+ QualType FieldType = Field->getType();
+ if (FieldType->isReferenceType()) {
+ Loc->addChild(*Field, nullptr);
+ } else {
+ Loc->addChild(*Field, &Env.createStorageLocation(FieldType));
+ }
+
+ for (const auto &Entry :
+ Env.getDataflowAnalysisContext().getSyntheticFields(Derived)) {
+ Loc->addSyntheticField(Entry.getKey(),
+ Env.createStorageLocation(Entry.getValue()));
+ }
+ }
+ Env.initializeFieldsWithValues(*Loc, Derived);
+
+ // Fall through to propagate SubExpr's StorageLocation to the CastExpr.
+ [[fallthrough]];
+ }
case CK_IntegralCast:
// FIXME: This cast creates a new integral value from the
// subexpression. But, because we don't model integers, we don't
@@ -324,10 +381,9 @@ class TransferVisitor : public
ConstStmtVisitor<TransferVisitor> {
// modeling is added, then update this code to create a fresh location
and
// value.
case CK_UncheckedDerivedToBase:
+ case CK_DerivedToBase:
case CK_ConstructorConversion:
case CK_UserDefinedConversion:
- // FIXME: Add tests that excercise CK_UncheckedDerivedToBase,
- // CK_ConstructorConversion, and CK_UserDefinedConversion.
case CK_NoOp: {
// FIXME: Consider making `Environment::getStorageLocation` skip noop
// expressions (this and other similar expressions in the file) instead
@@ -684,15 +740,6 @@ class TransferVisitor : public
ConstStmtVisitor<TransferVisitor> {
propagateValue(*SubExpr, *S, Env);
}
- void VisitCXXStaticCastExpr(const CXXStaticCastExpr *S) {
- if (S->getCastKind() == CK_NoOp) {
- const Expr *SubExpr = S->getSubExpr();
- assert(SubExpr != nullptr);
-
- propagateValueOrStorageLocation(*SubExpr, *S, Env);
- }
- }
-
void VisitConditionalOperator(const ConditionalOperator *S) {
const Environment *TrueEnv = StmtToEnv.getEnvironment(*S->getTrueExpr());
const Environment *FalseEnv = StmtToEnv.getEnvironment(*S->getFalseExpr());
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 214aaee9f97f6..96e759e73c154 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -9,17 +9,25 @@
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
#include "clang/Analysis/FlowSensitive/RecordOps.h"
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/LangStandard.h"
#include "clang/Testing/TestAST.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
#include "llvm/Testing/Support/Error.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -27,6 +35,7 @@
#include <string>
#include <string_view>
#include <utility>
+#include <vector>
namespace clang {
namespace dataflow {
@@ -3541,7 +3550,7 @@ TEST(TransferTest,
ResultObjectLocationDontVisitUnevaluatedContexts) {
testFunction(Code, "noexceptTarget");
}
-TEST(TransferTest, StaticCast) {
+TEST(TransferTest, StaticCastNoOp) {
std::string Code = R"(
void target(int Foo) {
int Bar = static_cast<int>(Foo);
@@ -3561,6 +3570,13 @@ TEST(TransferTest, StaticCast) {
const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
ASSERT_THAT(BarDecl, NotNull());
+ const auto *Cast = ast_matchers::selectFirst<CXXStaticCastExpr>(
+ "cast",
+ ast_matchers::match(ast_matchers::cxxStaticCastExpr().bind("cast"),
+ ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_NoOp);
+
const auto *FooVal = Env.getValue(*FooDecl);
const auto *BarVal = Env.getValue(*BarDecl);
EXPECT_TRUE(isa<IntegerValue>(FooVal));
@@ -3569,6 +3585,268 @@ TEST(TransferTest, StaticCast) {
});
}
+TEST(TransferTest, StaticCastBaseToDerived) {
+ std::string Code = R"cc(
+ struct Base {
+ char C;
+ };
+ struct Intermediate : public Base {
+ bool B;
+ };
+ struct Derived : public Intermediate {
+ int I;
+ };
+ Base& getBaseRef();
+ void target(Base* BPtr) {
+ Derived* DPtr = static_cast<Derived*>(BPtr);
+ DPtr->C;
+ DPtr->B;
+ DPtr->I;
+ Derived& DRef = static_cast<Derived&>(*BPtr);
+ DRef.C;
+ DRef.B;
+ DRef.I;
+ Derived& DRefFromFunc = static_cast<Derived&>(getBaseRef());
+ DRefFromFunc.C;
+ DRefFromFunc.B;
+ DRefFromFunc.I;
+ // [[p]]
+ }
+ )cc";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ const ValueDecl *BPtrDecl = findValueDecl(ASTCtx, "BPtr");
+ ASSERT_THAT(BPtrDecl, NotNull());
+
+ const ValueDecl *DPtrDecl = findValueDecl(ASTCtx, "DPtr");
+ ASSERT_THAT(DPtrDecl, NotNull());
+
+ const ValueDecl *DRefDecl = findValueDecl(ASTCtx, "DRef");
+ ASSERT_THAT(DRefDecl, NotNull());
+
+ const ValueDecl *DRefFromFuncDecl =
+ findValueDecl(ASTCtx, "DRefFromFunc");
+ ASSERT_THAT(DRefFromFuncDecl, NotNull());
+
+ const auto *Cast = ast_matchers::selectFirst<CXXStaticCastExpr>(
+ "cast",
+ ast_matchers::match(ast_matchers::cxxStaticCastExpr().bind("cast"),
+ ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_BaseToDerived);
+
+ EXPECT_EQ(Env.getValue(*BPtrDecl), Env.getValue(*DPtrDecl));
+ EXPECT_EQ(&Env.get<PointerValue>(*BPtrDecl)->getPointeeLoc(),
+ Env.getStorageLocation(*DRefDecl));
+ // For DRefFromFunc, not crashing when analyzing the field accesses is
+ // enough.
+ });
+}
+
+TEST(TransferTest, ExplicitDerivedToBaseCast) {
+ std::string Code = R"cc(
+ struct Base {};
+ struct Derived : public Base {};
+ void target(Derived D) {
+ (Base*)&D;
+ // [[p]]
+ }
+)cc";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *Cast = ast_matchers::selectFirst<ImplicitCastExpr>(
+ "cast", ast_matchers::match(
+ ast_matchers::implicitCastExpr().bind("cast"),
ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_DerivedToBase);
+
+ auto *AddressOf = ast_matchers::selectFirst<UnaryOperator>(
+ "addressof",
+
ast_matchers::match(ast_matchers::unaryOperator().bind("addressof"),
+ ASTCtx));
+ ASSERT_THAT(AddressOf, NotNull());
+ ASSERT_EQ(AddressOf->getOpcode(), UO_AddrOf);
+
+ EXPECT_EQ(Env.getValue(*Cast), Env.getValue(*AddressOf));
+ });
+}
+
+TEST(TransferTest, ConstructorConversion) {
+ std::string Code = R"cc(
+ struct Base {};
+ struct Derived : public Base {};
+ void target(Derived D) {
+ Base B = (Base)D;
+ // [[p]]
+ }
+)cc";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *Cast = ast_matchers::selectFirst<CStyleCastExpr>(
+ "cast", ast_matchers::match(
+ ast_matchers::cStyleCastExpr().bind("cast"), ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_ConstructorConversion);
+
+ auto &DLoc = getLocForDecl<StorageLocation>(ASTCtx, Env, "D");
+ auto &BLoc = getLocForDecl<StorageLocation>(ASTCtx, Env, "B");
+ EXPECT_NE(&BLoc, &DLoc);
+ });
+}
+
+TEST(TransferTest, UserDefinedConversion) {
+ std::string Code = R"cc(
+ struct To {};
+ struct From {
+ operator To();
+ };
+ void target(From F) {
+ To T = (To)F;
+ // [[p]]
+ }
+)cc";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *Cast = ast_matchers::selectFirst<ImplicitCastExpr>(
+ "cast", ast_matchers::match(
+ ast_matchers::implicitCastExpr().bind("cast"),
ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_UserDefinedConversion);
+
+ auto &FLoc = getLocForDecl<StorageLocation>(ASTCtx, Env, "F");
+ auto &TLoc = getLocForDecl<StorageLocation>(ASTCtx, Env, "T");
+ EXPECT_NE(&TLoc, &FLoc);
+ });
+}
+
+TEST(TransferTest, ImplicitUncheckedDerivedToBaseCast) {
+ std::string Code = R"cc(
+ struct Base {
+ void method();
+ };
+ struct Derived : public Base {};
+ void target(Derived D) {
+ D.method();
+ // [[p]]
+ }
+)cc";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *Cast = ast_matchers::selectFirst<ImplicitCastExpr>(
+ "cast", ast_matchers::match(
+ ast_matchers::implicitCastExpr().bind("cast"),
ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_UncheckedDerivedToBase);
+
+ auto &DLoc = getLocForDecl<StorageLocation>(ASTCtx, Env, "D");
+ EXPECT_EQ(Env.getStorageLocation(*Cast), &DLoc);
+ });
+}
+
+TEST(TransferTest, ImplicitDerivedToBaseCast) {
+ std::string Code = R"cc(
+ struct Base {};
+ struct Derived : public Base {};
+ void target() {
+ Base* B = new Derived();
+ // [[p]]
+ }
+)cc";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+ auto *Cast = ast_matchers::selectFirst<ImplicitCastExpr>(
+ "cast", ast_matchers::match(
+ ast_matchers::implicitCastExpr().bind("cast"),
ASTCtx));
+ ASSERT_THAT(Cast, NotNull());
+ ASSERT_EQ(Cast->getCastKind(), CK_DerivedToBase);
+
+ auto *New = ast_matchers::selectFirst<CXXNewExpr>(
+ "new", ast_matchers::match(ast_matchers::cxxNewExpr().bind("new"),
+ ASTCtx));
+ ASSERT_THAT(New, NotNull());
+
+ EXPECT_EQ(Env.getValue(*Cast), Env.getValue(*New));
+ });
+}
+
+TEST(TransferTest, ReinterpretCast) {
+ std::string Code = R"cc(
+ struct S {
+ int I;
+ };
+
+ void target(unsigned char* Bytes) {
+ S& SRef = reinterpret_cast<S&>(Bytes);
+ SRef.I;
+ S* SPtr = reinterpret_cast<S*>(Bytes);
+ SPtr->I;
+ // [[p]]
+ }
+ )cc";
+ runDataflow(Code, [](const
llvm::StringMap<DataflowAnalysisState<NoopLattice>>
+ &Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+ const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+ const ValueDecl *I = findValueDecl(ASTCtx, "I");
+ ASSERT_THAT(I, NotNull());
+
+ // No particular knowledge of I's value is modeled, but for both casts,
+ // the fields of S are modeled.
+
+ {
+ auto &Loc = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "SRef");
+ std::vector<const ValueDecl *> Children;
+ for (const auto &Entry : Loc.children()) {
+ Children.push_back(Entry.getFirst());
+ }
+
+ EXPECT_THAT(Children, UnorderedElementsAre(I));
+ }
+
+ {
+ auto &Loc = cast<RecordStorageLocation>(
+ getValueForDecl<PointerValue>(ASTCtx, Env, "SPtr").getPointeeLoc());
+ std::vector<const ValueDecl *> Children;
+ for (const auto &Entry : Loc.children()) {
+ Children.push_back(Entry.getFirst());
+ }
+
+ EXPECT_THAT(Children, UnorderedElementsAre(I));
+ }
+ });
+}
+
TEST(TransferTest, IntegralCast) {
std::string Code = R"(
void target(int Foo) {
>From 3941927cb9ad628d6d4d59e294557933c0e15039 Mon Sep 17 00:00:00 2001
From: Samira Bakon <baz...@google.com>
Date: Mon, 8 Sep 2025 15:32:20 -0400
Subject: [PATCH 2/3] Fix copyRecord usage for transfer of CXXConstructExpr for
base class initializers.
---
.../clang/Analysis/FlowSensitive/RecordOps.h | 6 +-
.../lib/Analysis/FlowSensitive/RecordOps.cpp | 31 ++++++--
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 10 ++-
.../Analysis/FlowSensitive/RecordOpsTest.cpp | 70 ++++++++++++++++++-
.../Analysis/FlowSensitive/TransferTest.cpp | 34 +++++++++
5 files changed, 142 insertions(+), 9 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index 8fad45fc11d81..91204c071af56 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
+#include "clang/AST/Type.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
@@ -36,8 +37,11 @@ namespace dataflow {
/// - The type of `Src` must be derived from `Dest`, or
/// - The type of `Dest` must be derived from `Src` (in this case, any
fields
/// that are only present in `Dest` are not overwritten).
+/// - The types of `Dest` and `Src` are both derived from a non-null
+/// `TypeToCopy` (in this case, only fields present in `TypeToCopy` are
+/// overwritten).
void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
- Environment &Env);
+ Environment &Env, QualType TypeToCopy = QualType());
/// Returns whether the records `Loc1` and `Loc2` are equal.
///
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index b8401230a83d4..71b2aa9ff2ec4 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -11,6 +11,9 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/FlowSensitive/RecordOps.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Type.h"
#define DEBUG_TYPE "dataflow"
@@ -49,25 +52,30 @@ static void copySyntheticField(QualType FieldType,
StorageLocation &SrcFieldLoc,
}
void copyRecord(RecordStorageLocation &Src, RecordStorageLocation &Dst,
- Environment &Env) {
+ Environment &Env, const QualType TypeToCopy) {
auto SrcType = Src.getType().getCanonicalType().getUnqualifiedType();
auto DstType = Dst.getType().getCanonicalType().getUnqualifiedType();
auto SrcDecl = SrcType->getAsCXXRecordDecl();
auto DstDecl = DstType->getAsCXXRecordDecl();
- [[maybe_unused]] bool compatibleTypes =
+ const CXXRecordDecl *DeclToCopy =
+ TypeToCopy.isNull() ? nullptr : TypeToCopy->getAsCXXRecordDecl();
+
+ [[maybe_unused]] bool CompatibleTypes =
SrcType == DstType ||
(SrcDecl != nullptr && DstDecl != nullptr &&
- (SrcDecl->isDerivedFrom(DstDecl) || DstDecl->isDerivedFrom(SrcDecl)));
+ (SrcDecl->isDerivedFrom(DstDecl) || DstDecl->isDerivedFrom(SrcDecl) ||
+ (DeclToCopy != nullptr && SrcDecl->isDerivedFrom(DeclToCopy) &&
+ DstDecl->isDerivedFrom(DeclToCopy))));
LLVM_DEBUG({
- if (!compatibleTypes) {
+ if (!CompatibleTypes) {
llvm::dbgs() << "Source type " << Src.getType() << "\n";
llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
}
});
- assert(compatibleTypes);
+ assert(CompatibleTypes);
if (SrcType == DstType || (SrcDecl != nullptr && DstDecl != nullptr &&
SrcDecl->isDerivedFrom(DstDecl))) {
@@ -76,12 +84,23 @@ void copyRecord(RecordStorageLocation &Src,
RecordStorageLocation &Dst,
for (const auto &[Name, DstFieldLoc] : Dst.synthetic_fields())
copySyntheticField(DstFieldLoc->getType(), Src.getSyntheticField(Name),
*DstFieldLoc, Env);
- } else {
+ } else if (DstDecl->isDerivedFrom(SrcDecl)) {
for (auto [Field, SrcFieldLoc] : Src.children())
copyField(*Field, SrcFieldLoc, Dst.getChild(*Field), Dst, Env);
for (const auto &[Name, SrcFieldLoc] : Src.synthetic_fields())
copySyntheticField(SrcFieldLoc->getType(), *SrcFieldLoc,
Dst.getSyntheticField(Name), Env);
+ } else {
+ for (const FieldDecl *Field :
+ Env.getDataflowAnalysisContext().getModeledFields(TypeToCopy)) {
+ copyField(*Field, Src.getChild(*Field), Dst.getChild(*Field), Dst, Env);
+ }
+ for (const auto &[SyntheticFieldName, SyntheticFieldType] :
+ Env.getDataflowAnalysisContext().getSyntheticFields(TypeToCopy)) {
+ copySyntheticField(SyntheticFieldType,
+ Src.getSyntheticField(SyntheticFieldName),
+ Dst.getSyntheticField(SyntheticFieldName), Env);
+ }
}
}
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 23a6de45e18b1..60371d9498c25 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -610,7 +610,15 @@ class TransferVisitor : public
ConstStmtVisitor<TransferVisitor> {
// Even if the copy/move constructor call is elidable, we choose to copy
// the record in all cases (which isn't wrong, just potentially not
// optimal).
- copyRecord(*ArgLoc, Loc, Env);
+ //
+ // To handle cases of base class initializers in constructors, where a
+ // sibling derived class can be used to initialize a shared-base-class
+ // subobject through a DerivedToBase cast, intentionally copy only the
+ // parts of `ArgLoc` that are part of the base class being initialized.
+ // This is necessary because the type of `Loc` in these cases is the
+ // derived type ultimately being constructed, not the type of the base
+ // class subobject.
+ copyRecord(*ArgLoc, Loc, Env, S->getType());
return;
}
diff --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index 88b92668c850c..57162cd2efcc4 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -8,8 +8,15 @@
#include "clang/Analysis/FlowSensitive/RecordOps.h"
#include "TestingSupport.h"
+#include "clang/AST/Type.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/NoopLattice.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/Testing/Support/Error.h"
#include "gtest/gtest.h"
+#include <string>
namespace clang {
namespace dataflow {
@@ -190,7 +197,7 @@ TEST(RecordOpsTest, RecordsEqual) {
});
}
-TEST(TransferTest, CopyRecordBetweenDerivedAndBase) {
+TEST(RecordOpsTest, CopyRecordBetweenDerivedAndBase) {
std::string Code = R"(
struct A {
int i;
@@ -266,6 +273,67 @@ TEST(TransferTest, CopyRecordBetweenDerivedAndBase) {
});
}
+TEST(RecordOpsTest, CopyRecordWithExplicitSharedBaseTypeToCopy) {
+ std::string Code = R"(
+ struct Base {
+ bool BaseField;
+ char UnmodeledField;
+ };
+
+ struct DerivedOne : public Base {
+ int DerivedOneField;
+ };
+
+ struct DerivedTwo : public Base {
+ int DerivedTwoField;
+ };
+
+ void target(Base B, DerivedOne D1, DerivedTwo D2) {
+ (void) B.BaseField;
+ // [[p]]
+ }
+ )";
+ auto SyntheticFieldCallback = [](QualType Ty) -> llvm::StringMap<QualType> {
+ CXXRecordDecl *BaseDecl = nullptr;
+ std::string TypeAsString = Ty.getAsString();
+ if (TypeAsString == "Base")
+ BaseDecl = Ty->getAsCXXRecordDecl();
+ else if (TypeAsString == "DerivedOne" || TypeAsString == "DerivedTwo")
+ BaseDecl = Ty->getAsCXXRecordDecl()
+ ->bases_begin()
+ ->getType()
+ ->getAsCXXRecordDecl();
+ else
+ return {};
+ QualType FieldType = getFieldNamed(BaseDecl, "BaseField")->getType();
+ return {{"synth_field", FieldType}};
+ };
+ // Test copying derived to base class.
+ runDataflow(
+ Code, SyntheticFieldCallback,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment Env = getEnvironmentAtAnnotation(Results, "p").fork();
+
+ const ValueDecl *BaseFieldDecl = findValueDecl(ASTCtx, "BaseField");
+ auto &B = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "B");
+ auto &D1 = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "D1");
+ auto &D2 = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "D2");
+
+ EXPECT_NE(Env.getValue(*D1.getChild(*BaseFieldDecl)),
+ Env.getValue(*D2.getChild(*BaseFieldDecl)));
+ EXPECT_NE(Env.getValue(D1.getSyntheticField("synth_field")),
+ Env.getValue(D2.getSyntheticField("synth_field")));
+
+ copyRecord(D1, D2, Env, B.getType());
+
+ EXPECT_EQ(Env.getValue(*D1.getChild(*BaseFieldDecl)),
+ Env.getValue(*D2.getChild(*BaseFieldDecl)));
+ EXPECT_EQ(Env.getValue(D1.getSyntheticField("synth_field")),
+ Env.getValue(D2.getSyntheticField("synth_field")));
+ });
+}
+
} // namespace
} // namespace test
} // namespace dataflow
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 96e759e73c154..d97e2b0c2425a 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1536,6 +1536,40 @@ TEST(TransferTest, BaseClassInitializer) {
llvm::Succeeded());
}
+TEST(TransferTest, BaseClassInitializerFromSiblingDerivedInstance) {
+ using ast_matchers::cxxConstructorDecl;
+ using ast_matchers::hasName;
+ using ast_matchers::ofClass;
+
+ std::string Code = R"(
+ struct Base {
+ bool BaseField;
+ char UnmodeledField;
+ };
+
+ struct DerivedOne : public Base {
+ int DerivedOneField;
+ };
+
+ struct DerivedTwo : public Base {
+ int DerivedTwoField;
+
+ DerivedTwo(const DerivedOne& d1)
+ : Base(d1), DerivedTwoField(d1.DerivedOneField) {
+ (void)BaseField;
+ }
+ };
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ // Regression test only; we used to crash when transferring the base
+ // class initializer from the DerivedToBase-cast `d1`.
+ },
+ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "DerivedTwo");
+}
+
TEST(TransferTest, FieldsDontHaveValuesInConstructor) {
// In a constructor, unlike in regular member functions, we don't want fields
// to be pre-initialized with values, because doing so is the job of the
>From b0c713d5851dcb5dd5002ea15793947e17b4ae1f Mon Sep 17 00:00:00 2001
From: Samira Bakon <baz...@google.com>
Date: Tue, 9 Sep 2025 14:12:39 -0400
Subject: [PATCH 3/3] Add null checks
---
clang/lib/Analysis/FlowSensitive/RecordOps.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
index 71b2aa9ff2ec4..ed827ac2c7c90 100644
--- a/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
+++ b/clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -84,7 +84,8 @@ void copyRecord(RecordStorageLocation &Src,
RecordStorageLocation &Dst,
for (const auto &[Name, DstFieldLoc] : Dst.synthetic_fields())
copySyntheticField(DstFieldLoc->getType(), Src.getSyntheticField(Name),
*DstFieldLoc, Env);
- } else if (DstDecl->isDerivedFrom(SrcDecl)) {
+ } else if (SrcDecl != nullptr && DstDecl != nullptr &&
+ DstDecl->isDerivedFrom(SrcDecl)) {
for (auto [Field, SrcFieldLoc] : Src.children())
copyField(*Field, SrcFieldLoc, Dst.getChild(*Field), Dst, Env);
for (const auto &[Name, SrcFieldLoc] : Src.synthetic_fields())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
