NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:124
+ return;
+ updateTrackedRegion(Call, C, ThisValRegion);
+}
----------------
NoQ wrote:
> Not all constructors behave this way. In particular, if it's a copy/move
> constructor this function would track the value of the original smart pointer
> to this smart pointer, but what we need is to track the value of the raw
> pointer instead.
Actually, let's add an assertion into `updateTrackedRegion` that the value
that's put into the map is of pointer type. This would give us an automatic
notification when we make such mistakes.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:137
+ if (IsRegDead) {
+ State = State->remove<TrackedRegionMap>(Region);
+ }
----------------
xazax.hun wrote:
> In LLVM we often omit braces for small single statement branches. Also, Artem
> mentioned that we could interact with the malloc checker. Maybe it is worth
> considering to notify the malloc checker to mark the appropriate region as
> deallocated? This would help find double release errors, avoid spurious leak
> errors and so on.
> Maybe it is worth considering to notify the malloc checker to mark the
> appropriate region as deallocated?
This happens in destructor.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:213
+ if (NumArgs == 0) {
+ auto NullSVal = C.getSValBuilder().makeNull();
+ State = State->set<TrackedRegionMap>(ThisValRegion, NullSVal);
----------------
xazax.hun wrote:
> I wonder if we should use `makeNullWithType`. @NoQ what do you think? I am
> always confused when should we prefer one over the other.
I think `makeNull()` produces a null //pointer// (as opposed to numeric zero
produced by `makeZeroVal(Ty)`) and there's not much choice when it comes to
types of pointers.
Like, i mean, there were a few attempts to add support for architectures in
which pointers come in different sizes but i don't think we care about that
situation yet.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81315/new/
https://reviews.llvm.org/D81315
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
