[PATCH] D76604: [Analyzer] Model `size()` member function of containers

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:456
+                                   SVal RetVal) const {
+  const auto *ContReg = Cont.getAsRegion();
+  if (!ContReg)
----------------
Szelethus wrote:
> martong wrote:
> > Just out of curiosity: How do we handle containers that do not have a contiguous memory region? Balanced trees, bucketed hash-maps, etc.
> I suspect that this is referring to the memory address of the container object, not the storage of the elements.
Yes. The region just serves to identify the container. It is not necessarily the region where the data is stored.


================
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 =
----------------
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.


================
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
----------------
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.


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

https://reviews.llvm.org/D76604