[llvm-dev] Comparison with Statistic objects

[Craig Topper via llvm-dev]

We recently found a bug in an internal pass where we compared a Statistic
object against another value that resulted in a mismatch in output between
debug and release builds. This is because Statistic objects are designed to
be compiled out of release builds. So their increment operator doesn't do
anything and any attempt to read its value returns 0 in release builds.

Another thing to note is that Statistic objects are global variables that
count across multiple pass runs and never reset.

After noticing this behavior we did an audit of trunk to see if we could
find any cases of similar mistakes. We did this by marking all comparison
operators on the Statistic object as deleted. In total we found 6 files in
trunk that failed to build after this.

lib/Target/ARM/ARMOptimizeBarriersPass.cpp had a return value from
runOnMachineFunction dependent on a compare of a Statistic object with 0.
This would always return false in a release build, and always return true
for any pass run after the first pass run that incremented the statistic.
I've fixed this one in r302450 as it was definitely the most suspicious.

lib/CodeGen/TailDuplicator.cpp:257 has a compare between a statistic and a
command line option. I think this makes the command line option broken in
release builds. In debug builds, probably is also applying the command line
option commulatively across each function the pass runs on.

lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:339,359,730 have
more compares between command line options and statistics.

lib/Target/ARM/MLxExpansionPass.cpp:214 has another compare between
statistic and command line argument.

The last 3 don't appear to be a functional problem:

lib/Analysis/CallGraphSCCPass.cpp:481 has a compare in order to update a
"max" statistic.

tools/clang/lib/StaticAnalysis/Frontend/AnalysisConsumer.cpp:679 has a
compare for a max statistic.

tools/clang/lib/StaticAnalysis/Frontend/AnalysisConsumer.cpp:568 has a
compare to avoid a divide by 0 in calculating another statistic.


Should we do something about the first set of cases? Should we do something
to prevent creating more such issues in the future by deleting the
comparison operators and providing hooks specifically for the 3 ok cases?

~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170508/090a70e6/attachment.html>