Skip to content

Commit ffe21f1

Browse files
authored
[clang][dataflow] Transfer more cast expressions. (#153066)
Transfer all casts by kind as we currently do implicit casts. This obviates the need for specific handling of static casts. Also transfer CK_BaseToDerived and CK_DerivedToBase and add tests for these and missing tests for already-handled cast types. Ensure that CK_BaseToDerived casts result in modeling of the fields of the derived class.
1 parent 80f3b37 commit ffe21f1

File tree

3 files changed

+349
-13
lines changed

3 files changed

+349
-13
lines changed

clang/include/clang/Analysis/FlowSensitive/StorageLocation.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "clang/AST/Decl.h"
1818
#include "clang/AST/Type.h"
1919
#include "llvm/ADT/DenseMap.h"
20+
#include "llvm/ADT/StringRef.h"
2021
#include "llvm/Support/Debug.h"
2122
#include <cassert>
2223

@@ -152,6 +153,11 @@ class RecordStorageLocation final : public StorageLocation {
152153
return {SyntheticFields.begin(), SyntheticFields.end()};
153154
}
154155

156+
/// Add a synthetic field, if none by that name is already present.
157+
void addSyntheticField(llvm::StringRef Name, StorageLocation &Loc) {
158+
SyntheticFields.insert({Name, &Loc});
159+
}
160+
155161
/// Changes the child storage location for a field `D` of reference type.
156162
/// All other fields cannot change their storage location and always retain
157163
/// the storage location passed to the `RecordStorageLocation` constructor.
@@ -164,6 +170,11 @@ class RecordStorageLocation final : public StorageLocation {
164170
Children[&D] = Loc;
165171
}
166172

173+
/// Add a child storage location for a field `D`, if not already present.
174+
void addChild(const ValueDecl &D, StorageLocation *Loc) {
175+
Children.insert({&D, Loc});
176+
}
177+
167178
llvm::iterator_range<FieldToLoc::const_iterator> children() const {
168179
return {Children.begin(), Children.end()};
169180
}

clang/lib/Analysis/FlowSensitive/Transfer.cpp

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,17 @@
2020
#include "clang/AST/OperationKinds.h"
2121
#include "clang/AST/Stmt.h"
2222
#include "clang/AST/StmtVisitor.h"
23+
#include "clang/AST/Type.h"
2324
#include "clang/Analysis/FlowSensitive/ASTOps.h"
2425
#include "clang/Analysis/FlowSensitive/AdornedCFG.h"
2526
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
2627
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
2728
#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
2829
#include "clang/Analysis/FlowSensitive/RecordOps.h"
30+
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
2931
#include "clang/Analysis/FlowSensitive/Value.h"
3032
#include "clang/Basic/Builtins.h"
33+
#include "clang/Basic/LLVM.h"
3134
#include "clang/Basic/OperatorKinds.h"
3235
#include "llvm/Support/Casting.h"
3336
#include <assert.h>
@@ -287,7 +290,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
287290
}
288291
}
289292

290-
void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
293+
void VisitCastExpr(const CastExpr *S) {
291294
const Expr *SubExpr = S->getSubExpr();
292295
assert(SubExpr != nullptr);
293296

@@ -317,17 +320,70 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
317320
break;
318321
}
319322

323+
case CK_BaseToDerived: {
324+
// This is a cast of (single-layer) pointer or reference to a record type.
325+
// We should now model the fields for the derived type.
326+
327+
// Get the RecordStorageLocation for the record object underneath.
328+
RecordStorageLocation *Loc = nullptr;
329+
if (S->getType()->isPointerType()) {
330+
auto *PV = Env.get<PointerValue>(*SubExpr);
331+
assert(PV != nullptr);
332+
if (PV == nullptr)
333+
break;
334+
Loc = cast<RecordStorageLocation>(&PV->getPointeeLoc());
335+
} else {
336+
assert(S->getType()->isRecordType());
337+
if (SubExpr->isGLValue()) {
338+
Loc = Env.get<RecordStorageLocation>(*SubExpr);
339+
} else {
340+
Loc = &Env.getResultObjectLocation(*SubExpr);
341+
}
342+
}
343+
if (!Loc) {
344+
// Nowhere to add children or propagate from, so we're done.
345+
break;
346+
}
347+
348+
// Get the derived record type underneath the reference or pointer.
349+
QualType Derived = S->getType().getNonReferenceType();
350+
if (Derived->isPointerType()) {
351+
Derived = Derived->getPointeeType();
352+
}
353+
354+
// Add children to the storage location for fields (including synthetic
355+
// fields) of the derived type and initialize their values.
356+
for (const FieldDecl *Field :
357+
Env.getDataflowAnalysisContext().getModeledFields(Derived)) {
358+
assert(Field != nullptr);
359+
QualType FieldType = Field->getType();
360+
if (FieldType->isReferenceType()) {
361+
Loc->addChild(*Field, nullptr);
362+
} else {
363+
Loc->addChild(*Field, &Env.createStorageLocation(FieldType));
364+
}
365+
366+
for (const auto &Entry :
367+
Env.getDataflowAnalysisContext().getSyntheticFields(Derived)) {
368+
Loc->addSyntheticField(Entry.getKey(),
369+
Env.createStorageLocation(Entry.getValue()));
370+
}
371+
}
372+
Env.initializeFieldsWithValues(*Loc, Derived);
373+
374+
// Fall through to propagate SubExpr's StorageLocation to the CastExpr.
375+
[[fallthrough]];
376+
}
320377
case CK_IntegralCast:
321378
// FIXME: This cast creates a new integral value from the
322379
// subexpression. But, because we don't model integers, we don't
323380
// distinguish between this new value and the underlying one. If integer
324381
// modeling is added, then update this code to create a fresh location and
325382
// value.
326383
case CK_UncheckedDerivedToBase:
384+
case CK_DerivedToBase:
327385
case CK_ConstructorConversion:
328386
case CK_UserDefinedConversion:
329-
// FIXME: Add tests that excercise CK_UncheckedDerivedToBase,
330-
// CK_ConstructorConversion, and CK_UserDefinedConversion.
331387
case CK_NoOp: {
332388
// FIXME: Consider making `Environment::getStorageLocation` skip noop
333389
// expressions (this and other similar expressions in the file) instead
@@ -684,15 +740,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
684740
propagateValue(*SubExpr, *S, Env);
685741
}
686742

687-
void VisitCXXStaticCastExpr(const CXXStaticCastExpr *S) {
688-
if (S->getCastKind() == CK_NoOp) {
689-
const Expr *SubExpr = S->getSubExpr();
690-
assert(SubExpr != nullptr);
691-
692-
propagateValueOrStorageLocation(*SubExpr, *S, Env);
693-
}
694-
}
695-
696743
void VisitConditionalOperator(const ConditionalOperator *S) {
697744
const Environment *TrueEnv = StmtToEnv.getEnvironment(*S->getTrueExpr());
698745
const Environment *FalseEnv = StmtToEnv.getEnvironment(*S->getFalseExpr());

0 commit comments

Comments
 (0)