From 04febd30a8dab3ff4b6e6032f1a1a9f4725f8267 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Thu, 23 Jul 2020 15:06:21 -0700 Subject: [PATCH] [lld][WebAssembly] Error on import/export of mutable global without `mutable-globals` feature Also add the +mutable-globals features in clang when building with `-fPIC` since the linker will generate mutable globals imports and exports in that case. Differential Revision: https://reviews.llvm.org/D87537 --- clang/lib/Driver/ToolChains/WebAssembly.cpp | 21 +++++++++++++++++++++ clang/test/Driver/wasm-toolchain.c | 11 +++++++++++ lld/test/wasm/Inputs/undefined-globals.s | 4 ++-- lld/test/wasm/emit-relocs-fpic.s | 4 ++-- lld/test/wasm/gc-imports.s | 6 +++--- lld/test/wasm/mutable-globals.s | 13 +++++++++++++ lld/test/wasm/pie.ll | 2 +- lld/test/wasm/shared.ll | 2 +- lld/wasm/Writer.cpp | 23 +++++++++++++++++++++++ 9 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 lld/test/wasm/mutable-globals.s diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp index 1016873..d953082 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.cpp +++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp @@ -243,6 +243,27 @@ void WebAssembly::addClangTargetOptions(const ArgList &DriverArgs, CC1Args.push_back("+sign-ext"); } + if (!DriverArgs.hasFlag(options::OPT_mmutable_globals, + options::OPT_mno_mutable_globals, false)) { + // -fPIC implies +mutable-globals because the PIC ABI used by the linker + // depends on importing and exporting mutable globals. + llvm::Reloc::Model RelocationModel; + unsigned PICLevel; + bool IsPIE; + std::tie(RelocationModel, PICLevel, IsPIE) = + ParsePICArgs(*this, DriverArgs); + if (RelocationModel == llvm::Reloc::PIC_) { + if (DriverArgs.hasFlag(options::OPT_mno_mutable_globals, + options::OPT_mmutable_globals, false)) { + getDriver().Diag(diag::err_drv_argument_not_allowed_with) + << "-fPIC" + << "-mno-mutable-globals"; + } + CC1Args.push_back("-target-feature"); + CC1Args.push_back("+mutable-globals"); + } + } + if (DriverArgs.getLastArg(options::OPT_fwasm_exceptions)) { // '-fwasm-exceptions' is not compatible with '-mno-exception-handling' if (DriverArgs.hasFlag(options::OPT_mno_exception_handing, diff --git a/clang/test/Driver/wasm-toolchain.c b/clang/test/Driver/wasm-toolchain.c index ad8b000..3c2eb66 100644 --- a/clang/test/Driver/wasm-toolchain.c +++ b/clang/test/Driver/wasm-toolchain.c @@ -119,3 +119,14 @@ // RUN: | FileCheck -check-prefix=CHECK-REACTOR %s // CHECK-REACTOR: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" // CHECK-REACTOR: wasm-ld{{.*}}" "crt1-reactor.o" "--entry" "_initialize" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" + +// -fPIC implies +mutable-globals + +// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=%s/no-sysroot-there -fPIC 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-PIC %s +// CHECK-PIC: clang{{.*}}" "-cc1" {{.*}} "-target-feature" "+mutable-globals" + +// '-mno-mutable-globals' is not allowed with '-fPIC' +// RUN: %clang %s -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=%s/no-sysroot-there -fPIC -mno-mutable-globals %s 2>&1 \ +// RUN: | FileCheck -check-prefix=PIC_NO_MUTABLE_GLOBALS %s +// PIC_NO_MUTABLE_GLOBALS: error: invalid argument '-fPIC' not allowed with '-mno-mutable-globals' diff --git a/lld/test/wasm/Inputs/undefined-globals.s b/lld/test/wasm/Inputs/undefined-globals.s index 607d794..54dc418 100644 --- a/lld/test/wasm/Inputs/undefined-globals.s +++ b/lld/test/wasm/Inputs/undefined-globals.s @@ -7,5 +7,5 @@ use_undef_global: global.get used_undef_global end_function -.globaltype unused_undef_global, i64 -.globaltype used_undef_global, i64 +.globaltype unused_undef_global, i64, immutable +.globaltype used_undef_global, i64, immutable diff --git a/lld/test/wasm/emit-relocs-fpic.s b/lld/test/wasm/emit-relocs-fpic.s index c70e1e6..1d81ca6 100644 --- a/lld/test/wasm/emit-relocs-fpic.s +++ b/lld/test/wasm/emit-relocs-fpic.s @@ -1,6 +1,6 @@ -# RUN: llvm-mc -triple=wasm32-unknown-unknown -filetype=obj -o %t.o < %s +# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %s -o %t.o # RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown %p/Inputs/ret32.s -o %t.ret32.o -# RUN: wasm-ld -pie --export-all --no-gc-sections --no-entry --emit-relocs -o %t.wasm %t.o %t.ret32.o +# RUN: wasm-ld -pie --export-all --no-check-features --no-gc-sections --no-entry --emit-relocs -o %t.wasm %t.o %t.ret32.o # RUN: obj2yaml %t.wasm | FileCheck %s load_hidden_data: diff --git a/lld/test/wasm/gc-imports.s b/lld/test/wasm/gc-imports.s index 6564b5c..1f8bca9 100644 --- a/lld/test/wasm/gc-imports.s +++ b/lld/test/wasm/gc-imports.s @@ -31,7 +31,7 @@ _start: # CHECK-NEXT: Field: used_undef_global # CHECK-NEXT: Kind: GLOBAL # CHECK-NEXT: GlobalType: I64 -# CHECK-NEXT: GlobalMutable: true +# CHECK-NEXT: GlobalMutable: false # CHECK-NEXT: - Type: # CHECK: - Type: CUSTOM # CHECK-NEXT: Name: name @@ -62,12 +62,12 @@ _start: # NO-GC-NEXT: Field: unused_undef_global # NO-GC-NEXT: Kind: GLOBAL # NO-GC-NEXT: GlobalType: I64 -# NO-GC-NEXT: GlobalMutable: true +# NO-GC-NEXT: GlobalMutable: false # NO-GC-NEXT: - Module: env # NO-GC-NEXT: Field: used_undef_global # NO-GC-NEXT: Kind: GLOBAL # NO-GC-NEXT: GlobalType: I64 -# NO-GC-NEXT: GlobalMutable: true +# NO-GC-NEXT: GlobalMutable: false # NO-GC-NEXT: - Type: # NO-GC: - Type: CUSTOM # NO-GC-NEXT: Name: name diff --git a/lld/test/wasm/mutable-globals.s b/lld/test/wasm/mutable-globals.s new file mode 100644 index 0000000..98f216e --- /dev/null +++ b/lld/test/wasm/mutable-globals.s @@ -0,0 +1,13 @@ +# RUN: llvm-mc -filetype=obj -triple=wasm32-unknown-unknown -o %t.o %s +# RUN: not wasm-ld %t.o -o %t.wasm 2>&1 | FileCheck %s + +.globl _start +_start: + .functype _start () -> () + i32.const 1 + global.set foo + end_function + +.globaltype foo, i32 + +# CHECK: error: mutable global imported but 'mutable-globals' feature not present in inputs: `foo`. Use --no-check-features to suppress. diff --git a/lld/test/wasm/pie.ll b/lld/test/wasm/pie.ll index c576e7c..a203d31 100644 --- a/lld/test/wasm/pie.ll +++ b/lld/test/wasm/pie.ll @@ -1,4 +1,4 @@ -; RUN: llc -relocation-model=pic -filetype=obj %s -o %t.o +; RUN: llc -relocation-model=pic -mattr=+mutable-globals -filetype=obj %s -o %t.o ; RUN: wasm-ld --no-gc-sections --allow-undefined -pie -o %t.wasm %t.o ; RUN: obj2yaml %t.wasm | FileCheck %s diff --git a/lld/test/wasm/shared.ll b/lld/test/wasm/shared.ll index 89fae33..59c1855 100644 --- a/lld/test/wasm/shared.ll +++ b/lld/test/wasm/shared.ll @@ -1,4 +1,4 @@ -; RUN: llc -relocation-model=pic -filetype=obj %s -o %t.o +; RUN: llc -relocation-model=pic -mattr=+mutable-globals -filetype=obj %s -o %t.o ; RUN: wasm-ld -shared -o %t.wasm %t.o ; RUN: obj2yaml %t.wasm | FileCheck %s diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp index 495050c..fb4b79c 100644 --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -461,6 +461,29 @@ void Writer::populateTargetFeatures() { if (!config->checkFeatures) return; + if (!config->relocatable && used.count("mutable-globals") == 0) { + for (Symbol *sym : symtab->getSymbols()) { + if (auto *global = dyn_cast(sym)) { + if (global->getGlobalType()->Mutable) { + if (!sym->isLive()) + continue; + if (!sym->isUsedInRegularObj) + continue; + if (sym->isUndefined() && sym->isWeak() && !config->relocatable) + continue; + if (sym->isUndefined()) + error(Twine("mutable global imported but 'mutable-globals' feature " + "not present in inputs: `") + + toString(*sym) + "`. Use --no-check-features to suppress."); + else if (sym->isExported()) + error(Twine("mutable global exported but 'mutable-globals' feature " + "not present in inputs: `") + + toString(*sym) + "`. Use --no-check-features to suppress."); + } + } + } + } + if (config->sharedMemory) { if (disallowed.count("shared-mem")) error("--shared-memory is disallowed by " + disallowed["shared-mem"] + -- 2.7.4