[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
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)
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)
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)
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)
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)
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)
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)
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)
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)
@@ -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)
@@ -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)
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)
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)
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)
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)
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