[clang] [clang][dataflow] Model the fields that are accessed via inline accessors (PR #66368)

[Kinuko Yasuda via cfe-commits]

https://github.com/kinu created https://github.com/llvm/llvm-project/pull/66368:

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.

>From d3f686a9f342d3cf38ed0cf742fd0377bb998891 Mon Sep 17 00:00:00 2001
From: Kinuko Yasuda <kinuko at chromium.org>
Date: Thu, 14 Sep 2023 12:45:04 +0000
Subject: [PATCH] [clang][dataflow] Model the fields that are accessed via
 inline accessors

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.
---
 .../FlowSensitive/DataflowEnvironment.cpp     | 18 ++++++++
 .../Analysis/FlowSensitive/TransferTest.cpp   | 44 +++++++++++++++++++
 2 files changed, 62 insertions(+)

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 {