From e08fcbbd8d241ab68d72dc8b288ca55b8c1114fe Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 18 Jul 2018 12:12:53 -0700 Subject: [PATCH] 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 --- src/rustllvm/PassWrapper.cpp | 31 ++++++++++++++++++- .../wasm-custom-sections-opt/Makefile | 9 ++++++ .../run-make/wasm-custom-sections-opt/foo.js | 25 +++++++++++++++ .../run-make/wasm-custom-sections-opt/foo.rs | 31 +++++++++++++++++++ 4 files changed, 95 insertions(+), 1 deletion(-) create mode 100644 src/test/run-make/wasm-custom-sections-opt/Makefile create mode 100644 src/test/run-make/wasm-custom-sections-opt/foo.js create mode 100644 src/test/run-make/wasm-custom-sections-opt/foo.rs diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index a00ff3b345d11..637f2e0beaaa6 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/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> 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 Result = Importer.importFunctions(Mod, ImportList); diff --git a/src/test/run-make/wasm-custom-sections-opt/Makefile b/src/test/run-make/wasm-custom-sections-opt/Makefile new file mode 100644 index 0000000000000..63644c513c30d --- /dev/null +++ b/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 diff --git a/src/test/run-make/wasm-custom-sections-opt/foo.js b/src/test/run-make/wasm-custom-sections-opt/foo.js new file mode 100644 index 0000000000000..1d1a9bd13eea7 --- /dev/null +++ b/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 or the MIT license +// , 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); diff --git a/src/test/run-make/wasm-custom-sections-opt/foo.rs b/src/test/run-make/wasm-custom-sections-opt/foo.rs new file mode 100644 index 0000000000000..4d983514a2360 --- /dev/null +++ b/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 or the MIT license +// , 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(); +}