[PATCH] D49570: [analyzer] Improve warning messages and notes of DanglingInternalBufferChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:253
+      allocation_state::getContainerObjRegion(N->getState(), PtrToBuf);
+  const auto *TypedRegion = dyn_cast<TypedValueRegion>(ObjRegion);
+  QualType ObjTy = TypedRegion->getValueType();
----------------
`dyn_cast` may fail by returning a null pointer. This either needs to be changed to `cast` or there needs to be a check for a null pointer before use. I guess it should be a `cast` because you're only acting on typed regions in the checker itself.


================
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2931-2932
+            OS << "deallocated by call to destructor";
+            StackHint = new StackHintGeneratorForSymbol(Sym,
+                                      "Returning; inner buffer was deallocated");
           } else {
----------------
Cool stuff!


================
Comment at: test/Analysis/dangling-internal-buffer.cpp:63
     // expected-note at -4 {{Taking false branch}}
-    consume(c); // expected-warning {{Use of memory after it is freed}}
-    // expected-note at -1 {{Use of memory after it is freed}}
+    consume(c); // expected-warning {{Deallocated pointer returned to the caller}}
+    // expected-note at -1 {{Deallocated pointer returned to the caller}}
----------------
Mm, nono, there's no `return` statement here, so we shouldn't say that our pointer is returned to the caller. Whether it should say "returned to the caller" or not, should not depend on the allocation family, but on the kind of "use" we encounter "after" "free".


================
Comment at: test/Analysis/dangling-internal-buffer.cpp:75
     std::string s;
-    c = s.data(); // expected-note {{Dangling inner pointer obtained here}}
-  }               // expected-note {{Inner pointer invalidated by call to destructor}}
+    c = s.data(); // expected-note {{Pointer to inner buffer of 'std::string' object obtained here}}
+  }               // expected-note {{Inner buffer deallocated by call to destructor}}
----------------
I suggest a shorter `on a 'std::string'` instead of `on 'std::string' object`.


Repository:
  rC Clang

https://reviews.llvm.org/D49570