[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:645-646
 
-  auto ValueToUpdate = C.getSValBuilder().makeNull();
+  auto ValueToUpdate = C.getSValBuilder().makeNullWithType(
+      IC->getCXXThisVal().getType(C.getASTContext()));
   State = State->set<TrackedRegionMap>(ThisRegion, ValueToUpdate);
----------------
I don't think this is the right type. This should be the raw pointer type for the corresponding smart pointer, not a pointer //to// the smart pointer.

`Call.getResultType()` should be good. 

Also general advice, do not use `SVal::getType()` when you can obtain the type from the AST instead. `SVal::getType()` is speculative and it inevitably loses some information (in particular, it doesn't discriminate between lvalues and pointers).


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:785
+    auto NullVal = C.getSValBuilder().makeNullWithType(
+        OtherInnerPtr->getType(C.getASTContext()));
     State = State->set<TrackedRegionMap>(OtherSmartPtrRegion, NullVal);
----------------
Same here.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:812
+    const ClassTemplateSpecializationDecl *SpecDecl =
+        dyn_cast<ClassTemplateSpecializationDecl>(
+            cast<CXXMethodDecl>(Call.getDecl())->getParent());
----------------
You're using `dyn_cast` but you're unconditionally dereferencing the result later. Looking at other copies of similar code, you probably need a null check.

(Does running the static analyzer produce a warning here? We have a checker for this!)


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:885
 
-    auto NullVal = C.getSValBuilder().makeNull();
+    auto NullVal = C.getSValBuilder().makeNullWithType(CallExpr->getType());
     // Explicitly tracking the region as null.
----------------
I suspect that this type is going to be `bool` which is probably not what you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601