[analyzer] Re-enable the "System is over constrained" assertion on optimized builds.

[NoQ]

Summary

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.

[Szelethus]

Hmmm, came across this one in the not too distant future, and always wondered how painful that performance hit would be that even Release+Asserts should be spared from it. I think 5% performance hit, if asserts are enabled, is an acceptable tradeoff, if this check is crucial.

[Szelethus]

And if we find out that this is far too painful, we can always put it back anyways. Doubt that'll happen though.

[a_sidorin]

Oh, I remember how much pain have debug mode-only assertions caused.

[george.karpenkov]

We would start getting crashes from TrustNonnullChecker if we enable it. Shouldn't those be fixed first?
An input which crashes with this assertion is test/Analysis/trustnonnullchecker_test.m.

[NoQ]

I think there are much more problems that we will find this way, starting from

long foo(long x) {
  unsigned y = (unsigned)x;
  if (y <= -1)
    return 0;

  return (y + 2) < 2; // crash :(
}

I don't mind delaying this patch until at least some of them are sorted out.

---

+@alexfh because he seems to enjoy running us with assertions :)

include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h

public:
   /// Returns a pair of states (StTrue, StFalse) where the given condition is
   /// assumed to be true or false, respectively.
   ProgramStatePair assumeDual(ProgramStateRef State, DefinedSVal Cond) {
     ProgramStateRef StTrue = assume(State, Cond, true);
 
     // 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);
     }
 
     ProgramStateRef StFalse = assume(State, Cond, false);
     if (!StFalse) {
       // We are careful to return the original state, /not/ StTrue,
       // because we want to avoid having callers generate a new node
       // in the ExplodedGraph.