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

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs marked an inline comment as done.
rnkovacs 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();
----------------
NoQ wrote:
> `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.
I hope that in a few more reviews I'll learn to use all of these properly.


================
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 {
----------------
NoQ wrote:
> 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}}
----------------
NoQ wrote:
> 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".
I don't even know how this went so off, sorry.


https://reviews.llvm.org/D49570