Auto merge of rust-lang#52506 - alexcrichton:dont-duplicate-wasm-sections, r=michaelwoerister

rustc: Work around an upstream wasm ThinLTO bug

This commit implements a workaround for an [upstream LLVM bug][1] where custom
sections were accidentally duplicated amongst codegen units when ThinLTO passes
were performed. This is due to the fact that custom sections for wasm are stored
as metadata nodes which are automatically imported into modules when ThinLTO
happens. The fix here is to forcibly delete the metadata node from imported
modules before LLVM has a chance to try to copy it over.

[1]: https://bugs.llvm.org/show_bug.cgi?id=38184

Author: bors

src/rustllvm/PassWrapper.cpp

@@ -1084,11 +1084,40 @@ LLVMRustPrepareThinLTOInternalize(const LLVMRustThinLTOData *Data, LLVMModuleRef
 extern "C" bool
 LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) {
   Module &Mod = *unwrap(M);
+
   const auto &ImportList = Data->ImportLists.lookup(Mod.getModuleIdentifier());
   auto Loader = [&](StringRef Identifier) {
     const auto &Memory = Data->ModuleMap.lookup(Identifier);
     auto &Context = Mod.getContext();
-    return getLazyBitcodeModule(Memory, Context, true, true);
+    auto MOrErr = getLazyBitcodeModule(Memory, Context, true, true);
+
+    if (!MOrErr)
+      return std::move(MOrErr);
+
+    // The rest of this closure is a workaround for
+    // https://bugs.llvm.org/show_bug.cgi?id=38184 where during ThinLTO imports
+    // we accidentally import wasm custom sections into different modules,
+    // duplicating them by in the final output artifact.
+    //
+    // The issue is worked around here by manually removing the
+    // `wasm.custom_sections` named metadata node from any imported module. This
+    // we know isn't used by any optimization pass so there's no need for it to
+    // be imported.
+    //
+    // Note that the metadata is currently lazily loaded, so we materialize it
+    // here before looking up if there's metadata inside. The `FunctionImporter`
+    // will immediately materialize metadata anyway after an import, so this
+    // shouldn't be a perf hit.
+    if (Error Err = (*MOrErr)->materializeMetadata()) {
+      Expected<std::unique_ptr<Module>> Ret(std::move(Err));
+      return std::move(Ret);
+    }
+
+    auto *WasmCustomSections = (*MOrErr)->getNamedMetadata("wasm.custom_sections");
+    if (WasmCustomSections)
+      WasmCustomSections->eraseFromParent();
+
+    return std::move(MOrErr);
   };
   FunctionImporter Importer(Data->Index, Loader);
   Expected<bool> Result = Importer.importFunctions(Mod, ImportList);

src/test/run-make/wasm-custom-sections-opt/Makefile

@@ -0,0 +1,9 @@
+-include ../../run-make-fulldeps/tools.mk
+
+ifeq ($(TARGET),wasm32-unknown-unknown)
+all:
+	$(RUSTC) foo.rs -O --target wasm32-unknown-unknown
+	$(NODE) foo.js $(TMPDIR)/foo.wasm
+else
+all:
+endif

src/test/run-make/wasm-custom-sections-opt/foo.js

@@ -0,0 +1,25 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+const fs = require('fs');
+const process = require('process');
+const assert = require('assert');
+const buffer = fs.readFileSync(process.argv[2]);
+
+let m = new WebAssembly.Module(buffer);
+
+sections = WebAssembly.Module.customSections(m, "foo");
+console.log('section foo', sections);
+assert.strictEqual(sections.length, 1, "didn't create `foo` section");
+section = new Uint8Array(sections[0]);
+console.log('contents', section);
+assert.strictEqual(section.length, 4, "didn't concatenate `foo` sections");
+
+process.exit(0);

src/test/run-make/wasm-custom-sections-opt/foo.rs

@@ -0,0 +1,31 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![crate_type = "cdylib"]
+#![deny(warnings)]
+
+#[link_section = "foo"]
+pub static A: [u8; 2] = [1, 2];
+
+// make sure this is in another CGU
+pub mod another {
+    #[link_section = "foo"]
+    pub static FOO: [u8; 2] = [3, 4];
+
+    pub fn foo() {}
+}
+
+#[no_mangle]
+pub extern fn foo() {
+    // This will import `another::foo` through ThinLTO passes, and it better not
+    // also accidentally import the `FOO` custom section into this module as
+    // well
+    another::foo();
+}