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