Fix compiler assert. (#585)

This change fixes the compiler assert reported in Github issue #575. We are inserting temporary variables for strings so that we can describe their bounds.  The assert was checking that we don't compute a temporary variable more than once in an expression.

The problem was that we were copying an expression that creates and binds a temporary into the bounds.   The bounds is used at runtime in the dynamic_bounds_cast.  This causes the temporary to be computed twice, which is what the assert is protecting against.  Since we've already computed the value into a temporary, the fix is to replace the binding with a read of the temporary.

Testing:
- Added simple repro case that caused a compiler crash.
- Added a runtime test that will be covered by a PR to the Checked C repo
- Passed testing locally for Window x64.
- Passed automated testing for Linux.

lib/AST/Expr.cpp

@@ -1668,9 +1668,6 @@ namespace {
     if (CXXBindTemporaryExpr *Binder = dyn_cast<CXXBindTemporaryExpr>(expr))
       expr = Binder->getSubExpr();
 
-    if (CHKCBindTemporaryExpr *Temp = dyn_cast<CHKCBindTemporaryExpr>(expr))
-      expr = Temp->getSubExpr();
-
     return expr;
   }
 }

lib/Sema/SemaBounds.cpp

@@ -404,6 +404,32 @@ namespace {
 }
 #endif
 
+// Convert all temporary bindings in an expression to uses of the values
+// produced by a binding.   This should be done for bounds expressions that
+// are used in runtime checks.  That way we don't try to recompute a
+// temporary multiple times in an expression.
+namespace {
+  class PruneTemporaryHelper : public TreeTransform<PruneTemporaryHelper> {
+    typedef TreeTransform<PruneTemporaryHelper> BaseTransform;
+
+
+  public:
+    PruneTemporaryHelper(Sema &SemaRef) :
+      BaseTransform(SemaRef) { }
+
+    ExprResult TransformCHKCBindTemporaryExpr(CHKCBindTemporaryExpr *E) {
+      return new (SemaRef.Context) BoundsValueExpr(SourceLocation(), E);
+    }
+  };
+
+  Expr *PruneTemporaryBindings(Sema &SemaRef, Expr *E) {
+    Sema::ExprSubstitutionScope Scope(SemaRef); // suppress diagnostics
+    ExprResult R = PruneTemporaryHelper(SemaRef).TransformExpr(E);
+    assert(!R.isInvalid());
+    return R.get();
+  }
+}
+
 namespace {
   // Class for inferring bounds expressions for C expressions.
 
@@ -2664,6 +2690,14 @@ namespace {
         }
 
         assert(NormalizedBounds);
+
+        // These bounds will be computed and tested at runtime.  Don't
+        // recompute any expressions computed to temporaries already.
+        NormalizedBounds =
+          cast<BoundsExpr>(PruneTemporaryBindings(S, NormalizedBounds));
+        SubExprBounds =
+          cast<BoundsExpr>(PruneTemporaryBindings(S, SubExprBounds));
+
         E->setNormalizedBoundsExpr(NormalizedBounds);
         E->setSubExprBoundsExpr(SubExprBounds);
         if (DumpBounds)

test/CheckedC/regression-cases/bug_575_string_literal.c

@@ -0,0 +1,14 @@
+// This is a regression test case for 
+// https://github.com/Microsoft/checkedc-clang/issues/575
+//
+// This test checks that the compiler does not crash on a dynamic bounds
+// cast whose argument is a string literal.
+//
+// RUN: %clang -cc1 -verify -fcheckedc-extension %s
+// expected-no-diagnostics
+
+int main(void) {
+  _Nt_array_ptr<const char> str_with_len : count(2) = 
+    _Dynamic_bounds_cast<_Nt_array_ptr<const char>>(("\n"), count(2));
+  return 0;
+}