[clang][codegen] Set CurLinkModule in CodeGenAction::ExecuteAction

[inglorion]

Summary

CodeGenAction::ExecuteAction creates a BackendConsumer for the
purpose of handling diagnostics. The BackendConsumer's
DiagnosticHandlerImpl method expects CurLinkModule to be set,
but this did not happen on the code path that goes through
ExecuteAction. This change makes it so that the BackendConsumer
constructor used by ExecuteAction requires the Module to be
specified and passes the appropriate module in ExecuteAction.

The change also adds a test that fails without this change
and passes with it. To make the test work, the FIXME in the
handling of DK_Linker diagnostics was addressed so that warnings
and notes are no longer silently discarded. Since this introduces
a new warning diagnostic, a flag to control it (-Wlinker-warnings)
has also been added.

[xur]

Thanks for fix this!
I like this fix and the patch looks good to me.

-Rong

clang/include/clang/Basic/DiagnosticFrontendKinds.td

 
 def err_fe_inline_asm : Error<"%0">, CatInlineAsm;
 def warn_fe_inline_asm : Warning<"%0">, CatInlineAsm, InGroup<BackendInlineAsm>;
 def note_fe_inline_asm : Note<"%0">, CatInlineAsm;
 def note_fe_inline_asm_here : Note<"instantiated into assembly here">;
 def err_fe_source_mgr : Error<"%0">, CatSourceMgr;
 def warn_fe_source_mgr : Warning<"%0">, CatSourceMgr, InGroup<BackendSourceMgr>;
 def note_fe_source_mgr : Note<"%0">, CatSourceMgr;
-def err_fe_cannot_link_module : Error<"cannot link module '%0': %1">,
-  DefaultFatal;
+def err_fe_linking_module : Error<"cannot link module '%0': %1">, DefaultFatal;
+def warn_fe_linking_module : Warning<"linking module '%0': %1">, InGroup<LinkerWarnings>;
+def note_fe_linking_module : Note<"linking module '%0': %1">;
 
 def warn_fe_frame_larger_than : Warning<"stack frame size (%0) exceeds limit (%1) in %q2">,
     BackendInfo, InGroup<BackendFrameLargerThan>;
 def warn_fe_backend_frame_larger_than: Warning<"%0">,
     BackendInfo, InGroup<BackendFrameLargerThan>;
 def err_fe_backend_frame_larger_than: Error<"%0">, BackendInfo;
 def note_fe_backend_frame_larger_than: Note<"%0">, BackendInfo;
 

clang/include/clang/Basic/DiagnosticGroups.td

DiagGroup<"strict-potentially-direct-selector",
             [ObjCPotentiallyDirectSelector]>;
 
 // Inline ASM warnings.
 def ASMOperandWidths : DiagGroup<"asm-operand-widths">;
 def ASM : DiagGroup<"asm", [
     ASMOperandWidths
   ]>;
 
+// Linker warnings.
+def LinkerWarnings : DiagGroup<"linker-warnings">;
+
 // OpenMP warnings.
 def SourceUsesOpenMP : DiagGroup<"source-uses-openmp">;
 def OpenMPClauses : DiagGroup<"openmp-clauses">;
 def OpenMPLoopForm : DiagGroup<"openmp-loop-form">;
 def OpenMPMapping : DiagGroup<"openmp-mapping">;
 def OpenMPTarget : DiagGroup<"openmp-target", [OpenMPMapping]>;
 def OpenMPPre51Compat : DiagGroup<"pre-openmp-51-compat">;
 def OpenMP51Ext : DiagGroup<"openmp-51-extensions">;

clang/lib/CodeGen/CodeGenAction.cpp

public:
     // This constructor is used in installing an empty BackendConsumer
     // to use the clang diagnostic handler for IR input files. It avoids
     // initializing the OS field.
     BackendConsumer(BackendAction Action, DiagnosticsEngine &Diags,
                     const HeaderSearchOptions &HeaderSearchOpts,
                     const PreprocessorOptions &PPOpts,
                     const CodeGenOptions &CodeGenOpts,
                     const TargetOptions &TargetOpts,
-                    const LangOptions &LangOpts,
+                    const LangOptions &LangOpts, llvm::Module *Module,
                     SmallVector<LinkModule, 4> LinkModules, LLVMContext &C,
                     CoverageSourceInfo *CoverageInfo = nullptr)
         : Diags(Diags), Action(Action), HeaderSearchOpts(HeaderSearchOpts),
           CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), LangOpts(LangOpts),
           Context(nullptr),
           LLVMIRGeneration("irgen", "LLVM IR Generation Time"),
           LLVMIRGenerationRefCount(0),
           Gen(CreateLLVMCodeGen(Diags, "", HeaderSearchOpts, PPOpts,
                                 CodeGenOpts, C, CoverageInfo)),
-          LinkModules(std::move(LinkModules)) {
+          LinkModules(std::move(LinkModules)), CurLinkModule(Module) {
       TimerIsEnabled = CodeGenOpts.TimePasses;
       llvm::TimePassesIsEnabled = CodeGenOpts.TimePasses;
       llvm::TimePassesPerRun = CodeGenOpts.TimePassesPerRun;
     }
     llvm::Module *getModule() const { return Gen->GetModule(); }
     std::unique_ptr<llvm::Module> takeModule() {
       return std::unique_ptr<llvm::Module>(Gen->ReleaseModule());
     }
case llvm::DK_SrcMgr:
     SrcMgrDiagHandler(cast<DiagnosticInfoSrcMgr>(DI));
     return;
   case llvm::DK_StackSize:
     if (StackSizeDiagHandler(cast<DiagnosticInfoStackSize>(DI)))
       return;
     ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
     break;
   case DK_Linker:
-    assert(CurLinkModule);
-    // FIXME: stop eating the warnings and notes.
-    if (Severity != DS_Error)
-      return;
-    DiagID = diag::err_fe_cannot_link_module;
+    ComputeDiagID(Severity, linking_module, DiagID);
     break;
   case llvm::DK_OptimizationRemark:
     // Optimization remarks are always handled completely by this
     // handler. There is no generic way of emitting them.
     OptimizationRemarkHandler(cast<OptimizationRemark>(DI));
     return;
   case llvm::DK_OptimizationRemarkMissed:
     // Optimization remarks are always handled completely by this
void BackendConsumer::DiagnosticHandlerImpl(const DiagnosticInfo &DI) {
   }
   std::string MsgStorage;
   {
     raw_string_ostream Stream(MsgStorage);
     DiagnosticPrinterRawOStream DP(Stream);
     DI.print(DP);
   }
 
-  if (DiagID == diag::err_fe_cannot_link_module) {
-    Diags.Report(diag::err_fe_cannot_link_module)
-        << CurLinkModule->getModuleIdentifier() << MsgStorage;
+  if (DI.getKind() == DK_Linker) {
+    assert(CurLinkModule && "CurLinkModule must be set for linker diagnostics");
+    Diags.Report(DiagID) << CurLinkModule->getModuleIdentifier() << MsgStorage;
     return;
   }
 
   // Report the backend message using the usual diagnostic mechanism.
   FullSourceLoc Loc;
   Diags.Report(Loc, DiagID).AddString(MsgStorage);
 }
 #undef ComputeDiagID
struct RAII {
     std::unique_ptr<DiagnosticHandler> PrevHandler = Ctx.getDiagnosticHandler();
     ~RAII() { Ctx.setDiagnosticHandler(std::move(PrevHandler)); }
   } _{Ctx};
 
   // Set clang diagnostic handler. To do this we need to create a fake
   // BackendConsumer.
   BackendConsumer Result(BA, CI.getDiagnostics(), CI.getHeaderSearchOpts(),
                          CI.getPreprocessorOpts(), CI.getCodeGenOpts(),
-                         CI.getTargetOpts(), CI.getLangOpts(),
+                         CI.getTargetOpts(), CI.getLangOpts(), TheModule.get(),
                          std::move(LinkModules), *VMContext, nullptr);
   // PR44896: Force DiscardValueNames as false. DiscardValueNames cannot be
   // true here because the valued names are needed for reading textual IR.
   Ctx.setDiscardValueNames(false);
   Ctx.setDiagnosticHandler(
       std::make_unique<ClangDiagnosticHandler>(CodeGenOpts, &Result));
 
   Expected<std::unique_ptr<llvm::ToolOutputFile>> OptRecordFileOrErr =

clang/test/CodeGen/Inputs/linker-diagnostic1.ll

+target triple = "armv4-none-unknown-eabi"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+declare i32 @foo(i32)
+
+define i32 @bar(i32 %x) {
+  %1 = tail call i32 @foo(i32 %x)
+  ret i32 %1
+}

clang/test/CodeGen/linker-diagnostic.ll

+; RUN: mkdir -p %t
+; RUN: opt -module-summary -o %t/foo.o %s
+; RUN: opt -module-summary -o %t/bar.o %S/Inputs/linker-diagnostic1.ll
+; RUN: llvm-lto2 run --thinlto-distributed-indexes -r %t/foo.o,foo,plx -r %t/bar.o,bar,plx \
+; RUN:   -r %t/bar.o,foo, -o %t/foobar.so %t/foo.o %t/bar.o
+; RUN: %clang -c -o %t/lto.bar.o --target=armv4-none-unknown-eabi -O2 \
+; RUN:   -fthinlto-index=%t/bar.o.thinlto.bc %t/bar.o -Wno-override-module 2>&1 | FileCheck %s
+
+; CHECK: linking module '{{.*}}/bar.o': Linking two modules of different target triples: '{{.*}}/foo.o' is 'thumbv6-unknown-linux-gnueabihf' whereas '{{.*}}/bar.o' is 'armv4-none-unknown-eabi'
+
+target triple = "thumbv6-unknown-linux-gnueabihf"
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+define i32 @foo(i32 %x) {
+  %1 = add i32 %x, 1
+  ret i32 %1
+}

clang/test/Misc/serialized-diags-driver.c

 // Test that the driver correctly combines its own diagnostics with CC1's in the
 // serialized diagnostics. To test this, we need to trigger diagnostics from
 // both processes, so we compile code that has a warning (with an associated
 // note) and then force the driver to crash. We compile stdin so that the crash
 // doesn't litter the user's system with preprocessed output.
 
 // RUN: rm -f %t
 // RUN: %clang -Wx-typoed-warning -Wall -fsyntax-only --serialize-diagnostics %t.diag %s
 // RUN: c-index-test -read-diagnostics %t.diag 2>&1 | FileCheck %s
 
-// CHECK: warning: unknown warning option '-Wx-typoed-warning' [-Wunknown-warning-option] []
+// CHECK: warning: unknown warning option '-Wx-typoed-warning'
+// CHECK-SAME: [-Wunknown-warning-option] []
 
 // CHECK: warning: variable 'voodoo' is uninitialized when used here [-Wuninitialized]
 // CHECK: note: initialize the variable 'voodoo' to silence this warning []
 // CHECK: Number of diagnostics: 2
 
 void foo() {
   int voodoo;
   voodoo = voodoo + 1;
 }