[clang] 11857d4 - [remark][diagnostics] [codegen] Fix PR44896

[Rong Xu via cfe-commits]

Author: Rong Xu
Date: 2020-02-25T08:15:17-08:00
New Revision: 11857d49948b845dcfd7c7f78595095e3add012d

URL: https://github.com/llvm/llvm-project/commit/11857d49948b845dcfd7c7f78595095e3add012d
DIFF: https://github.com/llvm/llvm-project/commit/11857d49948b845dcfd7c7f78595095e3add012d.diff

LOG: [remark][diagnostics] [codegen] Fix PR44896

This patch fixes PR44896. For IR input files, option fdiscard-value-names
should be ignored as we need named values in loadModule().
Commit 60d3947922 sets this option after loadModule() where valued names
already created. This creates an inconsistent state in setNameImpl()
that leads to a seg fault.
This patch forces fdiscard-value-names to be false for IR input files.

This patch also emits a warning of "ignoring -fdiscard-value-names" if
option fdiscard-value-names is explictly enabled in the commandline for
IR input files.

Differential Revision: https://reviews.llvm.org/D74878

Added: 
    clang/test/CodeGen/PR44896.ll

Modified: 
    clang/include/clang/Basic/DiagnosticDriverKinds.td
    clang/lib/CodeGen/CodeGenAction.cpp
    clang/lib/Driver/ToolChains/Clang.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index ecd871e36ee8..48ece91d3c45 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -271,6 +271,9 @@ def warn_drv_unsupported_debug_info_opt_for_target : Warning<
   InGroup<UnsupportedTargetOpt>;
 def warn_c_kext : Warning<
   "ignoring -fapple-kext which is valid for C++ and Objective-C++ only">;
+def warn_ignoring_fdiscard_for_bitcode : Warning<
+  "ignoring -fdiscard-value-names for LLVM Bitcode">,
+  InGroup<UnusedCommandLineArgument>;
 def warn_drv_input_file_unused : Warning<
   "%0: '%1' input unused%select{ when '%3' is present|}2">,
   InGroup<UnusedCommandLineArgument>;

diff  --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp
index 5ebc34cd2700..81946b194495 100644
--- a/clang/lib/CodeGen/CodeGenAction.cpp
+++ b/clang/lib/CodeGen/CodeGenAction.cpp
@@ -1146,6 +1146,9 @@ void CodeGenAction::ExecuteAction() {
                            CI.getTargetOpts(), CI.getLangOpts(),
                            CI.getFrontendOpts().ShowTimers,
                            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));
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 19a23c9de5b3..d387a1dc2079 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4332,8 +4332,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
 
   // Discard value names in assert builds unless otherwise specified.
   if (Args.hasFlag(options::OPT_fdiscard_value_names,
-                   options::OPT_fno_discard_value_names, !IsAssertBuild))
+                   options::OPT_fno_discard_value_names, !IsAssertBuild)) {
+    if (Args.hasArg(options::OPT_fdiscard_value_names) &&
+        (std::any_of(Inputs.begin(), Inputs.end(),
+                     [](const clang::driver::InputInfo &II) {
+                       return types::isLLVMIR(II.getType());
+                     }))) {
+      D.Diag(diag::warn_ignoring_fdiscard_for_bitcode);
+    }
     CmdArgs.push_back("-discard-value-names");
+  }
 
   // Set the main file name, so that debug info works even with
   // -save-temps.

diff  --git a/clang/test/CodeGen/PR44896.ll b/clang/test/CodeGen/PR44896.ll
new file mode 100644
index 000000000000..a4d344579870
--- /dev/null
+++ b/clang/test/CodeGen/PR44896.ll
@@ -0,0 +1,15 @@
+; RUN: %clang -fdiscard-value-names -S %s -o /dev/null 2>&1 | FileCheck --check-prefix=WARNING %s
+; RUN: %clang -S %s -o /dev/null 2>&1 | FileCheck --check-prefix=NOWARNING %s
+; RUN: %clang_cc1 -S -emit-llvm %s -discard-value-names -o /dev/null
+; PR 44896
+
+; WARNING: ignoring -fdiscard-value-names for LLVM Bitcode
+; NOWARNING-NOT: ignoring -fdiscard-value-names for LLVM Bitcode
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux-gnu"
+
+define linkonce i8* @b(i8* %a) {
+  ret i8* %a
+}
+