Re: [PATCH] D15450: Avoid double deletion in Clang driver.

[Serge Pavlov via cfe-commits]

sepavloff updated this revision to Diff 46575.
sepavloff added a comment.

Added regression test.


http://reviews.llvm.org/D15450

Files:
  lib/CodeGen/CodeGenAction.cpp
  unittests/Frontend/CMakeLists.txt
  unittests/Frontend/CodeGenActionTest.cpp


Index: unittests/Frontend/CodeGenActionTest.cpp
===================================================================
--- /dev/null
+++ unittests/Frontend/CodeGenActionTest.cpp
@@ -0,0 +1,61 @@
+//===- unittests/Frontend/CodeGenActionTest.cpp --- FrontendAction tests --===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Unit tests for CodeGenAction.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/CodeGen/CodeGenAction.h"
+#include "clang/CodeGen/BackendUtil.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::frontend;
+
+namespace {
+
+
+class NullCodeGenAction : public CodeGenAction {
+public:
+  NullCodeGenAction(llvm::LLVMContext *_VMContext = nullptr)
+    : CodeGenAction(Backend_EmitLL, _VMContext) {}
+
+  // The action does not call methods of ATContext.
+  void ExecuteAction() override {
+    CompilerInstance &CI = getCompilerInstance();
+    if (!CI.hasPreprocessor())
+      return;
+    if (!CI.hasSema())
+      CI.createSema(getTranslationUnitKind(), nullptr);
+  }
+};
+
+
+TEST(CodeGenTest, TestNullCodeGen) {
+  CompilerInvocation *Invocation = new CompilerInvocation;
+  Invocation->getPreprocessorOpts().addRemappedFile(
+      "test.cc",
+      MemoryBuffer::getMemBuffer("").release());
+  Invocation->getFrontendOpts().Inputs.push_back(
+      FrontendInputFile("test.cc", IK_CXX));
+  Invocation->getFrontendOpts().ProgramAction = EmitLLVM;
+  Invocation->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+  CompilerInstance Compiler;
+  Compiler.setInvocation(Invocation);
+  Compiler.createDiagnostics();
+  EXPECT_TRUE(Compiler.hasDiagnostics());
+
+  std::unique_ptr<FrontendAction> Act(new NullCodeGenAction);
+  bool Success = Compiler.ExecuteAction(*Act);
+  EXPECT_TRUE(Success);
+}
+
+}
Index: unittests/Frontend/CMakeLists.txt
===================================================================
--- unittests/Frontend/CMakeLists.txt
+++ unittests/Frontend/CMakeLists.txt
@@ -4,11 +4,13 @@
 
 add_clang_unittest(FrontendTests
   FrontendActionTest.cpp
+  CodeGenActionTest.cpp
   )
 target_link_libraries(FrontendTests
   clangAST
   clangBasic
   clangFrontend
   clangLex
   clangSema
+  clangCodeGen
   )
Index: lib/CodeGen/CodeGenAction.cpp
===================================================================
--- lib/CodeGen/CodeGenAction.cpp
+++ lib/CodeGen/CodeGenAction.cpp
@@ -53,7 +53,6 @@
 
     std::unique_ptr<CodeGenerator> Gen;
 
-    std::unique_ptr<llvm::Module> TheModule;
     SmallVector<std::pair<unsigned, std::unique_ptr<llvm::Module>>, 4>
         LinkModules;
 
@@ -81,7 +80,10 @@
         this->LinkModules.push_back(
             std::make_pair(I.first, std::unique_ptr<llvm::Module>(I.second)));
     }
-    std::unique_ptr<llvm::Module> takeModule() { return std::move(TheModule); }
+    llvm::Module *getModule() const { return Gen->GetModule(); }
+    std::unique_ptr<llvm::Module> takeModule() {
+      return std::unique_ptr<llvm::Module>(Gen->ReleaseModule());
+    }
     void releaseLinkModules() {
       for (auto &I : LinkModules)
         I.second.release();
@@ -101,8 +103,6 @@
 
       Gen->Initialize(Ctx);
 
-      TheModule.reset(Gen->GetModule());
-
       if (llvm::TimePassesIsEnabled)
         LLVMIRGeneration.stopTimer();
     }
@@ -149,25 +149,12 @@
       }
 
       // Silently ignore if we weren't initialized for some reason.
-      if (!TheModule)
-        return;
-
-      // Make sure IR generation is happy with the module. This is released by
-      // the module provider.
-      llvm::Module *M = Gen->ReleaseModule();
-      if (!M) {
-        // The module has been released by IR gen on failures, do not double
-        // free.
-        TheModule.release();
+      if (!getModule())
         return;
-      }
-
-      assert(TheModule.get() == M &&
-             "Unexpected module change during IR generation");
 
       // Install an inline asm handler so that diagnostics get printed through
       // our diagnostics hooks.
-      LLVMContext &Ctx = TheModule->getContext();
+      LLVMContext &Ctx = getModule()->getContext();
       LLVMContext::InlineAsmDiagHandlerTy OldHandler =
         Ctx.getInlineAsmDiagnosticHandler();
       void *OldContext = Ctx.getInlineAsmDiagnosticContext();
@@ -182,13 +169,13 @@
       for (auto &I : LinkModules) {
         unsigned LinkFlags = I.first;
         CurLinkModule = I.second.get();
-        if (Linker::linkModules(*M, std::move(I.second), LinkFlags))
+        if (Linker::linkModules(*getModule(), std::move(I.second), LinkFlags))
           return;
       }
 
       EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,
                         C.getTargetInfo().getDataLayoutString(),
-                        TheModule.get(), Action, AsmOutStream);
+                        getModule(), Action, AsmOutStream);
 
       Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits