[PATCH] D63093: [analyzer] WIP: MallocChecker: Release temporary CXXNewExpr

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 10 19:11:15 PDT 2019


NoQ added a comment.

In such cases i recommend starting with writing down a test. Like in TDD: first test, //then// code.

The general direction doesn't seem reasonable to me; it introduces some pattern-matching for a specific scenario, but it's unclear why is this scenario a problem on its own. We might eventually do something similar, but I recommend //fully// debugging the false positive - i.e., understanding what exactly is wrong with it, before picking a suppression mechanism.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1119-1122
+      // Set the symbol's state to Released.
+      State = State->set<RegionState>(
+          Sym, RefState::getReleased(NE->isArray() ? AF_CXXNewArray : AF_CXXNew,
+                                     NE));
----------------
I think it's clearly too early for marking the pointer as released. I.e., what about:
```lang=c++
auto x = std::shared_ptr(new int); // the pointer is marked as released
use(x.get()); // use-after-free???
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D63093





More information about the cfe-commits mailing list