Browse Source

Additional fixes for error handling exposed by OOM testing (#425)

In particular, out-of-memory exceptions thrown during string construction in the context of certain move constructors (in this case, and std::pair) would cause the runtime to terminate the process.

The problem is that a noexcept specification was added to some STL move constructors incorrectly, and when the runtime finds such a frame while unwinding, it will invoke std::terminate(). See the comments and static assertions for more details.

Fixes the Realloc implementation in the test memory allocator.
Marcelo Lopez Ruiz 8 years ago
parent
commit
9903286944

+ 20 - 0
include/llvm/ADT/StringRef.h

@@ -568,4 +568,24 @@ namespace llvm {
   template <> struct isPodLike<StringRef> { static const bool value = true; };
 }
 
+// HLSL Change Starts
+// StringRef provides an operator string; that trips up the std::pair noexcept specification,
+// which (a) enables the moves constructor (because conversion is allowed), but (b)
+// misclassifies the the construction as nothrow.
+namespace std {
+  template<>
+  struct is_nothrow_constructible <std::string, llvm::StringRef>
+    : std::false_type {
+  };
+  template<>
+  struct is_nothrow_constructible <std::string, llvm::StringRef &>
+    : std::false_type {
+  };
+  template<>
+  struct is_nothrow_constructible <std::string, const llvm::StringRef &>
+    : std::false_type {
+  };
+}
+// HLSL Change Ends
+
 #endif

+ 16 - 1
tools/clang/lib/Basic/DiagnosticIDs.cpp

@@ -676,7 +676,15 @@ bool DiagnosticIDs::ProcessDiag(DiagnosticsEngine &Diag) const {
   }
 
   // Finally, report it.
-  EmitDiag(Diag, DiagLevel);
+  // HLSL Change - guard bad_alloc with a fatal error report.
+  try {
+    EmitDiag(Diag, DiagLevel);
+  }
+  catch (const std::bad_alloc &) {
+    Diag.FatalErrorOccurred = true;
+    Diag.ErrorOccurred = true;
+    Diag.UnrecoverableErrorOccurred = true;
+  }
   return true;
 }
 
@@ -719,3 +727,10 @@ bool DiagnosticIDs::isARCDiagnostic(unsigned DiagID) {
   unsigned cat = getCategoryNumberForDiag(DiagID);
   return DiagnosticIDs::getCategoryNameFromID(cat).startswith("ARC ");
 }
+
+// HLSL Change Starts
+static_assert(std::is_nothrow_constructible<clang::DiagnosticIDs::Level>::value == 1, "enum cannot nothrow");
+static_assert(std::is_nothrow_constructible<std::string, llvm::StringRef>::value == 0, "string from StringRef can throw");
+static_assert(std::is_nothrow_constructible<std::string, llvm::StringRef &>::value == 0, "string from StringRef & can throw");
+static_assert(std::is_nothrow_constructible<std::pair<clang::DiagnosticIDs::Level, std::string>, std::pair<clang::DiagnosticIDs::Level, std::string>&>::value == 0, "pair can throw");
+// HLSL Change Starts Ends

+ 9 - 1
tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -2171,9 +2171,12 @@ public:
   }
 
   virtual void *STDMETHODCALLTYPE Realloc(_In_opt_ void *pv, _In_ SIZE_T cb) {
+    SIZE_T priorSize = pv == nullptr ? (SIZE_T)0 : GetSize(pv);
     void *R = Alloc(cb);
     if (!R)
       return nullptr;
+    SIZE_T copySize = std::min(cb, priorSize);
+    memcpy(R, pv, copySize);
     Free(pv);
     return R;
   }
@@ -2241,6 +2244,11 @@ TEST_F(CompilerTest, CompileWhenNoMemThenOOM) {
     L"cs_6_0", nullptr, 0, nullptr, 0, nullptr, &pResult));
   allocCount = InstrMalloc.GetAllocCount();
   allocSize = InstrMalloc.GetAllocSize();
+
+  HRESULT hrWithMemory;
+  VERIFY_SUCCEEDED(pResult->GetStatus(&hrWithMemory));
+  VERIFY_SUCCEEDED(hrWithMemory);
+
   pCompiler.Release();
   pResult.Release();
 
@@ -2264,7 +2272,7 @@ TEST_F(CompilerTest, CompileWhenNoMemThenOOM) {
 
   // Now, fail each allocation and make sure we get an error.
   for (ULONG i = 0; i <= allocCount; ++i) {
-    LogCommentFmt(L"alloc fail %u", i);
+    // LogCommentFmt(L"alloc fail %u", i);
     bool isLast = i == allocCount;
     InstrMalloc.ResetCounts();
     InstrMalloc.ResetHeap();