This is an archive of the discontinued LLVM Phabricator instance.

[Analyzer] Model `size()` member function of containers
AcceptedPublic

Authored by baloghadamsoftware on Mar 23 2020, 6:06 AM.

Details

Summary

After empty(), also model size() to further improve existing checkers and enable more future checkers for iterators and containers.

Diff Detail

Event Timeline

martong added inline comments.Mar 23 2020, 8:47 AM
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
458

Just out of curiosity: How do we handle containers that do not have a contiguous memory region? Balanced trees, bucketed hash-maps, etc.

484–485

How? I don't see how does it access the size.

494

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?)

Szelethus added inline comments.Mar 24 2020, 9:03 AM
clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
458

I suspect that this is referring to the memory address of the container object, not the storage of the elements.

clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
458

Yes. The region just serves to identify the container. It is not necessarily the region where the data is stored.

484–485

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.

494

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.

martong accepted this revision.Sep 8 2020, 6:22 AM

This basically looks good to me.

clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
484–485

Ok, makes sense.

494

Ok. Perhpas we could move the relevant comment just right about the call of relateSymbols().

This revision is now accepted and ready to land.Sep 8 2020, 6:22 AM

New test cases added.