[PATCH] D47135: [analyzer][WIP] A checker for dangling string pointers in C++

Henry Wong via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 21 06:28:47 PDT 2018


MTC added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:59
+  QualType RegType = TypedR->getValueType();
+  if (RegType.getAsString() != "std::string")
+    return;
----------------
A little tip, there are other string types besides `std::string`, like `std::wstring`, which can be added in the future. See [[ http://en.cppreference.com/w/cpp/string/basic_string | basic_string ]].


================
Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:71
+
+  if (dyn_cast<CXXDestructorCall>(ICall)) {
+    if (State->contains<RawPtrMap>(TypedR)) {
----------------
`CXXDestructorCall::classof(const CallEvent *CA)` can also be used here. 


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:3002
+  // FIXME: This might not be the best allocation family for this purpose.
+  AllocationFamily Family = AF_CXXNew;
+  return State->set<RegionState>(Sym, RefState::getReleased(Family, Origin));
----------------
In addition to `std::string`, `std::vector::data()` in C++11 and `std::string_view` in C++17 may have similar problems. If these are all supported, a new `AllocationFamily` may be needed, like `AF_StdLibContainers` or something like that.


================
Comment at: lib/StaticAnalyzer/Checkers/RegionState.h:18
+
+namespace region_state {
+
----------------
I'm not sure whether `region_state` follows the naming conventions of LLVM coding standards. Can someone answer this question?


Repository:
  rC Clang

https://reviews.llvm.org/D47135





More information about the cfe-commits mailing list