[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366391: [analyzer] MallocChecker: Prevent Integer Set 
Library false positives (authored by Charusso, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64680?vs=210458=210463#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  cfe/trunk/test/Analysis/retain-count-alloc.cpp

Index: cfe/trunk/test/Analysis/retain-count-alloc.cpp
===
--- cfe/trunk/test/Analysis/retain-count-alloc.cpp
+++ cfe/trunk/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,11 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// See if deallocation happens in a suspicious context. If so, escape the
+  /// pointers that otherwise would have been deallocated and return true.
+  bool suppressDeallocationsInSuspiciousContexts(const CallExpr *CE,
+ CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +883,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (suppressDeallocationsInSuspiciousContexts(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2541,35 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::suppressDeallocationsInSuspiciousContexts(
+const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  if (Body->getBeginLoc().isValid())
+FunctionStr =
+Lexer::getSourceText(CharSourceRange::getTokenRange(
+ {FD->getBeginLoc(), Body->getBeginLoc()}),
+ C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments())
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol())
+  if (const RefState *RS = State->get(Sym))
+State = State->set(Sym, RefState::getEscaped(RS));
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2871,6 @@
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D64680#1590619 , @NoQ wrote:

> Great, thanks!


Thanks for the review! I like that new name.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Great, thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:363-364
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+

One last thing: let's make it obvious what does the function do.
- It not only checks and returns a boolean value, it also adds transitions. It 
is very important to know that the function adds transitions so that to avoid 
accidental state splits.
- In this case we're talking about a deallocation rather than allocation.
- Technically, "not modeled" is not quite correct, as we *are* modeling it, 
just differently.

I suggest something like this:
```lang=c++
/// See if deallocation happens in a suspicious context. If so, escape the 
pointers
/// that otherwise would have been deallocated and return true.
bool suppressDeallocationsInSuspiciousContexts(const CallExpr *CE, 
CheckerContext ) const;
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > I'm slightly worried that it'll crash when `free()` is being called from 
> > > within a body farm.
> > > 
> > > For now it probably cannot happen because none of the bodyfarmed 
> > > functions can call `free()` directly, but i'd anyway rather add a check 
> > > that the source locations we're taking are valid.
> > Oh, I missed that, thanks! I wanted to check for everything, yes.
> I think this is not fixed yet. I'm thinking of something like `if 
> (!Body->getBeginLoc().isValid()) { ... }`.
Ugh, silly mistake, thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 210458.
Charusso marked 4 inline comments as done.
Charusso added a comment.

- More fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +881,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (isNotModeled(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2539,34 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  if (Body->getBeginLoc().isValid())
+FunctionStr =
+Lexer::getSourceText(CharSourceRange::getTokenRange(
+ {FD->getBeginLoc(), Body->getBeginLoc()}),
+ C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments())
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol())
+  if (const RefState *RS = State->get(Sym))
+State = State->set(Sym, RefState::getEscaped(RS));
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2868,6 @@
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

Charusso wrote:
> NoQ wrote:
> > I'm slightly worried that it'll crash when `free()` is being called from 
> > within a body farm.
> > 
> > For now it probably cannot happen because none of the bodyfarmed functions 
> > can call `free()` directly, but i'd anyway rather add a check that the 
> > source locations we're taking are valid.
> Oh, I missed that, thanks! I wanted to check for everything, yes.
I think this is not fixed yet. I'm thinking of something like `if 
(!Body->getBeginLoc().isValid()) { ... }`.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2569
+  if (const RefState *RS = State->get(Sym)) {
+State = State->remove(Sym);
+State = State->set(Sym, RefState::getEscaped(RS));

Charusso wrote:
> NoQ wrote:
> > `remove` is unnecessary, we overwrite it anyway.
> I believe in so as well, just the official code base has this semantic. I 
> have rewritten that, see below in `checkPointerEscapeAux()`.
Yeah, and the official codebase is wrong :)



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2570
+
+  if (IsEscaped)
+C.addTransition(State);

The check is unnecessary. `addTransition` automatically takes care of the "no 
changes have happened" case and throws away the transition.

Moreover, in the current patch you aren't really checking if any changes have 
happened: the symbol may already be escaped, so in this case the flag is set to 
true but the state remains the same.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

NoQ wrote:
> I'm slightly worried that it'll crash when `free()` is being called from 
> within a body farm.
> 
> For now it probably cannot happen because none of the bodyfarmed functions 
> can call `free()` directly, but i'd anyway rather add a check that the source 
> locations we're taking are valid.
Oh, I missed that, thanks! I wanted to check for everything, yes.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2554-2559
+  if (FunctionStr.equals(""))
+return false;
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;

NoQ wrote:
> If the string is empty, it clearly cannot contain `__isl_`, so the first 
> check is redundant.
The first check is: "We got a body and its decl?", the second check is: "We got 
an ISL macro?". Yea, it is kind of redundant, just I like to pack one check in 
one IfStmt, and now that two question merges. Also I like to make them 
one-liners so they are self-explanatory.

Here is an example why I like it:
```
// Escape pointers passed into the list, unless it's an ObjC boxed  
 
// expression which is not a boxable C structure.   
 
if (!(isa(Ex) && 
 
  !cast(Ex)->getSubExpr()
 
  ->getType()->isRecordType()))
```
- from `ExprEngine::Visit()` - `Expr::ObjCBoxedExprClass` case.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2569
+  if (const RefState *RS = State->get(Sym)) {
+State = State->remove(Sym);
+State = State->set(Sym, RefState::getEscaped(RS));

NoQ wrote:
> `remove` is unnecessary, we overwrite it anyway.
I believe in so as well, just the official code base has this semantic. I have 
rewritten that, see below in `checkPointerEscapeAux()`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 210347.
Charusso marked 9 inline comments as done.
Charusso added a comment.

- Fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation. If any of the parameters has an
+//  '__isl_' prefixed macro definition we escape every
+//  of them when we are about to 'free()' something.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O); // Here we notice that the parameter contains '__isl_', escape it.
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +881,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (isNotModeled(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2539,40 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const auto *FD = dyn_cast(C.getStackFrame()->getDecl()))
+if (const Stmt *Body = FD->getBody())
+  FunctionStr =
+  Lexer::getSourceText(CharSourceRange::getTokenRange(
+   {FD->getBeginLoc(), Body->getBeginLoc()}),
+   C.getSourceManager(), C.getLangOpts());
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  bool IsEscaped = false;
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments()) {
+if (SymbolRef Sym = C.getSVal(Arg).getAsSymbol()) {
+  if (const RefState *RS = State->get(Sym)) {
+State = State->set(Sym, RefState::getEscaped(RS));
+IsEscaped = true;
+  }
+}
+  }
+
+  if (IsEscaped)
+C.addTransition(State);
+
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
@@ -2833,7 +2874,6 @@
 if (const RefState *RS = State->get(sym)) {
   if ((RS->isAllocated() || RS->isAllocatedOfSizeZero()) &&
   CheckRefState(RS)) {
-State = State->remove(sym);
 State = State->set(sym, RefState::getEscaped(RS));
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2547-2548
+  StringRef FunctionStr = "";
+  if (const Decl *D = C.getStackFrame()->getDecl())
+if (const FunctionDecl *FD = D->getAsFunction())
+  FunctionStr = Lexer::getSourceText(

`const auto *FD = dyn_cast(C.getStackFrame()->getDecl())`.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2549-2552
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());

I'm slightly worried that it'll crash when `free()` is being called from within 
a body farm.

For now it probably cannot happen because none of the bodyfarmed functions can 
call `free()` directly, but i'd anyway rather add a check that the source 
locations we're taking are valid.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2554-2559
+  if (FunctionStr.equals(""))
+return false;
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;

If the string is empty, it clearly cannot contain `__isl_`, so the first check 
is redundant.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2564-2566
+if (const auto *DRE = dyn_cast(Arg->IgnoreImpCasts())) {
+  if (const auto *VD = dyn_cast(DRE->getDecl())) {
+SVal V = State->getSVal(State->getLValue(VD, C.getLocationContext()));

`C.getSVal(Arg)`.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2569
+  if (const RefState *RS = State->get(Sym)) {
+State = State->remove(Sym);
+State = State->set(Sym, RefState::getEscaped(RS));

`remove` is unnecessary, we overwrite it anyway.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 4 inline comments as done.
Charusso added a comment.

In D64680#1584315 , @NoQ wrote:

> P.S. I think you should attach the report to Phabricator directly, as the 
> link will expire as soon as these reports get regenerated.


Luckily the stable scan-build namings are stable, so that is why I picked that 
handy option.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:56-58
+// If this checker does not model the allocation.
+DoNothing,
+// Reference to allocated memory.

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > We already have `Escaped`, it's the same thing in practice.
> > It is more strict than `Escaped`, also it made for the purpose of 
> > `PSK_EscapeOther` to force out we lost the entire pointer and do not make 
> > false warnings of use-after-free.
> How exactly is it more strict? I.e., what warnings are getting suppressed by 
> you that aren't going to be suppressed if you use `Escaped` instead?
After some measurements the previously attached report has nothing to do with 
strictness, just we really miss some escaping. Reverted that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 209849.
Charusso added a comment.

- Remove unnecessary `DoNothing` kind.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O);
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -359,6 +360,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +881,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (isNotModeled(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);
 } else if (FunI == II_strdup || FunI == II_win_strdup ||
FunI == II_wcsdup || FunI == II_win_wcsdup) {
@@ -2532,6 +2539,45 @@
   return (RS && RS->isReleased());
 }
 
+bool MallocChecker::isNotModeled(const CallExpr *CE, CheckerContext ) const {
+  if (CE->getNumArgs() == 0)
+return false;
+
+  StringRef FunctionStr = "";
+  if (const Decl *D = C.getStackFrame()->getDecl())
+if (const FunctionDecl *FD = D->getAsFunction())
+  FunctionStr = Lexer::getSourceText(
+  CharSourceRange::getTokenRange(
+  {FD->getBeginLoc(), FD->getBody()->getBeginLoc()}),
+  C.getSourceManager(), C.getLangOpts());
+
+  if (FunctionStr.equals(""))
+return false;
+
+  // We do not model the Integer Set Library's retain-count based allocation.
+  if (!FunctionStr.contains("__isl_"))
+return false;
+
+  ProgramStateRef State = C.getState();
+
+  for (const Expr *Arg : CE->arguments()) {
+if (const auto *DRE = dyn_cast(Arg->IgnoreImpCasts())) {
+  if (const auto *VD = dyn_cast(DRE->getDecl())) {
+SVal V = State->getSVal(State->getLValue(VD, C.getLocationContext()));
+if (SymbolRef Sym = V.getAsSymbol()) {
+  if (const RefState *RS = State->get(Sym)) {
+State = State->remove(Sym);
+State = State->set(Sym, RefState::getEscaped(RS));
+  }
+}
+  }
+}
+  }
+
+  C.addTransition(State);
+  return true;
+}
+
 bool MallocChecker::checkUseAfterFree(SymbolRef Sym, CheckerContext ,
   const Stmt *S) const {
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D64680#1584130 , @Charusso wrote:

> Here is an example of the mentioned use-after-free by pointer-escaping as an 
> argument:
>  
> https://llvm.org/reports/scan-build/report-DeclBase.cpp-getFromVoidPointer-0-1.html#EndPath


Not sure how is this false positive related to that report, but this false 
positive looks super weird and i'd love to debug it more.

P.S. I think you should attach the report to Phabricator directly, as the link 
will expire as soon as these reports get regenerated.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:56-58
+// If this checker does not model the allocation.
+DoNothing,
+// Reference to allocated memory.

Charusso wrote:
> NoQ wrote:
> > We already have `Escaped`, it's the same thing in practice.
> It is more strict than `Escaped`, also it made for the purpose of 
> `PSK_EscapeOther` to force out we lost the entire pointer and do not make 
> false warnings of use-after-free.
How exactly is it more strict? I.e., what warnings are getting suppressed by 
you that aren't going to be suppressed if you use `Escaped` instead?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

Here is an example of the mentioned use-after-free by pointer-escaping as an 
argument:
https://llvm.org/reports/scan-build/report-DeclBase.cpp-getFromVoidPointer-0-1.html#EndPath


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 209654.
Charusso marked 2 inline comments as done.
Charusso added a comment.

- Fix.
- Move the logic to `free()` for better matching.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O);
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -41,6 +42,7 @@
 // Used to check correspondence between allocators and deallocators.
 enum AllocationFamily {
   AF_None,
+  AF_NotModeled,
   AF_Malloc,
   AF_CXXNew,
   AF_CXXNewArray,
@@ -50,19 +52,22 @@
 };
 
 class RefState {
-  enum Kind { // Reference to allocated memory.
-  Allocated,
-  // Reference to zero-allocated memory.
-  AllocatedOfSizeZero,
-  // Reference to released/freed memory.
-  Released,
-  // The responsibility for freeing resources has transferred from
-  // this reference. A relinquished symbol should not be freed.
-  Relinquished,
-  // We are no longer guaranteed to have observed all manipulations
-  // of this pointer/memory. For example, it could have been
-  // passed as a parameter to an opaque function.
-  Escaped
+  enum Kind {
+// If this checker does not model the allocation.
+DoNothing,
+// Reference to allocated memory.
+Allocated,
+// Reference to zero-allocated memory.
+AllocatedOfSizeZero,
+// Reference to released/freed memory.
+Released,
+// The responsibility for freeing resources has transferred from
+// this reference. A relinquished symbol should not be freed.
+Relinquished,
+// We are no longer guaranteed to have observed all manipulations
+// of this pointer/memory. For example, it could have been
+// passed as a parameter to an opaque function.
+Escaped
   };
 
   const Stmt *S;
@@ -75,6 +80,7 @@
 assert(family != AF_None);
   }
 public:
+  bool isDoNothing() const { return K == DoNothing; }
   bool isAllocated() const { return K == Allocated; }
   bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; }
   bool isReleased() const { return K == Released; }
@@ -105,6 +111,9 @@
   static RefState getEscaped(const RefState *RS) {
 return RefState(Escaped, RS->getStmt(), RS->getAllocationFamily());
   }
+  static RefState getDoNothing(const Stmt *S) {
+return RefState(DoNothing, S, AF_NotModeled);
+  }
 
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddInteger(K);
@@ -115,6 +124,7 @@
   void dump(raw_ostream ) const {
 switch (static_cast(K)) {
 #define CASE(ID) case ID: OS << #ID; break;
+CASE(DoNothing)
 CASE(Allocated)
 CASE(AllocatedOfSizeZero)
 CASE(Released)
@@ -359,6 +369,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -877,6 +890,9 @@
   State = ProcessZeroAllocation(C, CE, 0, State);
   State = ProcessZeroAllocation(C, CE, 1, State);
 } else if (FunI == II_free || FunI == II_g_free || FunI == II_kfree) {
+  if (isNotModeled(CE, C))
+return;
+
   State = FreeMemAux(C, CE, State, 0, false, 

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D64680#1584076 , @NoQ wrote:

> Change of plans: let's suppress the warning when our `free()` is done within 
> the function that has `__isl_take` in its definition. So, like, ascend the 
> chain of location contexts and check your callers when you're about to mark 
> the pointer as released. If any of the callers contain `__isl_take`, mark it 
> as escaped instead.


I think if the `__isl_*` macro is in use it should be used in the immediate 
StackFrame. Btw: some magic happened and with the previous approach we did not 
suppress some reports, now we do. Thanks!




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:56-58
+// If this checker does not model the allocation.
+DoNothing,
+// Reference to allocated memory.

NoQ wrote:
> We already have `Escaped`, it's the same thing in practice.
It is more strict than `Escaped`, also it made for the purpose of 
`PSK_EscapeOther` to force out we lost the entire pointer and do not make false 
warnings of use-after-free.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2937-2942
+  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+
+  if (RS && RS->isDoNothing()) {
+BR.markInvalid(getTag(), S);
+return nullptr;
+  }

NoQ wrote:
> {F9551724}
I wanted to make it bulletproof, but your meme-proof is way more better.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Oh damn, i just realized that this way we track much more pointers than before, 
because we cannot restrict ourselves to pointers that have been explicitly 
malloc()ed during analysis. After all, we don't need to see the allocation site 
to diagnose use-after-free.

I'm afraid that it's going to be too many pointers.

Change of plans: let's suppress the warning when our `free()` is done within 
the function that has `__isl_take` in its definition. So, like, ascend the 
chain of location contexts and check your callers when you're about to mark the 
pointer as released. If any of the callers contain `__isl_take`, mark it as 
escaped instead.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:56-58
+// If this checker does not model the allocation.
+DoNothing,
+// Reference to allocated memory.

We already have `Escaped`, it's the same thing in practice.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2937-2942
+  const Stmt *S = PathDiagnosticLocation::getStmt(N);
+
+  if (RS && RS->isDoNothing()) {
+BR.markInvalid(getTag(), S);
+return nullptr;
+  }

{F9551724}


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64680/new/

https://reviews.llvm.org/D64680



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

[PATCH] D64680: [analyzer] MallocChecker: Prevent Integer Set Library false positives

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

Integer Set Library using retain-count based allocation which is not
modeled in MallocChecker.


Repository:
  rC Clang

https://reviews.llvm.org/D64680

Files:
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/test/Analysis/retain-count-alloc.cpp

Index: clang/test/Analysis/retain-count-alloc.cpp
===
--- /dev/null
+++ clang/test/Analysis/retain-count-alloc.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix.Malloc \
+// RUN:  -verify %s
+
+// expected-no-diagnostics: We do not model Integer Set Library's retain-count
+//  based allocation.
+
+#define __isl_take
+#define __isl_keep
+
+struct Object { int Ref; };
+void free(void *);
+
+Object *copyObj(__isl_keep Object *O) {
+  O->Ref++;
+  return O;
+}
+
+void freeObj(__isl_take Object *O) {
+  if (--O->Ref > 0)
+return;
+
+  free(O);
+}
+
+void useAfterFree(__isl_take Object *A) {
+  if (!A)
+return;
+
+  Object *B = copyObj(A);
+  freeObj(B);
+
+  A->Ref = 13;
+  // no-warning: 'Use of memory after it is freed' was here.
+}
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/ParentMap.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -41,6 +42,7 @@
 // Used to check correspondence between allocators and deallocators.
 enum AllocationFamily {
   AF_None,
+  AF_NotModeled,
   AF_Malloc,
   AF_CXXNew,
   AF_CXXNewArray,
@@ -50,19 +52,22 @@
 };
 
 class RefState {
-  enum Kind { // Reference to allocated memory.
-  Allocated,
-  // Reference to zero-allocated memory.
-  AllocatedOfSizeZero,
-  // Reference to released/freed memory.
-  Released,
-  // The responsibility for freeing resources has transferred from
-  // this reference. A relinquished symbol should not be freed.
-  Relinquished,
-  // We are no longer guaranteed to have observed all manipulations
-  // of this pointer/memory. For example, it could have been
-  // passed as a parameter to an opaque function.
-  Escaped
+  enum Kind {
+// If this checker does not model the allocation.
+DoNothing,
+// Reference to allocated memory.
+Allocated,
+// Reference to zero-allocated memory.
+AllocatedOfSizeZero,
+// Reference to released/freed memory.
+Released,
+// The responsibility for freeing resources has transferred from
+// this reference. A relinquished symbol should not be freed.
+Relinquished,
+// We are no longer guaranteed to have observed all manipulations
+// of this pointer/memory. For example, it could have been
+// passed as a parameter to an opaque function.
+Escaped
   };
 
   const Stmt *S;
@@ -75,6 +80,7 @@
 assert(family != AF_None);
   }
 public:
+  bool isDoNothing() const { return K == DoNothing; }
   bool isAllocated() const { return K == Allocated; }
   bool isAllocatedOfSizeZero() const { return K == AllocatedOfSizeZero; }
   bool isReleased() const { return K == Released; }
@@ -105,6 +111,9 @@
   static RefState getEscaped(const RefState *RS) {
 return RefState(Escaped, RS->getStmt(), RS->getAllocationFamily());
   }
+  static RefState getDoNothing(const Stmt *S) {
+return RefState(DoNothing, S, AF_NotModeled);
+  }
 
   void Profile(llvm::FoldingSetNodeID ) const {
 ID.AddInteger(K);
@@ -115,6 +124,7 @@
   void dump(raw_ostream ) const {
 switch (static_cast(K)) {
 #define CASE(ID) case ID: OS << #ID; break;
+CASE(DoNothing)
 CASE(Allocated)
 CASE(AllocatedOfSizeZero)
 CASE(Released)
@@ -359,6 +369,9 @@
   /// Check if the memory associated with this symbol was released.
   bool isReleased(SymbolRef Sym, CheckerContext ) const;
 
+  /// Check whether we do not model the memory allocation.
+  bool isNotModeled(const CallExpr *CE, CheckerContext ) const;
+
   bool checkUseAfterFree(SymbolRef Sym, CheckerContext , const Stmt *S) const;
 
   void checkUseZeroAllocated(SymbolRef Sym, CheckerContext ,
@@ -822,13 +835,16 @@
 }
 
 void MallocChecker::checkPostStmt(const CallExpr *CE, CheckerContext ) const {
-  if (C.wasInlined)
-return;
-
   const FunctionDecl *FD = C.getCalleeDecl(CE);
   if (!FD)