Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][dataflow] Fix two null pointer dereferences in getMemberForAccessor(). #66742

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

martinboehme
Copy link
Contributor

The additions to the test trigger crashes without the fixes.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:analysis labels Sep 19, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 19, 2023

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-analysis

Changes

The additions to the test trigger crashes without the fixes.


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

2 Files Affected:

  • (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+4-1)
  • (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+9)
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 26e097349057238..98128693e145da4 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -289,11 +289,14 @@ static void insertIfFunction(const Decl &D,
 }
 
 static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) {
+  if (!C.getMethodDecl())
+    return nullptr;
   auto *Body = dyn_cast_or_null<CompoundStmt>(C.getMethodDecl()->getBody());
   if (!Body || Body->size() != 1)
     return nullptr;
   if (auto *RS = dyn_cast<ReturnStmt>(*Body->body_begin()))
-    return dyn_cast<MemberExpr>(RS->getRetValue()->IgnoreParenImpCasts());
+    if (RS->getRetValue() != nullptr)
+      return dyn_cast<MemberExpr>(RS->getRetValue()->IgnoreParenImpCasts());
   return nullptr;
 }
 
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 14188f5acd5b36e..e8cbca756460369 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1463,6 +1463,7 @@ TEST(TransferTest, StructModeledFieldsWithAccessor) {
       int getIntNotAccessed() const { return IntNotAccessed; }
       int getIntNoDefinition() const;
       int &getIntRef() { return IntRef; }
+      void returnVoid() const { return; }
     };
 
     void target() {
@@ -1473,6 +1474,14 @@ TEST(TransferTest, StructModeledFieldsWithAccessor) {
       int i2 = s.getWithInc(1);
       int i3 = s.getIntNoDefinition();
       int &iref = s.getIntRef();
+
+      // Regression test: Don't crash on an indirect call (which doesn't have
+      // an associated `CXXMethodDecl`).
+      auto ptr_to_member_fn = &S::getPtr;
+      p1 = (s.*ptr_to_member_fn)();
+
+      // Regression test: Don't crash on a return statement without a value.
+      s.returnVoid();
       // [[p]]
     }
   )";

…ccessor()`.

The additions to the test trigger crashes without the fixes.
@martinboehme martinboehme merged commit 1d7b59c into llvm:main Sep 19, 2023
1 of 2 checks passed
ZijunZhaoCCK pushed a commit to ZijunZhaoCCK/llvm-project that referenced this pull request Sep 19, 2023
…ccessor()`. (llvm#66742)

The additions to the test trigger crashes without the fixes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:analysis clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants