Revert "[llvm][EmbedBitcodePass] Prevent modifying the module with ThinLTO"

[ilovepi]

Reverts #139999

This has a reported crash in #139999 (comment)

This PR was intended to fix an error when linking, which is unfortunately preferable to crashing clang. For now, we'll revert and investigate the problem.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Paul Kirth (ilovepi)

Changes

Reverts llvm/llvm-project#139999

This has a reported crash in #139999 (comment)

This PR was intended to fix an error when linking, which is unfortunately preferable to crashing clang. For now, we'll revert and investigate the problem.

---

Full diff: https://github.com/llvm/llvm-project/pull/145987.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp (+1-5)
• (modified) llvm/test/Transforms/EmbedBitcode/embed-wpd.ll (+3-4)

diff --git a/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp b/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp
index 5e8b2a4e3d842..73f567734a91b 100644
--- a/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp
+++ b/llvm/lib/Transforms/IPO/EmbedBitcodePass.cpp
@@ -16,7 +16,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Triple.h"
 #include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
-#include "llvm/Transforms/Utils/Cloning.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 
 #include <string>
@@ -34,11 +33,8 @@ PreservedAnalyses EmbedBitcodePass::run(Module &M, ModuleAnalysisManager &AM) {
 
   std::string Data;
   raw_string_ostream OS(Data);
-  // Clone the module with Thin LTO, since ThinLTOBitcodeWriterPass changes
-  // vtable linkage that would break the non-lto object code for FatLTO.
   if (IsThinLTO)
-    ThinLTOBitcodeWriterPass(OS, /*ThinLinkOS=*/nullptr)
-        .run(*llvm::CloneModule(M), AM);
+    ThinLTOBitcodeWriterPass(OS, /*ThinLinkOS=*/nullptr).run(M, AM);
   else
     BitcodeWriterPass(OS, /*ShouldPreserveUseListOrder=*/false, EmitLTOSummary)
         .run(M, AM);
diff --git a/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll b/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll
index 54931be42b4eb..f1f7712f54039 100644
--- a/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll
+++ b/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll
@@ -1,13 +1,12 @@
 ; RUN: opt --mtriple x86_64-unknown-linux-gnu < %s -passes="embed-bitcode<thinlto>" -S | FileCheck %s
 
-; CHECK: $_ZTV3Foo = comdat any
+; CHECK-NOT: $_ZTV3Foo = comdat any
 $_ZTV3Foo = comdat any
 
 $_ZTI3Foo = comdat any
 
-;; ThinLTOBitcodeWriter will remove the vtable for Foo, and make it an external symbol
-; CHECK: @_ZTV3Foo = linkonce_odr hidden unnamed_addr constant { [5 x ptr] } { [5 x ptr] [ptr null, ptr @_ZTI3Foo, ptr @_ZN3FooD2Ev, ptr @_ZN3FooD0Ev, ptr @_ZNKSt13runtime_error4whatEv] }, comdat, align 8, !type !0, !type !1, !type !2, !type !3, !type !4, !type !5
-; CHECK-NOT: @foo = external unnamed_addr constant { [5 x ptr] }, align 8
+; CHECK: @_ZTV3Foo = external hidden unnamed_addr constant { [5 x ptr] }, align 8
+; CHECK: @_ZTI3Foo = linkonce_odr hidden constant { ptr, ptr, ptr } { ptr getelementptr inbounds (ptr, ptr @_ZTVN10__cxxabiv120__si_class_type_infoE, i64 2), ptr @_ZTS3Foo, ptr @_ZTISt13runtime_error }, comdat, align 8
 ; CHECK: @llvm.embedded.object = private constant {{.*}}, section ".llvm.lto", align 1
 ; CHECK: @llvm.compiler.used = appending global [1 x ptr] [ptr @llvm.embedded.object], section "llvm.metadata"
 @_ZTV3Foo = linkonce_odr hidden unnamed_addr constant { [5 x ptr] } { [5 x ptr] [ptr null, ptr @_ZTI3Foo, ptr @_ZN3FooD2Ev, ptr @_ZN3FooD0Ev, ptr @_ZNKSt13runtime_error4whatEv] }, comdat, align 8, !type !0, !type !1, !type !2, !type !3, !type !4, !type !5

[ilovepi]

We have a related report here as well https://fxbug.dev/428031906

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/21604

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: Transforms/EmbedBitcode/embed-wpd.ll' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt --mtriple x86_64-unknown-linux-gnu < /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll -passes="embed-bitcode<thinlto>" -S | /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll # RUN: at line 1
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt --mtriple x86_64-unknown-linux-gnu '-passes=embed-bitcode<thinlto>' -S
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll
LLVM ERROR: Module changed by EmbedBitcodePass without invalidating analyses
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt --mtriple x86_64-unknown-linux-gnu -passes=embed-bitcode<thinlto> -S
1.	Running pass "embed-bitcode" on module "<stdin>"
 #0 0x0000000004ae3aa7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x4ae3aa7)
 #1 0x0000000004ae1595 llvm::sys::RunSignalHandlers() (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x4ae1595)
 #2 0x0000000004ae41aa SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007fc5bf8c0140 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x13140)
 #4 0x00007fc5bf3d4d61 raise (/lib/x86_64-linux-gnu/libc.so.6+0x38d61)
 #5 0x00007fc5bf3be537 abort (/lib/x86_64-linux-gnu/libc.so.6+0x22537)
 #6 0x0000000004a42bfa llvm::report_fatal_error(llvm::Twine const&, bool) (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x4a42bfa)
 #7 0x00000000026c5a5a void llvm::detail::UniqueFunctionBase<void, llvm::StringRef, llvm::Any, llvm::PreservedAnalyses const&>::CallImpl<llvm::PreservedCFGCheckerInstrumentation::registerCallbacks(llvm::PassInstrumentationCallbacks&, llvm::AnalysisManager<llvm::Module>&)::$_18>(void*, llvm::StringRef, llvm::Any&, llvm::PreservedAnalyses const&) StandardInstrumentations.cpp:0:0
 #8 0x00000000048ff68f llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x48ff68f)
 #9 0x00000000008adbcd llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool) (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x8adbcd)
#10 0x00000000008a35c7 optMain (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x8a35c7)
#11 0x00007fc5bf3bfd7a __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d7a)
#12 0x000000000089f5aa _start (/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/opt+0x89f5aa)
FileCheck error: '<stdin>' is empty.
FileCheck command line:  /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/Transforms/EmbedBitcode/embed-wpd.ll

--

********************