[clang][dataflow] Model the fields that are accessed via inline accessors

[kinu]

So that the values that are accessed via such accessors can be analyzed as a limited version of context-sensitive analysis. We can potentially do this only when some option is set, but doing additional modeling like this won't be expensive and intrusive, so we do it by default for now.

[kinu]

/cc @martinboehme @ymand

[llvmbot]

@llvm/pr-subscribers-clang-analysis

@llvm/pr-subscribers-clang

Changes

So that the values that are accessed via such accessors can be analyzed as a limited version of context-sensitive analysis. We can potentially do this only when some option is set, but doing additional modeling like this won't be expensive and intrusive, so we do it by default for now. -- Full diff: https://github.com//pull/66368.diff

2 Files Affected:

• (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+18)
• (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+44)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e13f880896fc071..713df62e5009336 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -288,6 +288,18 @@ static void insertIfFunction(const Decl &D,
     Funcs.insert(FD);
 }
 
+static Expr *getRetValueFromSingleReturnStmtMethod(const CXXMemberCallExpr &C) {
+  auto *D = cast_or_null<CXXMethodDecl>(C.getMethodDecl()->getDefinition());
+  if (!D)
+    return nullptr;
+  auto *S = cast<CompoundStmt>(D->getBody());
+  if (S->size() != 1)
+    return nullptr;
+  if (auto *RS = dyn_cast<ReturnStmt>(*S->body_begin()))
+    return RS->getRetValue()->IgnoreParenImpCasts();
+  return nullptr;
+}
+
 static void
 getFieldsGlobalsAndFuncs(const Decl &D, FieldSet &Fields,
                          llvm::DenseSet<const VarDecl *> &Vars,
@@ -324,6 +336,12 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
   } else if (auto *E = dyn_cast<DeclRefExpr>(&S)) {
     insertIfGlobal(*E->getDecl(), Vars);
     insertIfFunction(*E->getDecl(), Funcs);
+  } else if (const auto *C = dyn_cast<CXXMemberCallExpr>(&S)) {
+    // If this is a method that returns a member variable but does nothing else,
+    // model the field of the return value.
+    if (MemberExpr *E = dyn_cast_or_null<MemberExpr>(
+        getRetValueFromSingleReturnStmtMethod(*C)))
+      getFieldsGlobalsAndFuncs(*E, Fields, Vars, Funcs);
   } else if (auto *E = dyn_cast<MemberExpr>(&S)) {
     // FIXME: should we be using `E->getFoundDecl()`?
     const ValueDecl *VD = E->getMemberDecl();
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 0abd171f1d0b7cb..d52433b14da142a 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1446,6 +1446,50 @@ TEST(TransferTest, BaseClassInitializer) {
       llvm::Succeeded());
 }
 
+TEST(TransferTest, StructModeledFieldsWithConstAccessor) {
+  std::string Code = R"(
+    class S {
+      int *P;
+      int *Q;
+      int X;
+      int Y;
+      int Z;
+    public:
+      int *getPtr() const { return P; }
+      int *getPtrNonConst() { return Q; }
+      int getInt() const { return X; }
+      int getInt(int i) const { return Y; }
+      int getIntNonConst() { return Z; }
+      int getIntNoDefinition() const;
+    };
+
+    void target() {
+      S s;
+      int *p = s.getPtr();
+      int *q = s.getPtrNonConst();
+      int x = s.getInt();
+      int y = s.getIntNonConst();
+      int z = s.getIntNoDefinition();
+      // [[p]]
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {
+        const Environment &Env =
+              getEnvironmentAtAnnotation(Results, "p");
+        auto &SLoc = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "s");
+        std::vector<const ValueDecl*> Fields;
+        for (auto [Field, _] : SLoc.children())
+          Fields.push_back(Field);
+        // Only the fields that have corresponding const accessor methods
+        // should be modeled.
+        ASSERT_THAT(Fields, UnorderedElementsAre(
+            findValueDecl(ASTCtx, "P"), findValueDecl(ASTCtx, "X")));
+      });
+}
+
 TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
   std::string Code = R"(
     struct Base1 {

[kinu]

Thanks! Addressed comments.