[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-04 Thread via cfe-commits

martinboehme wrote:

> @bazuzi and @martinboehme this change seems to be causing a test failure on 
> many bots. Can you take a look and revert if you need time to investigate?
> 
> * https://lab.llvm.org/buildbot/#/builders/139/builds/54773
> * https://lab.llvm.org/buildbot/#/builders/216/builds/31319
> * https://lab.llvm.org/buildbot/#/builders/275/builds/1805
> * https://lab.llvm.org/buildbot/#/builders/164/builds/47155
> * https://lab.llvm.org/buildbot/#/builders/245/builds/17508
> * https://lab.llvm.org/buildbot/#/builders/258/builds/10506
> * https://lab.llvm.org/buildbot/#/builders/230/builds/22088
> * https://lab.llvm.org/buildbot/#/builders/109/builds/78839
> * https://lab.llvm.org/buildbot/#/builders/272/builds/2382

Thanks for the heads up, and sorry for the breakage. Reverting.

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-04 Thread Jay Foad via cfe-commits

jayfoad wrote:

Hi, on my Release+Asserts build this is causing:
```
FAIL: Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/32/38 (134 of 658)
 TEST 'Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/32/38' FAILED 

Script(shard):
--
GTEST_OUTPUT=json:/home/jayfoad2/llvm-release/tools/clang/unittests/Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests-Clang-Unit-2611196-32-38.json
 GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=38 GTEST_SHARD_INDEX=32 
/home/jayfoad2/llvm-release/tools/clang/unittests/Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests
--

Script:
--
/home/jayfoad2/llvm-release/tools/clang/unittests/Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests
 
--gtest_filter=EnvironmentTest.ModelMemberForAccessorUsingMethodPointerThroughTemplate
--
/home/jayfoad2/git/llvm-project/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp:362:
 Failure
Value of: DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 0))
Expected: contains at least one element that is equal to 0x4b29e98
  Actual: {}


/home/jayfoad2/git/llvm-project/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp:362
Value of: DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 0))
Expected: contains at least one element that is equal to 0x4b29e98
  Actual: {}





Failed Tests (1):
  Clang-Unit :: 
Analysis/FlowSensitive/./ClangAnalysisFlowSensitiveTests/EnvironmentTest/ModelMemberForAccessorUsingMethodPointerThroughTemplate
```

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-04 Thread via cfe-commits

dyung wrote:

@bazuzi  and @martinboehme this change seems to be causing a test failure on 
many bots. Can you take a look and revert if you need time to investigate?

- https://lab.llvm.org/buildbot/#/builders/139/builds/54773
- https://lab.llvm.org/buildbot/#/builders/216/builds/31319
- https://lab.llvm.org/buildbot/#/builders/275/builds/1805
- https://lab.llvm.org/buildbot/#/builders/164/builds/47155
- https://lab.llvm.org/buildbot/#/builders/245/builds/17508
- https://lab.llvm.org/buildbot/#/builders/258/builds/10506
- https://lab.llvm.org/buildbot/#/builders/230/builds/22088
- https://lab.llvm.org/buildbot/#/builders/109/builds/78839
- https://lab.llvm.org/buildbot/#/builders/272/builds/2382

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-04 Thread via cfe-commits

https://github.com/martinboehme closed 
https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Samira Bazuzi via cfe-commits

bazuzi wrote:

Can you merge for me, now that I've finally remembered to format everything?

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Samira Bazuzi via cfe-commits

https://github.com/bazuzi updated 
https://github.com/llvm/llvm-project/pull/73978

>From aab1069636896c4a8289545ba01d2fdf4f1715c0 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Thu, 30 Nov 2023 14:18:04 -0500
Subject: [PATCH 1/4] [clang][dataflow] Retrieve members from accessors called
 using member pointers.

getMethodDecl does not handle pointers to members and returns nullptr.
---
 .../FlowSensitive/DataflowEnvironment.cpp |  5 +-
 .../FlowSensitive/DataflowEnvironmentTest.cpp | 50 +++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 525ab188b01b8aa..5a37fa0f02f5d21 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  if (!C.getMethodDecl())
+  const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
+  if (!MethodDecl)
 return nullptr;
-  auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody());
+  auto *Body = dyn_cast_or_null(MethodDecl->getBody());
   if (!Body || Body->size() != 1)
 return nullptr;
   if (auto *RS = dyn_cast(*Body->body_begin()))
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index ff6cbec5d20b744..55fb6549621737a 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   EXPECT_THAT(Env.getValue(*Var), NotNull());
 }
 
+// Pointers to Members are a tricky case of accessor calls, complicated further
+// when using templates where the pointer to the member is a template argument.
+// This is a repro of a failure case seen in the wild.
+TEST_F(EnvironmentTest,
+   ModelMemberForAccessorUsingMethodPointerThroughTemplate) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+  struct S {
+int accessor() {return member;}
+
+int member = 0;
+  };
+
+  template 
+  int Target(S* S) {
+return (S->*method)();
+  }
+
+ // We want to analyze the instantiation of Target for the accessor.
+ int Instantiator () {S S; return Target<&S::accessor>(&S); }
+  )cc";
+
+  auto Unit =
+  // C++17 for the simplifying use of auto in the template declaration.
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results = match(
+  decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation())
+ .bind("target"),
+ fieldDecl(hasName("member")).bind("member"),
+ recordDecl(hasName("S")).bind("struct"))),
+  Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *Struct = selectFirst("struct", Results);
+  const auto *Member = selectFirst("member", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(Struct, NotNull());
+  ASSERT_THAT(Member, NotNull());
+
+  // Verify that `member` is modeled for `S` when we analyze
+  // `Target<&S::accessor>`.
+  Environment Env(DAContext, *Fun);
+  EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 
0)),
+  Contains(Member));
+}
+
 TEST_F(EnvironmentTest, RefreshRecordValue) {
   using namespace ast_matchers;
 

>From 120bdd51496dd69c601181bccae072effb547920 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Fri, 1 Dec 2023 10:33:54 -0500
Subject: [PATCH 2/4] Add missing using declaration to test.

And add comment.
---
 clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 1 +
 .../unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 1 +
 2 files changed, 2 insertions(+)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 5a37fa0f02f5d21..0b962bff4c183bf 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,6 +300,7 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
+  // Use getCalleeDecl instead of getMethodDecl in order to handle 
pointer-to-member calls.
   const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
   if (!MethodDecl)
 return nullptr;
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index 55fb6549621737a..abebd362bec471f 100644
--- a/clang/unittests/Analysis/FlowSensiti

[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Samira Bazuzi via cfe-commits

https://github.com/bazuzi updated 
https://github.com/llvm/llvm-project/pull/73978

>From aab1069636896c4a8289545ba01d2fdf4f1715c0 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Thu, 30 Nov 2023 14:18:04 -0500
Subject: [PATCH 1/3] [clang][dataflow] Retrieve members from accessors called
 using member pointers.

getMethodDecl does not handle pointers to members and returns nullptr.
---
 .../FlowSensitive/DataflowEnvironment.cpp |  5 +-
 .../FlowSensitive/DataflowEnvironmentTest.cpp | 50 +++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 525ab188b01b8aa..5a37fa0f02f5d21 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  if (!C.getMethodDecl())
+  const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
+  if (!MethodDecl)
 return nullptr;
-  auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody());
+  auto *Body = dyn_cast_or_null(MethodDecl->getBody());
   if (!Body || Body->size() != 1)
 return nullptr;
   if (auto *RS = dyn_cast(*Body->body_begin()))
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index ff6cbec5d20b744..55fb6549621737a 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   EXPECT_THAT(Env.getValue(*Var), NotNull());
 }
 
+// Pointers to Members are a tricky case of accessor calls, complicated further
+// when using templates where the pointer to the member is a template argument.
+// This is a repro of a failure case seen in the wild.
+TEST_F(EnvironmentTest,
+   ModelMemberForAccessorUsingMethodPointerThroughTemplate) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+  struct S {
+int accessor() {return member;}
+
+int member = 0;
+  };
+
+  template 
+  int Target(S* S) {
+return (S->*method)();
+  }
+
+ // We want to analyze the instantiation of Target for the accessor.
+ int Instantiator () {S S; return Target<&S::accessor>(&S); }
+  )cc";
+
+  auto Unit =
+  // C++17 for the simplifying use of auto in the template declaration.
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results = match(
+  decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation())
+ .bind("target"),
+ fieldDecl(hasName("member")).bind("member"),
+ recordDecl(hasName("S")).bind("struct"))),
+  Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *Struct = selectFirst("struct", Results);
+  const auto *Member = selectFirst("member", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(Struct, NotNull());
+  ASSERT_THAT(Member, NotNull());
+
+  // Verify that `member` is modeled for `S` when we analyze
+  // `Target<&S::accessor>`.
+  Environment Env(DAContext, *Fun);
+  EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 
0)),
+  Contains(Member));
+}
+
 TEST_F(EnvironmentTest, RefreshRecordValue) {
   using namespace ast_matchers;
 

>From 120bdd51496dd69c601181bccae072effb547920 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Fri, 1 Dec 2023 10:33:54 -0500
Subject: [PATCH 2/3] Add missing using declaration to test.

And add comment.
---
 clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 1 +
 .../unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 1 +
 2 files changed, 2 insertions(+)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 5a37fa0f02f5d21..0b962bff4c183bf 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,6 +300,7 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
+  // Use getCalleeDecl instead of getMethodDecl in order to handle 
pointer-to-member calls.
   const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
   if (!MethodDecl)
 return nullptr;
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index 55fb6549621737a..abebd362bec471f 100644
--- a/clang/unittests/Analysis/FlowSensiti

[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread via cfe-commits

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff ff485a0e77a55847cb50768b01c04fe45a6879ea 
120bdd51496dd69c601181bccae072effb547920 -- 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 0b962bff4c..bb43d179a0 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,7 +300,8 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  // Use getCalleeDecl instead of getMethodDecl in order to handle 
pointer-to-member calls.
+  // Use getCalleeDecl instead of getMethodDecl in order to handle
+  // pointer-to-member calls.
   const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
   if (!MethodDecl)
 return nullptr;
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index abebd362be..0e77383290 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -25,9 +25,9 @@ namespace {
 using namespace clang;
 using namespace dataflow;
 using ::clang::dataflow::test::getFieldValue;
+using ::testing::Contains;
 using ::testing::IsNull;
 using ::testing::NotNull;
-using ::testing::Contains;
 
 class EnvironmentTest : public ::testing::Test {
 protected:

``




https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Samira Bazuzi via cfe-commits

https://github.com/bazuzi updated 
https://github.com/llvm/llvm-project/pull/73978

>From aab1069636896c4a8289545ba01d2fdf4f1715c0 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Thu, 30 Nov 2023 14:18:04 -0500
Subject: [PATCH 1/2] [clang][dataflow] Retrieve members from accessors called
 using member pointers.

getMethodDecl does not handle pointers to members and returns nullptr.
---
 .../FlowSensitive/DataflowEnvironment.cpp |  5 +-
 .../FlowSensitive/DataflowEnvironmentTest.cpp | 50 +++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 525ab188b01b8aa..5a37fa0f02f5d21 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  if (!C.getMethodDecl())
+  const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
+  if (!MethodDecl)
 return nullptr;
-  auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody());
+  auto *Body = dyn_cast_or_null(MethodDecl->getBody());
   if (!Body || Body->size() != 1)
 return nullptr;
   if (auto *RS = dyn_cast(*Body->body_begin()))
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index ff6cbec5d20b744..55fb6549621737a 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   EXPECT_THAT(Env.getValue(*Var), NotNull());
 }
 
+// Pointers to Members are a tricky case of accessor calls, complicated further
+// when using templates where the pointer to the member is a template argument.
+// This is a repro of a failure case seen in the wild.
+TEST_F(EnvironmentTest,
+   ModelMemberForAccessorUsingMethodPointerThroughTemplate) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+  struct S {
+int accessor() {return member;}
+
+int member = 0;
+  };
+
+  template 
+  int Target(S* S) {
+return (S->*method)();
+  }
+
+ // We want to analyze the instantiation of Target for the accessor.
+ int Instantiator () {S S; return Target<&S::accessor>(&S); }
+  )cc";
+
+  auto Unit =
+  // C++17 for the simplifying use of auto in the template declaration.
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results = match(
+  decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation())
+ .bind("target"),
+ fieldDecl(hasName("member")).bind("member"),
+ recordDecl(hasName("S")).bind("struct"))),
+  Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *Struct = selectFirst("struct", Results);
+  const auto *Member = selectFirst("member", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(Struct, NotNull());
+  ASSERT_THAT(Member, NotNull());
+
+  // Verify that `member` is modeled for `S` when we analyze
+  // `Target<&S::accessor>`.
+  Environment Env(DAContext, *Fun);
+  EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 
0)),
+  Contains(Member));
+}
+
 TEST_F(EnvironmentTest, RefreshRecordValue) {
   using namespace ast_matchers;
 

>From 120bdd51496dd69c601181bccae072effb547920 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi 
Date: Fri, 1 Dec 2023 10:33:54 -0500
Subject: [PATCH 2/2] Add missing using declaration to test.

And add comment.
---
 clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 1 +
 .../unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 1 +
 2 files changed, 2 insertions(+)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 5a37fa0f02f5d21..0b962bff4c183bf 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,6 +300,7 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
+  // Use getCalleeDecl instead of getMethodDecl in order to handle 
pointer-to-member calls.
   const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
   if (!MethodDecl)
 return nullptr;
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index 55fb6549621737a..abebd362bec471f 100644
--- a/clang/unittests/Analysis/FlowSensiti

[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Samira Bazuzi via cfe-commits


@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  if (!C.getMethodDecl())
+  const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());

bazuzi wrote:

Done

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Yitzhak Mandelbaum via cfe-commits


@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  if (!C.getMethodDecl())
+  const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());

ymand wrote:

Please add a comment explaining why we prefer this method over `getMethodDecl`.

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Yitzhak Mandelbaum via cfe-commits

https://github.com/ymand approved this pull request.

Thanks!

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Yitzhak Mandelbaum via cfe-commits

https://github.com/ymand edited https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-12-01 Thread Yitzhak Mandelbaum via cfe-commits

ymand wrote:

> @ymand Can you review?

sure

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-11-30 Thread Samira Bazuzi via cfe-commits

bazuzi wrote:

@ymand Can you review?

https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)

2023-11-30 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-clang-analysis

Author: Samira Bazuzi (bazuzi)


Changes

… pointers.

getMethodDecl does not handle pointers to members and returns nullptr for them. 
getMethodDecl contains a decade-plus-old FIXME to handle pointers to members, 
but two approaches I looked at for fixing it are more invasive or complex than 
simply swapping to getCalleeDecl.

The first, have getMethodDecl call getCalleeDecl, creates a large tree of 
const-ness mismatches due to getMethodDecl returning a non-const value while 
being a const member function and getCalleeDecl only being a const member 
function when it returns a const value.

The second, implementing an AST walk to match how 
CXXMemberCallExpr::getImplicitObjectArgument grabs the LHS of the binary 
operator, is basically reimplementing Expr::getReferencedDeclOfCallee, which is 
used by Expr::getCalleeDecl. We don't need another copy of that code.

---
Full diff: https://github.com/llvm/llvm-project/pull/73978.diff


2 Files Affected:

- (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+3-2) 
- (modified) clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
(+50) 


``diff
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 525ab188b01b8aa..5a37fa0f02f5d21 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
-  if (!C.getMethodDecl())
+  const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl());
+  if (!MethodDecl)
 return nullptr;
-  auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody());
+  auto *Body = dyn_cast_or_null(MethodDecl->getBody());
   if (!Body || Body->size() != 1)
 return nullptr;
   if (auto *RS = dyn_cast(*Body->body_begin()))
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index ff6cbec5d20b744..55fb6549621737a 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   EXPECT_THAT(Env.getValue(*Var), NotNull());
 }
 
+// Pointers to Members are a tricky case of accessor calls, complicated further
+// when using templates where the pointer to the member is a template argument.
+// This is a repro of a failure case seen in the wild.
+TEST_F(EnvironmentTest,
+   ModelMemberForAccessorUsingMethodPointerThroughTemplate) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+  struct S {
+int accessor() {return member;}
+
+int member = 0;
+  };
+
+  template 
+  int Target(S* S) {
+return (S->*method)();
+  }
+
+ // We want to analyze the instantiation of Target for the accessor.
+ int Instantiator () {S S; return Target<&S::accessor>(&S); }
+  )cc";
+
+  auto Unit =
+  // C++17 for the simplifying use of auto in the template declaration.
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results = match(
+  decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation())
+ .bind("target"),
+ fieldDecl(hasName("member")).bind("member"),
+ recordDecl(hasName("S")).bind("struct"))),
+  Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *Struct = selectFirst("struct", Results);
+  const auto *Member = selectFirst("member", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(Struct, NotNull());
+  ASSERT_THAT(Member, NotNull());
+
+  // Verify that `member` is modeled for `S` when we analyze
+  // `Target<&S::accessor>`.
+  Environment Env(DAContext, *Fun);
+  EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 
0)),
+  Contains(Member));
+}
+
 TEST_F(EnvironmentTest, RefreshRecordValue) {
   using namespace ast_matchers;
 

``




https://github.com/llvm/llvm-project/pull/73978
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits