NoQ created this revision. NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, george.karpenkov, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware. Herald added subscribers: cfe-commits, dkrupp, donat.nagy, a.sidorin, szepet.
This assertion was only enabled in Debug+Asserts, not on Release+Asserts, because it's expensive. Indeed, enabling it on Release+Asserts builds seems to cause varied slowdown, up to roughly ~5%. However, i think it's worth it, because this assertion is very important because it's very fundamental: when it crashes, it means that we've investigated a path that was already //obviously// infeasible, i.e. we could have refuted the path much earlier if we simply looked closer at it with existing solvers. So, because, testing on large codebases is pretty much always done in Release+Asserts mode, it kinda scares me that we're missing out on such an important test while doing our ultimate real-world testing. The slowdown is at most a constant factor - like, it roughly doubles the cost of every assume() operation, so it won't cause more than 2x slowdown, and in practice assume() isn't showing up in our profiles too much, so the ~5% number is relatively reliable. Of course, the behavior of Clang we actually release (i.e., built without asserts) isn't really affected. Repository: rC Clang https://reviews.llvm.org/D57062 Files: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h @@ -94,13 +94,7 @@ // If StTrue is infeasible, asserting the falseness of Cond is unnecessary // because the existing constraints already establish this. if (!StTrue) { -#ifndef __OPTIMIZE__ - // This check is expensive and should be disabled even in Release+Asserts - // builds. - // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC - // does not. Is there a good equivalent there? assert(assume(State, Cond, false) && "System is over constrained."); -#endif return ProgramStatePair((ProgramStateRef)nullptr, State); }
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h @@ -94,13 +94,7 @@ // If StTrue is infeasible, asserting the falseness of Cond is unnecessary // because the existing constraints already establish this. if (!StTrue) { -#ifndef __OPTIMIZE__ - // This check is expensive and should be disabled even in Release+Asserts - // builds. - // FIXME: __OPTIMIZE__ is a GNU extension that Clang implements but MSVC - // does not. Is there a good equivalent there? assert(assume(State, Cond, false) && "System is over constrained."); -#endif return ProgramStatePair((ProgramStateRef)nullptr, State); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits