[analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

[Szelethus]

Summary

Since its important to know whether a function frees memory (even if its a reallocating function!), I used two CallDescriptionMaps to merge all CallDescriptions into it. MemFunctionInfoTy no longer makes sense, it may never have, but for now, it would be more of a distraction then anything else.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

890

It may be more precise to call this isCMemCall, because that's the question it answers.

1487

Ugh, no idea whats happening here, I'll be honest. I sweat to dig a bit deeper to find out, though I didn't want to bother delaying the publishing this patch for a feature not documented, or used anywhere.

[NoQ]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

378–379

Whenever i see a (CE, State) pair, it screams CallEvent to me. That said, i'm worried that State in these callbacks isn't necessarily equal to C.getState() (the latter, by the way, is always equal to the CallEvent's .getState() - that's a relief, right?), so if you'll ever be in the mood to check that, that'd be great :)

397–398

I think alloca deserves CDF_MaybeBuiltin. This would also probably allow you to remove the second line.

402–404

These are also builtins.

407–408

And these too.

1487

I think this is about the (lack of) support for __attribute__((malloc)).

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

378–379

It should be always equal to it. I'll change it.

397–398

Actually, BuiltinFunctionChecker uses evalCall to create an AllocaRegion for __builtin_alloca. I spent an hour when writing this patch to track a crash down when I initially made this CDF_MaybeBuiltin :)

[NoQ]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

397–398

Oh misery.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

378–379

Hmmm, I tried making this take a (CallEvent, CheckerContext), but it bloated the code for little gain, since every handler function would start with the retrieval of the state and the call expression. That said, I could cascade these down to FreeMemAux, MallocMemAux, etc, to also take this pair instead, but I'll be honest, I don't see point until something actually breaks.

[Szelethus]

Rebase.

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

1487

This should be an NFC change, and this part of the code will have to be rewritten anyways if we want a more serious annotation guided analysis.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

378–379

This is the standard way in the checkers: almost every handler function starts with the retrieval of the state from the CheckerContext. The only reason for an extra State parameter is that sometimes we create more states in the lower level functions but only add the final one to the CheckerContext as a new transition. Does something like this happen here?

877

Maybe return !!FreeingMemFnMap.lookup(Call)?

891

Same here: why not just return FreeingMemFnMap.lookup(Call) || NonFreeingMemFn.lookup(Call)?

[NoQ]

I think my strdup comments weren't addressed >.<

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

378–379

I agree with @baloghadamsoftware here. If you pass State explicitly, it looks like an indication for the reader that this State may be different from C.getState(). If in practice they're the same, i'd rather do C.getState() in every method than confuse the reader.

Also do you remember what makes us query CallExpr so often? Given that CallEvent includes CallExpr, we should be able to expose everything we need as CallEvent methods. Say, we should be able to replace MallocMemAux(C, CE, CE->getArg(0), ...) with MallocMemAux(C, Call, Call.getArgExpr(0), ...) and only do Call.getOriginExpr() once when we report a bug.

[baloghadamsoftware]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

877

I think the code changed after I wrote this comment, it is not valid anymore.

891

Here as well. Disregard these comments.

[Szelethus]

I've spent a lot of time on this code, and I don't think it'd be a great idea to do everything in a single step. Collapsing CallExpr, ProgramState into CallEvent is a shockingly a non-trivial task. While I'm happy to not commit this patch before uploading a solution to that problem, I'd prefer (and I think you would too!) to do them in followup patches.

The point of this revision was to drop in CallDescriptionMap -- I like to think that it introduces no new problems, it just doesn't solve many existing ones :^)

edit: I will soon address the inlines and other comments with an update

[Szelethus]

Address inlines.

[Szelethus]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

378–379

I'll upload followups where I'm addressing these issues -- to keep things simple, I decided against increasing this patch's scope. I won't commit until those are also accepted.

[balazske]

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

358

The CHECK_FN could be used even here?

template <bool ShouldFreeOnFail>
CHECK_FN(checkRealloc)

1215

If these cases are exclusive the code looks better when else statements are added?

[Charusso]

I wish for a third map, something like ReallocationMap. Other than that it is a great direction, I love it. Thanks!

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

358

I think about the opposite, passing around arguments by templates is not cool. They meant to be arguments.

This type of callback simply could be a third CallDescriptionMap for future profeness.

[Szelethus]

In D68165#1902702, @Charusso wrote:

I wish for a third map, something like ReallocationMap. Other than that it is a great direction, I love it. Thanks!

Hah, that is a neat idea.

[NoQ]

Looks wonderful, thanks!

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

358

passing around arguments by templates is not cool. They meant to be arguments.

+1! It's accidental that these values are currently known at compile time.

1215

Yup, or even better, returns, because this code is very fragile because it'll do horrible things if we accidentally invoke two callbacks at once.

[ldionne]

Closed by commit rG703a1b8caf09: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap (authored by ldionne, committed by Szelethus). · Explain Why

This is really strange -- it looks like 703a1b8caf09 was attributed to me in git, but I have nothing to do with this change. How did you commit the patch? Is that some Phabricator bug?

[NoQ]

$ git show --pretty=full 703a1b8caf09

commit 703a1b8caf09a5262a45c2179b8131922f71cf25
Author: Louis Dionne <[e-mail]>
Commit: Kirstóf Umann <[e-mail]>

    [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap

    ...

Not a phabricator bug. Maybe @Szelethus's local git somehow got in a weird state(?)

[Szelethus]

In D68165#1949954, @ldionne wrote:

Closed by commit rG703a1b8caf09: [analyzer][MallocChecker][NFC] Split checkPostCall up, deploy CallDescriptionMap (authored by ldionne, committed by Szelethus). · Explain Why

This is really strange -- it looks like 703a1b8caf09 was attributed to me in git, but I have nothing to do with this change. How did you commit the patch? Is that some Phabricator bug?

Ugh, I squashed my local commits and pushed, it seems like for some reason it made you the author... it has happened to me before (always with you, for some reason), but was able to notice and correct it. There is little we can do about this, right?

In D68165#1950212, @NoQ wrote:

Not a phabricator bug. Maybe @Szelethus's local git somehow got in a weird state(?)

Its been a source of endless misery :)

[ldionne]

In D68165#1950357, @Szelethus wrote:

Ugh, I squashed my local commits and pushed, it seems like for some reason it made you the author... it has happened to me before (always with you, for some reason), but was able to notice and correct it.

Always with me? Maybe you should double-check that you don't have something weird in your git config, aliases or other? I don't know *why* it would be me in particular, but the fact that it is consistently the same person is kind of suspicious.

[Szelethus]

In D68165#1950451, @ldionne wrote:

Always with me? Maybe you should double-check that you don't have something weird in your git config, aliases or other? I don't know *why* it would be me in particular, but the fact that it is consistently the same person is kind of suspicious.

Will do. Sorry for the trouble!

clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <climits>
 #include <utility>
 
enum AllocationFamily {
   AF_Malloc,
   AF_CXXNew,
   AF_CXXNewArray,
   AF_IfNameIndex,
   AF_Alloca,
   AF_InnerBuffer
 };
 
-struct MemFunctionInfoTy;
-
 } // end of anonymous namespace
 
 /// Print names of allocators and deallocators.
 ///
 /// \returns true on success.
 static bool printMemFnName(raw_ostream &os, CheckerContext &C, const Expr *E);
 
 /// Print expected name of an allocator based on the deallocator's family

 } // end of anonymous namespace
 
 REGISTER_MAP_WITH_PROGRAMSTATE(ReallocPairs, SymbolRef, ReallocPair)
 
 //===----------------------------------------------------------------------===//
 // Kinds of memory operations, information about resource managing functions.
 //===----------------------------------------------------------------------===//
 
-namespace {
-
-struct MemFunctionInfoTy {
-  /// The value of the MallocChecker:Optimistic is stored in this variable.
-  ///
-  /// In pessimistic mode, the checker assumes that it does not know which
-  /// functions might free the memory.
-  /// In optimistic mode, the checker assumes that all user-defined functions
-  /// which might free a pointer are annotated.
-  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
-
-  CallDescription CD_alloca{{"alloca"}, 1}, CD_win_alloca{{"_alloca"}, 1},
-      CD_malloc{{"malloc"}, 1}, CD_free{{"free"}, 1},
-      CD_realloc{{"realloc"}, 2}, CD_calloc{{"calloc"}, 2},
-      CD_valloc{{"valloc"}, 1}, CD_reallocf{{"reallocf"}, 2},
-      CD_strndup{{"strndup"}, 2}, CD_strdup{{"strdup"}, 1},
-      CD_win_strdup{{"_strdup"}, 1}, CD_kmalloc{{"kmalloc"}, 2},
-      CD_if_nameindex{{"if_nameindex"}, 1},
-      CD_if_freenameindex{{"if_freenameindex"}, 1}, CD_wcsdup{{"wcsdup"}, 1},
-      CD_win_wcsdup{{"_wcsdup"}, 1}, CD_kfree{{"kfree"}, 2},
-      CD_g_malloc{{"g_malloc"}, 1}, CD_g_malloc0{{"g_malloc0"}, 1},
-      CD_g_realloc{{"g_realloc"}, 2}, CD_g_try_malloc{{"g_try_malloc"}, 1},
-      CD_g_try_malloc0{{"g_try_malloc0"}, 1},
-      CD_g_try_realloc{{"g_try_realloc"}, 2}, CD_g_free{{"g_free"}, 1},
-      CD_g_memdup{{"g_memdup"}, 2}, CD_g_malloc_n{{"g_malloc_n"}, 2},
-      CD_g_malloc0_n{{"g_malloc0_n"}, 2}, CD_g_realloc_n{{"g_realloc_n"}, 3},
-      CD_g_try_malloc_n{{"g_try_malloc_n"}, 2},
-      CD_g_try_malloc0_n{{"g_try_malloc0_n"}, 2},
-      CD_g_try_realloc_n{{"g_try_realloc_n"}, 3};
-
-  void initIdentifierInfo(ASTContext &C) const;
-
-  bool isMemFunction(const CallEvent &Call) const;
-  bool isCMemFunction(const CallEvent &Call) const;
-  bool isCMemFreeFunction(const CallEvent &Call) const;
-  bool isCMemAllocFunction(const CallEvent &Call) const;
-};
-} // end of anonymous namespace
-
 /// Tells if the callee is one of the builtin new/delete operators, including
 /// placement operators and other standard overloads.
 static bool isStandardNewDelete(const FunctionDecl *FD);
 static bool isStandardNewDelete(const CallEvent &Call) {
   if (!Call.getDecl())
     return false;
   return isStandardNewDelete(cast<FunctionDecl>(Call.getDecl()));
 }
 
 //===----------------------------------------------------------------------===//
 // Definition of the MallocChecker class.
 //===----------------------------------------------------------------------===//
 
 namespace {
 
 class MallocChecker
     : public Checker<check::DeadSymbols, check::PointerEscape,
                      check::ConstPointerEscape, check::PreStmt<ReturnStmt>,
                      check::EndFunction, check::PreCall, check::PostCall,
                      check::PostStmt<CXXNewExpr>, check::NewAllocator,
                      check::PreStmt<CXXDeleteExpr>, check::PostStmt<BlockExpr>,
                      check::PostObjCMessage, check::Location, eval::Assume> {
 public:
-  MemFunctionInfoTy MemFunctionInfo;
+  /// In pessimistic mode, the checker assumes that it does not know which
+  /// functions might free the memory.
+  /// In optimistic mode, the checker assumes that all user-defined functions
+  /// which might free a pointer are annotated.
+  DefaultBool ShouldIncludeOwnershipAnnotatedFunctions;
 
   /// Many checkers are essentially built into this one, so enabling them will
   /// make MallocChecker perform additional modeling and reporting.
   enum CheckKind {
     /// When a subchecker is enabled but MallocChecker isn't, model memory
     /// management but do not emit warnings emitted with MallocChecker only
     /// enabled.
     CK_MallocChecker,
private:
   mutable std::unique_ptr<BugType> BT_Leak[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BT_UseFree[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BT_BadFree[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BT_FreeAlloca[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BT_MismatchedDealloc;
   mutable std::unique_ptr<BugType> BT_OffsetFree[CK_NumCheckKinds];
   mutable std::unique_ptr<BugType> BT_UseZerroAllocated[CK_NumCheckKinds];
 
+#define CHECK_FN(NAME)                                                         \
+  void NAME(CheckerContext &C, const CallExpr *CE, ProgramStateRef State) const;
+
+  template <bool ShouldFreeOnFail>
+  void checkRealloc(CheckerContext &C, const CallExpr *CE,
+                    ProgramStateRef State) const;
+
+  CHECK_FN(checkBasicAlloc)
+  CHECK_FN(checkKernelMalloc)
+  CHECK_FN(checkMalloc)
+  CHECK_FN(checkCalloc)
+  CHECK_FN(checkFree)
+  CHECK_FN(checkAlloca)
+  CHECK_FN(checkStrdup)
+  CHECK_FN(checkIfNameIndex)
+  CHECK_FN(checkIfFreeNameIndex)
+  CHECK_FN(checkCXXNewOrCXXDelete)
+  CHECK_FN(checkGMalloc0)
+  CHECK_FN(checkGMemdup)
+  CHECK_FN(checkGMallocN)
+  CHECK_FN(checkGMallocN0)
+  CHECK_FN(checkReallocN)
+  CHECK_FN(checkOwnershipAttr)
+
+  using CheckFn = void (MallocChecker::*)(CheckerContext &C, const CallExpr *CE,
+                                          ProgramStateRef State) const;
+
+  CallDescriptionMap<CheckFn> FreeingMemFnMap{
+      {{"realloc", 2}, &MallocChecker::checkRealloc<false>},
+      {{"reallocf", 2}, &MallocChecker::checkRealloc<true>},
+      {{"g_realloc", 2}, &MallocChecker::checkRealloc<false>},
+      {{"g_try_realloc", 2}, &MallocChecker::checkRealloc<false>},
+      {{"g_realloc_n", 3}, &MallocChecker::checkReallocN},
+      {{"g_try_realloc_n", 3}, &MallocChecker::checkReallocN},
+      {{"free", 1}, &MallocChecker::checkFree},
+      {{"if_freenameindex", 1}, &MallocChecker::checkIfFreeNameIndex},
+      {{"kfree", 2}, &MallocChecker::checkFree},
+      {{"g_free", 1}, &MallocChecker::checkFree},
+  };
+
+  bool isFreeingCall(const CallEvent &Call) const;
+
+  CallDescriptionMap<CheckFn> NonFreeingMemFnMap{
+      {{"alloca", 1}, &MallocChecker::checkAlloca},
+      {{"_alloca", 1}, &MallocChecker::checkAlloca},
+      {{"malloc", 1}, &MallocChecker::checkMalloc},
+      {{"calloc", 2}, &MallocChecker::checkCalloc},
+      {{"valloc", 1}, &MallocChecker::checkBasicAlloc},
+      {{"strndup", 2}, &MallocChecker::checkStrdup},
+      {{"strdup", 1}, &MallocChecker::checkStrdup},
+      {{"_strdup", 1}, &MallocChecker::checkStrdup},
+      {{"kmalloc", 2}, &MallocChecker::checkKernelMalloc},
+      {{"if_nameindex", 1}, &MallocChecker::checkIfNameIndex},
+      {{"wcsdup", 1}, &MallocChecker::checkStrdup},
+      {{"_wcsdup", 1}, &MallocChecker::checkStrdup},
+      {{"g_malloc", 1}, &MallocChecker::checkMalloc},
+      {{"g_malloc0", 1}, &MallocChecker::checkGMalloc0},
+      {{"g_try_malloc", 1}, &MallocChecker::checkMalloc},
+      {{"g_try_malloc0", 1}, &MallocChecker::checkGMalloc0},
+      {{"g_memdup", 2}, &MallocChecker::checkGMemdup},
+      {{"g_malloc_n", 2}, &MallocChecker::checkGMallocN},
+      {{"g_malloc0_n", 2}, &MallocChecker::checkGMallocN0},
+      {{"g_try_malloc_n", 2}, &MallocChecker::checkGMallocN},
+      {{"g_try_malloc0_n", 2}, &MallocChecker::checkGMallocN0},
+  };
+
+  bool isMemCall(const CallEvent &Call) const;
+
   // TODO: Remove mutable by moving the initializtaion to the registry function.
   mutable Optional<uint64_t> KernelZeroFlagVal;
 
   /// Process C++ operator new()'s allocation, which is the part of C++
   /// new-expression that goes before the constructor.
   template <AllocationFamily Family>
   void processNewAllocation(const CXXNewExpr *NE, CheckerContext &C,
                             SVal Target) const;
#define CHECK_FN(NAME)                                                         \
   ///
   /// \param [in] CE The expression that reallocated memory
   /// \param [in] ShouldFreeOnFail Whether if reallocation fails, the supplied
   ///   memory should be freed.
   /// \param [in] State The \c ProgramState right before reallocation.
   /// \param [in] SuffixWithN Whether the reallocation function we're modeling
   ///   has an '_n' suffix, such as g_realloc_n.
   /// \returns The ProgramState right after reallocation.
-  template <AllocationFamily Family>
+  template <AllocationFamily Family, bool ShouldFreeOnFail,
+            bool SuffixWithN = false>
   ProgramStateRef ReallocMemAux(CheckerContext &C, const CallExpr *CE,
-                                bool ShouldFreeOnFail, ProgramStateRef State,
-                                bool SuffixWithN = false) const;
+                                ProgramStateRef State) const;
 
   /// Evaluates the buffer size that needs to be allocated.
   ///
   /// \param [in] Blocks The amount of blocks that needs to be allocated.
   /// \param [in] BlockBytes The size of a block.
   /// \returns The symbolic value of \p Blocks * \p BlockBytes.
   static SVal evalMulForBufferSize(CheckerContext &C, const Expr *Blocks,
                                    const Expr *BlockBytes);
public:
 
   bool VisitSymbol(SymbolRef sym) override {
     state = state->remove<RegionState>(sym);
     return true;
   }
 };
 } // end anonymous namespace
 
-//===----------------------------------------------------------------------===//
-// Methods of MemFunctionInfoTy.
-//===----------------------------------------------------------------------===//
+static bool isStandardNewDelete(const FunctionDecl *FD) {
+  if (!FD)
+    return false;
 
-bool MemFunctionInfoTy::isMemFunction(const CallEvent &Call) const {
-  return isCMemFunction(Call) || isStandardNewDelete(Call);
-}
+  OverloadedOperatorKind Kind = FD->getOverloadedOperator();
+  if (Kind != OO_New && Kind != OO_Array_New && Kind != OO_Delete &&
+      Kind != OO_Array_Delete)
+    return false;
 
-bool MemFunctionInfoTy::isCMemFunction(const CallEvent &Call) const {
-  return isCMemFreeFunction(Call) || isCMemAllocFunction(Call);
+  // This is standard if and only if it's not defined in a user file.
+  SourceLocation L = FD->getLocation();
+  // If the header for operator delete is not included, it's still defined
+  // in an invalid source location. Check to make sure we don't crash.
+  return !L.isValid() ||
+         FD->getASTContext().getSourceManager().isInSystemHeader(L);
 }
 
-bool MemFunctionInfoTy::isCMemFreeFunction(const CallEvent &Call) const {
-  if (Call.isCalled(CD_free, CD_realloc, CD_reallocf, CD_g_free, CD_kfree))
-    return true;
+//===----------------------------------------------------------------------===//
+// Methods of MallocChecker and MallocBugVisitor.
+//===----------------------------------------------------------------------===//
 
-  if (Call.isCalled(CD_if_freenameindex))
+bool MallocChecker::isFreeingCall(const CallEvent &Call) const {
+  if (FreeingMemFnMap.lookup(Call))
     return true;
 
-  if (!ShouldIncludeOwnershipAnnotatedFunctions)
-    return false;
-
   const auto *Func = dyn_cast<FunctionDecl>(Call.getDecl());
   if (Func && Func->hasAttrs()) {
     for (const auto *I : Func->specific_attrs<OwnershipAttr>()) {
       OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind();
       if (OwnKind == OwnershipAttr::Takes || OwnKind == OwnershipAttr::Holds)
         return true;
     }
   }
   return false;
 }
 
-bool MemFunctionInfoTy::isCMemAllocFunction(const CallEvent &Call) const {
-  if (Call.isCalled(CD_malloc, CD_realloc, CD_reallocf, CD_calloc, CD_valloc,
-                    CD_strdup, CD_win_strdup, CD_strndup, CD_wcsdup,
-                    CD_win_wcsdup, CD_kmalloc, CD_g_malloc, CD_g_malloc0,
-                    CD_g_realloc, CD_g_try_malloc, CD_g_try_malloc0,
-                    CD_g_try_realloc, CD_g_memdup, CD_g_malloc_n,
-                    CD_g_malloc0_n, CD_g_realloc_n, CD_g_try_malloc_n,
-                    CD_g_try_malloc0_n, CD_g_try_realloc_n))
-    return true;
-
-  if (Call.isCalled(CD_if_nameindex))
-    return true;
-
-  if (Call.isCalled(CD_alloca, CD_win_alloca))
+bool MallocChecker::isMemCall(const CallEvent &Call) const {
+  if (FreeingMemFnMap.lookup(Call) || NonFreeingMemFnMap.lookup(Call))
     return true;
 
   if (!ShouldIncludeOwnershipAnnotatedFunctions)
     return false;
 
   const auto *Func = dyn_cast<FunctionDecl>(Call.getDecl());
-  if (Func && Func->hasAttrs()) {
-    for (const auto *I : Func->specific_attrs<OwnershipAttr>()) {
-      OwnershipAttr::OwnershipKind OwnKind = I->getOwnKind();
-      if (OwnKind == OwnershipAttr::Returns)
-        return true;
-    }
-  }
+  return Func && Func->hasAttr<OwnershipAttr>();
 
-  return false;
-}
-
-static bool isStandardNewDelete(const FunctionDecl *FD) {
-  if (!FD)
-    return false;
-
-  OverloadedOperatorKind Kind = FD->getOverloadedOperator();
-  if (Kind != OO_New && Kind != OO_Array_New && Kind != OO_Delete &&
-      Kind != OO_Array_Delete)
-    return false;
-
-  // This is standard if and only if it's not defined in a user file.
-  SourceLocation L = FD->getLocation();
-  // If the header for operator delete is not included, it's still defined
-  // in an invalid source location. Check to make sure we don't crash.
-  return !L.isValid() ||
-         FD->getASTContext().getSourceManager().isInSystemHeader(L);
-}
-
-//===----------------------------------------------------------------------===//
-// Methods of MallocChecker and MallocBugVisitor.
-//===----------------------------------------------------------------------===//
-
 llvm::Optional<ProgramStateRef> MallocChecker::performKernelMalloc(
   const CallExpr *CE, CheckerContext &C, const ProgramStateRef &State) const {
   // 3-argument malloc(), as commonly used in {Free,Net,Open}BSD Kernels:
   //
   // void *malloc(unsigned long size, struct malloc_type *mtp, int flags);
   //
   // One of the possible flags is M_ZERO, which means 'give me back an
   // allocation which is already zeroed', like calloc.
SVal MallocChecker::evalMulForBufferSize(CheckerContext &C, const Expr *Blocks,
   SVal BlocksVal = C.getSVal(Blocks);
   SVal BlockBytesVal = C.getSVal(BlockBytes);
   ProgramStateRef State = C.getState();
   SVal TotalSize = SB.evalBinOp(State, BO_Mul, BlocksVal, BlockBytesVal,
                                 SB.getContext().getSizeType());
   return TotalSize;
 }
 
-void MallocChecker::checkPostCall(const CallEvent &Call,
-                                  CheckerContext &C) const {
-  if (C.wasInlined)
-    return;
-
-  const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
-  if (!CE)
-    return;
-
-  const FunctionDecl *FD = C.getCalleeDecl(CE);
-  if (!FD || FD->getKind() != Decl::Function)
-    return;
-
-  ProgramStateRef State = C.getState();
-  bool IsKnownToBeAllocatedMemory = false;
-
-  if (Call.isCalled(MemFunctionInfo.CD_malloc, MemFunctionInfo.CD_g_malloc,
-                    MemFunctionInfo.CD_g_try_malloc)) {
-    switch (CE->getNumArgs()) {
-    default:
-      return;
-    case 1:
-      State =
-          MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(), State);
-      State = ProcessZeroAllocCheck(C, CE, 0, State);
-      break;
-    case 2:
-      State =
-          MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(), State);
-      break;
-    case 3:
-      llvm::Optional<ProgramStateRef> MaybeState =
-          performKernelMalloc(CE, C, State);
-      if (MaybeState.hasValue())
-        State = MaybeState.getValue();
-      else
-        State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(),
-                                        State);
-      break;
-    }
-  } else if (Call.isCalled(MemFunctionInfo.CD_kmalloc)) {
-    if (CE->getNumArgs() < 1)
-      return;
-    llvm::Optional<ProgramStateRef> MaybeState =
-        performKernelMalloc(CE, C, State);
-    if (MaybeState.hasValue())
-      State = MaybeState.getValue();
-    else
-      State =
-          MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(), State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_valloc)) {
-    if (CE->getNumArgs() < 1)
-      return;
+void MallocChecker::checkBasicAlloc(CheckerContext &C, const CallExpr *CE,
+                                    ProgramStateRef State) const {
+  State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(), State);
+  C.addTransition(State);
+
+void MallocChecker::checkKernelMalloc(CheckerContext &C, const CallExpr *CE,
+                                      ProgramStateRef State) const {
+  C.addTransition(State);
+}
+
+void MallocChecker::checkMalloc(CheckerContext &C, const CallExpr *CE,
+                                ProgramStateRef State) const {
+  switch (CE->getNumArgs()) {
+  default:
+    return;
+  case 1:
+    checkBasicAlloc(C, CE, State);
+  case 2:
     State =
         MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), UndefinedVal(), State);
-    State = ProcessZeroAllocCheck(C, CE, 0, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_realloc,
-                           MemFunctionInfo.CD_g_realloc,
-                           MemFunctionInfo.CD_g_try_realloc)) {
-    State = ReallocMemAux<AF_Malloc>(C, CE, /*ShouldFreeOnFail*/ false, State);
-    State = ProcessZeroAllocCheck(C, CE, 1, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_reallocf)) {
-    State = ReallocMemAux<AF_Malloc>(C, CE, /*ShouldFreeOnFail*/ true, State);
-    State = ProcessZeroAllocCheck(C, CE, 1, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_calloc)) {
-    State = CallocMem(C, CE, State);
-    State = ProcessZeroAllocCheck(C, CE, 0, State);
-    State = ProcessZeroAllocCheck(C, CE, 1, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_free, MemFunctionInfo.CD_g_free,
-                           MemFunctionInfo.CD_kfree)) {
-    if (suppressDeallocationsInSuspiciousContexts(CE, C))
-      return;
-    State = FreeMemAux<AF_Malloc>(C, CE, State, 0, false,
-                                  IsKnownToBeAllocatedMemory);
-  } else if (Call.isCalled(
-                 MemFunctionInfo.CD_strdup, MemFunctionInfo.CD_win_strdup,
-                 MemFunctionInfo.CD_wcsdup, MemFunctionInfo.CD_win_wcsdup)) {
-    State = MallocUpdateRefState<AF_Malloc>(C, CE, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_strndup)) {
-    State = MallocUpdateRefState<AF_Malloc>(C, CE, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_alloca,
-                           MemFunctionInfo.CD_win_alloca)) {
-    if (CE->getNumArgs() < 1)
-      return;
-    State =
-        MallocMemAux<AF_Alloca>(C, CE, CE->getArg(0), UndefinedVal(), State);
-    State = ProcessZeroAllocCheck(C, CE, 0, State);
-  } else if (isStandardNewDelete(Call)) {
-    const auto *FD = cast<FunctionDecl>(Call.getDecl());
-    // Process direct calls to operator new/new[]/delete/delete[] functions
-    // as distinct from new/new[]/delete/delete[] expressions that are
-    // processed by the checkPostStmt callbacks for CXXNewExpr and
-    // CXXDeleteExpr.
-    switch (FD->getOverloadedOperator()) {
-    case OO_New:
-      State =
-          MallocMemAux<AF_CXXNew>(C, CE, CE->getArg(0), UndefinedVal(), State);
-      State = ProcessZeroAllocCheck(C, CE, 0, State);
-      break;
-    case OO_Array_New:
-      State = MallocMemAux<AF_CXXNewArray>(C, CE, CE->getArg(0), UndefinedVal(),
-                                           State);
-      State = ProcessZeroAllocCheck(C, CE, 0, State);
-      break;
-    case OO_Delete:
-      State = FreeMemAux<AF_CXXNew>(C, CE, State, 0, false,
-                                    IsKnownToBeAllocatedMemory);
-      break;
-    case OO_Array_Delete:
-      State = FreeMemAux<AF_CXXNewArray>(C, CE, State, 0, false,
-                                         IsKnownToBeAllocatedMemory);
-      break;
-    default:
-      llvm_unreachable("not a new/delete operator");
-    }
-  } else if (Call.isCalled(MemFunctionInfo.CD_if_nameindex)) {
-    // Should we model this differently? We can allocate a fixed number of
-    // elements with zeros in the last one.
-    State =
-        MallocMemAux<AF_IfNameIndex>(C, CE, UnknownVal(), UnknownVal(), State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_if_freenameindex)) {
-    State = FreeMemAux<AF_IfNameIndex>(C, CE, State, 0, false,
-                                       IsKnownToBeAllocatedMemory);
-  } else if (Call.isCalled(MemFunctionInfo.CD_g_malloc0,
-                           MemFunctionInfo.CD_g_try_malloc0)) {
-    if (CE->getNumArgs() < 1)
-      return;
-    SValBuilder &svalBuilder = C.getSValBuilder();
-    SVal zeroVal = svalBuilder.makeZeroVal(svalBuilder.getContext().CharTy);
-    State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(0), zeroVal, State);
-    State = ProcessZeroAllocCheck(C, CE, 0, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_g_memdup)) {
-    if (CE->getNumArgs() < 2)
-      return;
-    State =
-        MallocMemAux<AF_Malloc>(C, CE, CE->getArg(1), UndefinedVal(), State);
-    State = ProcessZeroAllocCheck(C, CE, 1, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_g_malloc_n,
-                           MemFunctionInfo.CD_g_try_malloc_n,
-                           MemFunctionInfo.CD_g_malloc0_n,
-                           MemFunctionInfo.CD_g_try_malloc0_n)) {
-    if (CE->getNumArgs() < 2)
-      return;
-    SVal Init = UndefinedVal();
-    if (Call.isCalled(MemFunctionInfo.CD_g_malloc0_n,
-                      MemFunctionInfo.CD_g_try_malloc0_n)) {
-      SValBuilder &SB = C.getSValBuilder();
-      Init = SB.makeZeroVal(SB.getContext().CharTy);
-    }
-    SVal TotalSize = evalMulForBufferSize(C, CE->getArg(0), CE->getArg(1));
-    State = MallocMemAux<AF_Malloc>(C, CE, TotalSize, Init, State);
-    State = ProcessZeroAllocCheck(C, CE, 0, State);
-    State = ProcessZeroAllocCheck(C, CE, 1, State);
-  } else if (Call.isCalled(MemFunctionInfo.CD_g_realloc_n,
-                           MemFunctionInfo.CD_g_try_realloc_n)) {
-    if (CE->getNumArgs() < 3)
-      return;
-    State = ReallocMemAux<AF_Malloc>(C, CE, /*ShouldFreeOnFail*/ false, State,
-                                     /*SuffixWithN*/ true);
-    State = ProcessZeroAllocCheck(C, CE, 1, State);
-    State = ProcessZeroAllocCheck(C, CE, 2, State);
-  }
+    break;
+  case 3:
+    checkKernelMalloc(C, CE, State);
+    return;
+  }
+  C.addTransition(State);
+}
+
+template <bool ShouldFreeOnFail>
+void MallocChecker::checkRealloc(CheckerContext &C, const CallExpr *CE,
+                                 ProgramStateRef State) const {
+  State = ReallocMemAux<AF_Malloc, ShouldFreeOnFail>(C, CE, State);
+  C.addTransition(State);
+}
+
+void MallocChecker::checkCalloc(CheckerContext &C, const CallExpr *CE,
+                                ProgramStateRef State) const {
+  C.addTransition(State);
+}
+
+void MallocChecker::checkFree(CheckerContext &C, const CallExpr *CE,
+                              ProgramStateRef State) const {
+  bool IsKnownToBeAllocatedMemory = false;
+      FreeMemAux<AF_Malloc>(C, CE, State, 0, false, IsKnownToBeAllocatedMemory);
+  C.addTransition(State);
+}
+
+void MallocChecker::checkAlloca(CheckerContext &C, const CallExpr *CE,
+                                ProgramStateRef State) const {
+  State = MallocMemAux<AF_Alloca>(C, CE, CE->getArg(0), UndefinedVal(), State);
+  C.addTransition(State);
+}
+
+void MallocChecker::checkStrdup(CheckerContext &C, const CallExpr *CE,
+                                ProgramStateRef State) const {
+  State = MallocUpdateRefState<AF_Malloc>(C, CE, State);
+
+  C.addTransition(State);
+}
+
+void MallocChecker::checkIfNameIndex(CheckerContext &C, const CallExpr *CE,
+                                     ProgramStateRef State) const {
+  // Should we model this differently? We can allocate a fixed number of
+  // elements with zeros in the last one.
+  State =
+      MallocMemAux<AF_IfNameIndex>(C, CE, UnknownVal(), UnknownVal(), State);
+
+  C.addTransition(State);
+}
+
+void MallocChecker::checkIfFreeNameIndex(CheckerContext &C, const CallExpr *CE,
+                                         ProgramStateRef State) const {
+  bool IsKnownToBeAllocatedMemory = false;
+  State = FreeMemAux<AF_IfNameIndex>(C, CE, State, 0, false,
+                                     IsKnownToBeAllocatedMemory);
+
+  C.addTransition(State);
+}
+
+void MallocChecker::checkCXXNewOrCXXDelete(CheckerContext &C,
+                                           const CallExpr *CE,
+                                           ProgramStateRef State) const {
+  bool IsKnownToBeAllocatedMemory = false;
+
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+
+  C.addTransition(State);
+}
+
+void MallocChecker::checkGMalloc0(CheckerContext &C, const CallExpr *CE,
+                                  ProgramStateRef State) const {
+  C.addTransition(State);
+}
+
+void MallocChecker::checkGMemdup(CheckerContext &C, const CallExpr *CE,
+                                 ProgramStateRef State) const {
+  State = MallocMemAux<AF_Malloc>(C, CE, CE->getArg(1), UndefinedVal(), State);
+  C.addTransition(State);
+}
+
+void MallocChecker::checkGMallocN(CheckerContext &C, const CallExpr *CE,
+                                  ProgramStateRef State) const {
+  SVal TotalSize = evalMulForBufferSize(C, CE->getArg(0), CE->getArg(1));
+  State = MallocMemAux<AF_Malloc>(C, CE, TotalSize, Init, State);
+  State = ProcessZeroAllocCheck(C, CE, 0, State);
+  State = ProcessZeroAllocCheck(C, CE, 1, State);
+  C.addTransition(State);
+
+void MallocChecker::checkGMallocN0(CheckerContext &C, const CallExpr *CE,
+                                   ProgramStateRef State) const {
+  SValBuilder &SB = C.getSValBuilder();
+  SVal Init = SB.makeZeroVal(SB.getContext().CharTy);
+  C.addTransition(State);
+}
+
+void MallocChecker::checkReallocN(CheckerContext &C, const CallExpr *CE,
+                                  ProgramStateRef State) const {
+  State = ReallocMemAux<AF_Malloc, /*ShouldFreeOnFail=*/false,
+                        /*SuffixWithN=*/true>(C, CE, State);
+  C.addTransition(State);
 
-  if (MemFunctionInfo.ShouldIncludeOwnershipAnnotatedFunctions ||
+void MallocChecker::checkOwnershipAttr(CheckerContext &C, const CallExpr *CE,
+                                       ProgramStateRef State) const {
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (ShouldIncludeOwnershipAnnotatedFunctions ||
       ChecksEnabled[CK_MismatchedDeallocatorChecker]) {
     // Check all the attributes, if there are any.
     // There can be multiple of these attributes.
     if (FD->hasAttrs())
       for (const auto *I : FD->specific_attrs<OwnershipAttr>()) {
         switch (I->getOwnKind()) {
         case OwnershipAttr::Returns:
           State = MallocMemReturnsAttr(C, CE, I, State);
           break;
         case OwnershipAttr::Takes:
         case OwnershipAttr::Holds:
           State = FreeMemAttr(C, CE, I, State);
           break;
         }
       }
   }
   C.addTransition(State);
 }
 
+void MallocChecker::checkPostCall(const CallEvent &Call,
+                                  CheckerContext &C) const {
+  if (C.wasInlined)
+    return;
+
+  const auto *CE = dyn_cast_or_null<CallExpr>(Call.getOriginExpr());
+  if (!CE)
+    return;
+
+  const FunctionDecl *FD = C.getCalleeDecl(CE);
+  if (!FD || FD->getKind() != Decl::Function)
+    return;
+
+  ProgramStateRef State = C.getState();
+
+  if (const CheckFn *Callback = FreeingMemFnMap.lookup(Call))
+    (this->**Callback)(C, CE, State);
+
+  if (const CheckFn *Callback = NonFreeingMemFnMap.lookup(Call))
+    (this->**Callback)(C, CE, State);
+
+  if (isStandardNewDelete(Call))
+    checkCXXNewOrCXXDelete(C, CE, State);
+
+  checkOwnershipAttr(C, CE, State);
+}
+
 // Performs a 0-sized allocations check.
 ProgramStateRef MallocChecker::ProcessZeroAllocCheck(
     CheckerContext &C, const Expr *E, const unsigned IndexOfSizeArg,
     ProgramStateRef State, Optional<SVal> RetVal) {
   if (!State)
     return nullptr;
 
   if (!RetVal)

 
 ProgramStateRef
 MallocChecker::MallocMemReturnsAttr(CheckerContext &C, const CallExpr *CE,
                                     const OwnershipAttr *Att,
                                     ProgramStateRef State) const {
   if (!State)
     return nullptr;
 
-  if (Att->getModule()->getName() !=
-      MemFunctionInfo.CD_malloc.getFunctionName())
+  if (Att->getModule()->getName() != "malloc")
     return nullptr;
 
   OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
   if (I != E) {
     return MallocMemAux<AF_Malloc>(C, CE, CE->getArg(I->getASTIndex()),
                                    UndefinedVal(), State);
   }
   return MallocMemAux<AF_Malloc>(C, CE, UnknownVal(), UndefinedVal(), State);

 
 ProgramStateRef MallocChecker::FreeMemAttr(CheckerContext &C,
                                            const CallExpr *CE,
                                            const OwnershipAttr *Att,
                                            ProgramStateRef State) const {
   if (!State)
     return nullptr;
 
-  if (Att->getModule()->getName() !=
-      MemFunctionInfo.CD_malloc.getFunctionName())
+  if (Att->getModule()->getName() != "malloc")
     return nullptr;
 
   bool IsKnownToBeAllocated = false;
 
   for (const auto &Arg : Att->args()) {
     ProgramStateRef StateI = FreeMemAux<AF_Malloc>(
         C, CE, State, Arg.getASTIndex(),
         Att->getOwnKind() == OwnershipAttr::Holds, IsKnownToBeAllocated);
if (ExplodedNode *N = C.generateErrorNode()) {
     auto R = std::make_unique<PathSensitiveBugReport>(*BT_BadFree[*CheckKind],
                                                       Os.str(), N);
     R->markInteresting(MR);
     R->addRange(Range);
     C.emitReport(std::move(R));
   }
 }
 
-template <AllocationFamily Family>
-ProgramStateRef
-MallocChecker::ReallocMemAux(CheckerContext &C, const CallExpr *CE,
-                             bool ShouldFreeOnFail, ProgramStateRef State,
-                             bool SuffixWithN) const {
+template <AllocationFamily Family, bool ShouldFreeOnFail, bool SuffixWithN>
+ProgramStateRef MallocChecker::ReallocMemAux(CheckerContext &C,
+                                             const CallExpr *CE,
+                                             ProgramStateRef State) const {
   if (!State)
     return nullptr;
 
   if (SuffixWithN && CE->getNumArgs() < 3)
     return nullptr;
   else if (CE->getNumArgs() < 2)
     return nullptr;
 
void MallocChecker::checkPreCall(const CallEvent &Call,
   }
 
   // We will check for double free in the post visit.
   if (const AnyFunctionCall *FC = dyn_cast<AnyFunctionCall>(&Call)) {
     const FunctionDecl *FD = FC->getDecl();
     if (!FD)
       return;
 
-    if (ChecksEnabled[CK_MallocChecker] &&
-        (MemFunctionInfo.isCMemFreeFunction(Call)))
+    if (ChecksEnabled[CK_MallocChecker] && isFreeingCall(Call))
       return;
   }
 
   // Check if the callee of a method is deleted.
   if (const CXXInstanceCall *CC = dyn_cast<CXXInstanceCall>(&Call)) {
     SymbolRef Sym = CC->getCXXThisVal().getAsSymbol();
     if (!Sym || checkUseAfterFree(Sym, C, CC->getCXXThisExpr()))
       return;
bool MallocChecker::mayFreeAnyEscapedMemoryOrIsModeledExplicitly(
 
   // At this point the only thing left to handle is straight function calls.
   const FunctionDecl *FD = cast<SimpleFunctionCall>(Call)->getDecl();
   if (!FD)
     return true;
 
   // If it's one of the allocation functions we can reason about, we model
   // its behavior explicitly.
-  if (MemFunctionInfo.isMemFunction(*Call))
+  if (isMemCall(*Call))
     return false;
 
   // If it's not a system call, assume it frees memory.
   if (!Call->isInSystemHeader())
     return true;
 
   // White list the system functions whose arguments escape.
   const IdentifierInfo *II = FD->getIdentifier();
void ento::registerInnerPointerCheckerAux(CheckerManager &mgr) {
   MallocChecker *checker = mgr.getChecker<MallocChecker>();
   checker->ChecksEnabled[MallocChecker::CK_InnerPointerChecker] = true;
   checker->CheckNames[MallocChecker::CK_InnerPointerChecker] =
       mgr.getCurrentCheckerName();
 }
 
 void ento::registerDynamicMemoryModeling(CheckerManager &mgr) {
   auto *checker = mgr.registerChecker<MallocChecker>();
-  checker->MemFunctionInfo.ShouldIncludeOwnershipAnnotatedFunctions =
+  checker->ShouldIncludeOwnershipAnnotatedFunctions =
       mgr.getAnalyzerOptions().getCheckerBooleanOption(checker, "Optimistic");
 }
 
 bool ento::shouldRegisterDynamicMemoryModeling(const LangOptions &LO) {
   return true;
 }
 
 #define REGISTER_CHECKER(name)                                                 \