NoQ marked an inline comment as done.
NoQ added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:378-379
+
+  using CheckFn = void (MallocChecker::*)(CheckerContext &C, const CallExpr 
*CE,
+                                          ProgramStateRef State) const;
+
----------------
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 :)


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:397-398
+  CallDescriptionMap<CheckFn> NonFreeingMemFnMap{
+      {{"alloca", 1}, &MallocChecker::checkAlloca},
+      {{"_alloca", 1}, &MallocChecker::checkAlloca},
+      {{"malloc", 1}, &MallocChecker::checkMalloc},
----------------
I think `alloca` deserves `CDF_MaybeBuiltin`. This would also probably allow 
you to remove the second line.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:402-404
+      {{"strndup", 2}, &MallocChecker::checkStrdup},
+      {{"strdup", 1}, &MallocChecker::checkStrdup},
+      {{"_strdup", 1}, &MallocChecker::checkStrdup},
----------------
These are also builtins.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:407-408
+      {{"if_nameindex", 1}, &MallocChecker::checkIfNameIndex},
+      {{"wcsdup", 1}, &MallocChecker::checkStrdup},
+      {{"_wcsdup", 1}, &MallocChecker::checkStrdup},
+      {{"g_malloc", 1}, &MallocChecker::checkMalloc},
----------------
And these too.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1487
 
-  if (Att->getModule()->getName() !=
-      MemFunctionInfo.CD_malloc.getFunctionName())
+  if (Att->getModule()->getName() != "malloc")
     return nullptr;
----------------
Szelethus wrote:
> 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.
I think this is about the (lack of) support for `__attribute__((malloc))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68165



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

Reply via email to