Page MenuHomePhabricator

[StaticAnalyzer] Fix failures due to the iteration order of ExplodedNode
ClosedPublic

Authored by mgrang on Sep 1 2017, 2:19 PM.

Details

Summary

This fixes failures seen in the reverse iteration builder:
http://lab.llvm.org:8011/builders/reverse-iteration/builds/26

Failing Tests (4):

Clang :: Analysis/MisusedMovedObject.cpp
Clang :: Analysis/keychainAPI.m
Clang :: Analysis/loop-unrolling.cpp
Clang :: Analysis/malloc.c

Diff Detail

Repository
rL LLVM

Event Timeline

mgrang created this revision.Sep 1 2017, 2:19 PM

The thing I am concerned with here is whether changing SmallPtrSet to a SetVector can potentially slow down the Static Analyzer? ExplodedNode seem to be used heavily in the analyzer and I can see it being iterated upon in several places.

mgrang updated this revision to Diff 113718.Sep 3 2017, 9:18 PM
This comment was removed by mgrang.
mgrang updated this revision to Diff 113720.Sep 3 2017, 9:21 PM

I had accidentally pushed a wrong patch to this commit. Fixed it now.

Ping for reviews please.

zaks.anna edited edge metadata.Sep 6 2017, 3:34 PM

Thanks for addressing the non-determinism!!!

The ExplodedNodeSet is predominantly used to hold very small sets and it is used quite a bit in the analyzer. Maybe we could we use SmallSetVector here instead?

mgrang updated this revision to Diff 114089.Sep 6 2017, 3:51 PM

Addressed comments.

mgrang added inline comments.Sep 6 2017, 3:52 PM
include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
407 ↗(On Diff #114089)

SmallSetVector size has to be a power of 2. I was split between 4 and 8. I chose 4 as it's closer to 5 :)

zaks.anna accepted this revision.Sep 6 2017, 3:53 PM

Thank you!
Anna

This revision is now accepted and ready to land.Sep 6 2017, 3:53 PM
This revision was automatically updated to reflect the committed changes.