martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

This basically looks good to me.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:482-483
+    // of the container (the difference between its `begin()` and `end()` to
+    // this size. Function `relateSymbols()` returns null if it contradits
+    // the current size.
+    const auto CalcEnd =
----------------
baloghadamsoftware wrote:
> martong wrote:
> > How? I don't see how does it access the `size`.
> As explained between the parenthesis, the actual size of the container is the 
> difference between its `begin()` and its `end()`. If we have this difference, 
> then we know the actual size. The other value we may have is the return value 
> of the `size()` function. We either have one of them, both or none. If we 
> have one, then we adjust the other. If we have both, then we check for 
> consistency, and generated a sink if they are inconsistent. If we have none, 
> then we do nothing.
Ok, makes sense.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:492
+  } else {
+    if (CalcSize) {
+      // If the current size is a concrete integer, bind this to the return
----------------
baloghadamsoftware wrote:
> martong wrote:
> > What if we have both `RetSize` and `CalcSize`? Should we check their values 
> > for consistency? (And perhaps adding another sink node if we have 
> > inconsistency?)
> This is handled in the `if` branch: having `CalcSize` means that we know the 
> difference between the `begin()` and the `end()`, thus inconsistency between 
> `RetSize` and `CalcSize` is the same as inconstistency between `CalcEnd` and 
> `EndSym`. The comment above explains that if there is such inconsistency, 
> then `relateSymbols()` returns a null pointer which we assign to `State`. At 
> the end of this functions we generate a sink whenever `State` is a null 
> pointer.
Ok. Perhpas we could move the relevant comment just right about the call of 
`relateSymbols()`.


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

https://reviews.llvm.org/D76604

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to