[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC330009: [analyzer] Fix null deref in 
AnyFunctionCall::getRuntimeDefinition (authored by xazax, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45564

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/undef-call.c


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -389,31 +389,32 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD)
+return {};
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();
Index: test/Analysis/undef-call.c
===
--- test/Analysis/undef-call.c
+++ test/Analysis/undef-call.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze 
-analyzer-checker=debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
+// expected-no-diagnostics
+
+struct S {
+  void (*fp)();
+};
+
+int main() {
+  struct S s;
+  // This will cause the analyzer to look for a function definition that has
+  // no FunctionDecl. It used to cause a crash in 
AnyFunctionCall::getRuntimeDefinition.
+  // It would only occur when CTU analysis is enabled.
+  s.fp();
+}


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -389,31 +389,32 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD)
+return {};
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();
Index: test/Analysis/undef-call.c
===
--- test/Analysis/undef-call.c
+++ test/Analysis/undef-call.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze -analyzer-checker=debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
+// expected-no-diagnostics
+
+struct S {

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 142381.
r.stahl edited the summary of this revision.
r.stahl added a comment.

addressed review comments.

I created a new test because certain checkers would cause early exits in the 
engine (because of undefined func ptr) and not cause the crash.

Since I don't have commit access, please commit for me.


https://reviews.llvm.org/D45564

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/undef-call.c


Index: test/Analysis/undef-call.c
===
--- test/Analysis/undef-call.c
+++ test/Analysis/undef-call.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze 
-analyzer-checker=debug.ExprInspection -analyzer-config 
experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir 
-verify %s
+// expected-no-diagnostics
+
+struct S {
+  void (*fp)();
+};
+
+int main() {
+  struct S s;
+  // This will cause the analyzer to look for a function definition that has
+  // no FunctionDecl. It used to cause a crash in 
AnyFunctionCall::getRuntimeDefinition.
+  // It would only occur when CTU analysis is enabled.
+  s.fp();
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,32 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD)
+return {};
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();


Index: test/Analysis/undef-call.c
===
--- test/Analysis/undef-call.c
+++ test/Analysis/undef-call.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze -analyzer-checker=debug.ExprInspection -analyzer-config experimental-enable-naive-ctu-analysis=true -analyzer-config ctu-dir=%T/ctudir -verify %s
+// expected-no-diagnostics
+
+struct S {
+  void (*fp)();
+};
+
+int main() {
+  struct S s;
+  // This will cause the analyzer to look for a function definition that has
+  // no FunctionDecl. It used to cause a crash in AnyFunctionCall::getRuntimeDefinition.
+  // It would only occur when CTU analysis is enabled.
+  s.fp();
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,32 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD)
+return {};
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

We encountered the same problem but did not have time yet to submit the patch. 
We have literally the same fix internally, so it looks good to me. One minor 
style nit inline.

Could you add your repro as a regression test? You can also extend existing CTU 
tests just make sure to trigger the crash before the patch.

Thank you for the submission and the minimal reproducer.




Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:390
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};

We usually do not write the braces for single statements.


Repository:
  rC Clang

https://reviews.llvm.org/D45564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added a comment.

I encountered this with a construct like this:

  struct S
  {
  void (*fp)();
  };
  
  int main()
  {
  struct S s;
  s.fp();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D45564



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45564: [analyzer] Fix null deref in AnyFunctionCall::getRuntimeDefinition

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl created this revision.
r.stahl added reviewers: xazax.hun, dcoughlin, a.sidorin, george.karpenkov.
Herald added subscribers: cfe-commits, rnkovacs, szepet.

In https://reviews.llvm.org/D30691 code was added to getRuntimeDefinition that 
does not handle the case when FD==nullptr.


Repository:
  rC Clang

https://reviews.llvm.org/D45564

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,33 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};
+  }
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();


Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -387,31 +387,33 @@
 
 RuntimeDefinition AnyFunctionCall::getRuntimeDefinition() const {
   const FunctionDecl *FD = getDecl();
+  if (!FD) {
+return {};
+  }
+
   // Note that the AnalysisDeclContext will have the FunctionDecl with
   // the definition (if one exists).
-  if (FD) {
-AnalysisDeclContext *AD =
-  getLocationContext()->getAnalysisDeclContext()->
-  getManager()->getContext(FD);
-bool IsAutosynthesized;
-Stmt* Body = AD->getBody(IsAutosynthesized);
-DEBUG({
-if (IsAutosynthesized)
-  llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
-   << "\n";
-});
-if (Body) {
-  const Decl* Decl = AD->getDecl();
-  return RuntimeDefinition(Decl);
-}
+  AnalysisDeclContext *AD =
+getLocationContext()->getAnalysisDeclContext()->
+getManager()->getContext(FD);
+  bool IsAutosynthesized;
+  Stmt* Body = AD->getBody(IsAutosynthesized);
+  DEBUG({
+  if (IsAutosynthesized)
+llvm::dbgs() << "Using autosynthesized body for " << FD->getName()
+ << "\n";
+  });
+  if (Body) {
+const Decl* Decl = AD->getDecl();
+return RuntimeDefinition(Decl);
   }
 
   SubEngine *Engine = getState()->getStateManager().getOwningEngine();
   AnalyzerOptions  = Engine->getAnalysisManager().options;
 
   // Try to get CTU definition only if CTUDir is provided.
   if (!Opts.naiveCTUEnabled())
-return RuntimeDefinition();
+return {};
 
   cross_tu::CrossTranslationUnitContext  =
   *Engine->getCrossTranslationUnitContext();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits