[PATCH] D65349: [analyzer] Be more careful with destructors of non-regions.

Mon Jul 29 07:37:19 PDT 2019

baloghadamsoftware added a comment.

Yes, it crashes! :-) I tried it because I did not believe it, but it does. Is there any real-world use-case for casting concrete integers to class instances? How did you find this crashing case?

================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1055
+  Loc ThisLoc = State->getSVal(ThisVal).castAs<Loc>();
+  SVal FieldLoc = State->getLValue(Member, ThisLoc);

----------------
Why do we rename it from `FieldVal` to `FieldLoc`? Are we sure that this is a `Loc`? If so, then we should apply `castAs<Loc>()` and change the type to `Loc` as well. (Or `auto` since `castAs<Loc>()` is enough according to the LLVM coding standards.) If we are not sure, then I think it is better to leave it `FieldVal`.

================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:627

+  if (!Dest) {
+    CallOpts.IsCtorOrDtorWithImproperlyModeledTargetRegion = true;
----------------
Could you please add some lines of comments to this code block?

Repository:
  rC Clang

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

https://reviews.llvm.org/D65349