NoQ added a comment.
This is amazing, great progress!
I added the parent revision because it didn't compile on pre-merge checks
otherwise.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:801
+ // must be a superset of, but not necessarily equal to ExitOwners.
+ return !llvm::set_is_subset(ExitOwners, CurrOwners);
+ }
----------------
Can you also comment on what's, generally, the default scenario / motivating
example where this is necessary? What makes you hunt down store bindings that
didn't cause an escape to happen (given that an escape would have been a state
change)? IIUC this is for the situation when the callee stores the pointer in a
caller-local variable and in this case you don't want to claim that ownership
didn't change?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:809-810
+ return std::make_shared<PathDiagnosticEventPiece>(
+ L, "Returning without changing the ownership status of allocated "
+ "memory");
+ }
----------------
I suggest: `"Returning without deallocating memory or storing the pointer for
later deallocation"`. Still a bit flimsy but less jargony.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:832-833
+ ArrayRef<ParmVarDecl *> Parameters, unsigned ParamIdx) override {
+ // TODO: Check whether the allocated memory was actually passed into the
+ // function.
+ return emitNote(N);
----------------
How difficult would it be to re-use this part as well?
================
Comment at: clang/test/Analysis/NewDeleteLeaks.cpp:104-106
+// TODO: We don't want a note here. sink() doesn't seem like a function that
+// even attempts to take care of any memory ownership problems.
+namespace memory_passed_into_fn_that_doesnt_intend_to_free {
----------------
How do you think this is different from the very first test,
`memory_allocated_in_fn_call()`? Is this just because of how the pointer is put
into a variable first? This function is kinda still the only place that could
free memory.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105819/new/
https://reviews.llvm.org/D105819
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
