浏览代码

Fix memory allocation issues (#1679)

- DxilModule was not freed
- PragmaMatrixHandler was freed twice when the compiler execution was aborted
  prematurely by an exception
- Improve diagnostic tracing in CompilerTest::CompileWhenNoMemThenOOM
Helena Kotas 6 年之前
父节点
当前提交
2a7c520fcb

+ 1 - 0
lib/DXIL/DxilModule.cpp

@@ -117,6 +117,7 @@ DxilModule::DxilModule(Module *pModule)
 
 
   DXASSERT_NOMSG(m_pModule != nullptr);
   DXASSERT_NOMSG(m_pModule != nullptr);
   m_pModule->pfnRemoveGlobal = &DxilModule_RemoveGlobal;
   m_pModule->pfnRemoveGlobal = &DxilModule_RemoveGlobal;
+  m_pModule->pfnResetDxilModule = &DxilModule_ResetModule;
 
 
 #if defined(_DEBUG) || defined(DBG)
 #if defined(_DEBUG) || defined(DBG)
   // Pin LLVM dump methods.
   // Pin LLVM dump methods.

+ 2 - 1
tools/clang/include/clang/Parse/Parser.h

@@ -169,7 +169,8 @@ class Parser : public CodeCompletionHandler {
   std::unique_ptr<PragmaHandler> LoopHintHandler;
   std::unique_ptr<PragmaHandler> LoopHintHandler;
   std::unique_ptr<PragmaHandler> UnrollHintHandler;
   std::unique_ptr<PragmaHandler> UnrollHintHandler;
   std::unique_ptr<PragmaHandler> NoUnrollHintHandler;
   std::unique_ptr<PragmaHandler> NoUnrollHintHandler;
-  std::unique_ptr<PragmaHandler> PackMatrixHandler; // HLSL Change -packmatrix.
+
+  PragmaHandler *pPackMatrixHandler = nullptr; // // HLSL Change -packmatrix
 
 
   std::unique_ptr<CommentHandler> CommentSemaHandler;
   std::unique_ptr<CommentHandler> CommentSemaHandler;
 
 

+ 8 - 4
tools/clang/lib/Parse/ParsePragma.cpp

@@ -251,8 +251,11 @@ void Parser::initializePragmaHandlers() {
   } // HLSL Change, matching HLSL check to remove pragma processing
   } // HLSL Change, matching HLSL check to remove pragma processing
   else {
   else {
     // HLSL Change Begin - packmatrix.
     // HLSL Change Begin - packmatrix.
-    PackMatrixHandler.reset(new PragmaPackMatrixHandler(Actions));
-    PP.AddPragmaHandler(PackMatrixHandler.get());
+    // The pointer ownership goes to PP, which deletes it in its destructor
+    // unless it is removed & deleted via resetPragmaHandlers
+    std::unique_ptr<PragmaHandler> pHandler(new PragmaPackMatrixHandler(Actions));
+    PP.AddPragmaHandler(pHandler.get());
+    pPackMatrixHandler = pHandler.release();
     // HLSL Change End.
     // HLSL Change End.
   }
   }
 }
 }
@@ -328,8 +331,9 @@ void Parser::resetPragmaHandlers() {
   } // HLSL Change - close conditional for skipping pragmas
   } // HLSL Change - close conditional for skipping pragmas
   else {
   else {
     // HLSL Change Begin - packmatrix.
     // HLSL Change Begin - packmatrix.
-    PP.RemovePragmaHandler(PackMatrixHandler.get());
-    PackMatrixHandler.reset();
+    PP.RemovePragmaHandler(pPackMatrixHandler);
+    delete pPackMatrixHandler;
+    pPackMatrixHandler = nullptr;
     // HLSL Change End.
     // HLSL Change End.
   }
   }
 }
 }

+ 23 - 2
tools/clang/unittests/HLSL/CompilerTest.cpp

@@ -2565,6 +2565,17 @@ public:
   }
   }
 
 
   virtual void STDMETHODCALLTYPE HeapMinimize(void) {}
   virtual void STDMETHODCALLTYPE HeapMinimize(void) {}
+
+  void DumpLeaks() {
+    PtrData *ptr = (PtrData*)AllocList.Flink;;
+    PtrData *end = (PtrData*)AllocList.Blink;;
+
+    WEX::Logging::Log::Comment(FormatToWString(L"Leaks total size: %d", (signed int)m_Size).data());
+    while (ptr != end) {
+      WEX::Logging::Log::Comment(FormatToWString(L"Memory leak at 0x0%X, size %d, alloc# %d", ptr + 1, ptr->Size, ptr->AllocAtCount).data());
+      ptr = (PtrData*)ptr->Entry.Flink;
+    }
+  }
 };
 };
 
 
 #ifndef DXC_ON_APPVEYOR_CI
 #ifndef DXC_ON_APPVEYOR_CI
@@ -2615,7 +2626,12 @@ TEST_F(CompilerTest, CompileWhenNoMemThenOOM) {
   // allocations or references.
   // allocations or references.
   //
   //
   // First leak is in ((InstrumentedHeapMalloc::PtrData *)InstrMalloc.AllocList.Flink)
   // First leak is in ((InstrumentedHeapMalloc::PtrData *)InstrMalloc.AllocList.Flink)
-  VERIFY_IS_TRUE(0 == InstrMalloc.GetSize());
+  if (InstrMalloc.GetSize() != 0) {
+    WEX::Logging::Log::Comment(L"Memory leak(s) detected");
+    InstrMalloc.DumpLeaks();
+    VERIFY_IS_TRUE(0 == InstrMalloc.GetSize());
+  }
+
   VERIFY_ARE_EQUAL(initialRefCount, InstrMalloc.GetRefCount());
   VERIFY_ARE_EQUAL(initialRefCount, InstrMalloc.GetRefCount());
 
 
   // In Debug, without /D_ITERATOR_DEBUG_LEVEL=0, debug iterators will be used;
   // In Debug, without /D_ITERATOR_DEBUG_LEVEL=0, debug iterators will be used;
@@ -2651,7 +2667,12 @@ TEST_F(CompilerTest, CompileWhenNoMemThenOOM) {
       VERIFY_FAILED(hrOp);
       VERIFY_FAILED(hrOp);
     pCompiler.Release();
     pCompiler.Release();
     pResult.Release();
     pResult.Release();
-    VERIFY_IS_TRUE(0 == InstrMalloc.GetSize()); // breakpoint for i failure - i == val
+    
+    if (InstrMalloc.GetSize() != 0) {
+      WEX::Logging::Log::Comment(FormatToWString(L"Memory leak(s) detected, allocCount = %d", i).data()); 
+      InstrMalloc.DumpLeaks();
+      VERIFY_IS_TRUE(0 == InstrMalloc.GetSize());
+    }
     VERIFY_ARE_EQUAL(initialRefCount, InstrMalloc.GetRefCount());
     VERIFY_ARE_EQUAL(initialRefCount, InstrMalloc.GetRefCount());
   }
   }
 }
 }