diff --git a/lld/test/wasm/shared-memory-no-atomics.yaml b/lld/test/wasm/shared-memory-no-atomics.yaml --- a/lld/test/wasm/shared-memory-no-atomics.yaml +++ b/lld/test/wasm/shared-memory-no-atomics.yaml @@ -49,7 +49,7 @@ Name: target_features Features: - Prefix: DISALLOWED - Name: "atomics" + Name: "shared-mem" ... # NO-SHARED: - Type: MEMORY @@ -57,4 +57,4 @@ # NO-SHARED-NEXT: - Initial: 0x00000002 # NO-SHARED-NOT: Maximum: -# SHARED: 'atomics' feature is disallowed by {{.*}}shared-memory-no-atomics.yaml.tmp1.o, so --shared-memory must not be used{{$}} +# SHARED: --shared-memory is disallowed by {{.*}}shared-memory-no-atomics.yaml.tmp1.o because it was not compiled with 'atomics' or 'bulk-memory' features. diff --git a/lld/wasm/Writer.cpp b/lld/wasm/Writer.cpp --- a/lld/wasm/Writer.cpp +++ b/lld/wasm/Writer.cpp @@ -441,21 +441,24 @@ if (!config->checkFeatures) return; - if (disallowed.count("atomics") && config->sharedMemory) - error("'atomics' feature is disallowed by " + disallowed["atomics"] + - ", so --shared-memory must not be used"); - - if (!allowed.count("atomics") && config->sharedMemory) - error("'atomics' feature must be used in order to use shared " - "memory"); - - if (!allowed.count("bulk-memory") && config->sharedMemory) - error("'bulk-memory' feature must be used in order to use shared " - "memory"); + if (config->sharedMemory) { + if (disallowed.count("shared-mem")) + error("--shared-memory is disallowed by " + disallowed["shared-mem"] + + " because it was not compiled with 'atomics' or 'bulk-memory' " + "features."); + + for (auto feature : {"atomics", "bulk-memory"}) + if (!allowed.count(feature)) + error(StringRef("'") + feature + + "' feature must be used in order to use shared memory"); + } - if (!allowed.count("bulk-memory") && tlsUsed) - error("'bulk-memory' feature must be used in order to use thread-local " - "storage"); + if (tlsUsed) { + for (auto feature : {"atomics", "bulk-memory"}) + if (!allowed.count(feature)) + error(StringRef("'") + feature + + "' feature must be used in order to use thread-local storage"); + } // Validate that used features are allowed in output if (!inferFeatures) { diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp --- a/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp @@ -230,20 +230,20 @@ void WebAssemblyAsmPrinter::EmitTargetFeatures(Module &M) { struct FeatureEntry { uint8_t Prefix; - StringRef Name; + std::string Name; }; // Read target features and linkage policies from module metadata SmallVector EmittedFeatures; - for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { - std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str(); + auto EmitFeature = [&](std::string Feature) { + std::string MDKey = (StringRef("wasm-feature-") + Feature).str(); Metadata *Policy = M.getModuleFlag(MDKey); if (Policy == nullptr) - continue; + return; FeatureEntry Entry; Entry.Prefix = 0; - Entry.Name = KV.Key; + Entry.Name = Feature; if (auto *MD = cast(Policy)) if (auto *I = cast(MD->getValue())) @@ -253,10 +253,16 @@ if (Entry.Prefix != wasm::WASM_FEATURE_PREFIX_USED && Entry.Prefix != wasm::WASM_FEATURE_PREFIX_REQUIRED && Entry.Prefix != wasm::WASM_FEATURE_PREFIX_DISALLOWED) - continue; + return; EmittedFeatures.push_back(Entry); + }; + + for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { + EmitFeature(KV.Key); } + // This pseudo-feature tells the linker whether shared memory would be safe + EmitFeature("shared-mem"); if (EmittedFeatures.size() == 0) return; diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp @@ -273,21 +273,22 @@ void recordFeatures(Module &M, const FeatureBitset &Features, bool Stripped) { for (const SubtargetFeatureKV &KV : WebAssemblyFeatureKV) { - std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str(); - if (KV.Value == WebAssembly::FeatureAtomics && Stripped) { - // "atomics" is special: code compiled without atomics may have had its - // atomics lowered to nonatomic operations. In that case, atomics is - // disallowed to prevent unsafe linking with atomics-enabled objects. - assert(!Features[WebAssembly::FeatureAtomics] || - !Features[WebAssembly::FeatureBulkMemory]); - M.addModuleFlag(Module::ModFlagBehavior::Error, MDKey, - wasm::WASM_FEATURE_PREFIX_DISALLOWED); - } else if (Features[KV.Value]) { - // Otherwise features are marked Used or not mentioned + if (Features[KV.Value]) { + // Mark features as used + std::string MDKey = (StringRef("wasm-feature-") + KV.Key).str(); M.addModuleFlag(Module::ModFlagBehavior::Error, MDKey, wasm::WASM_FEATURE_PREFIX_USED); } } + // Code compiled without atomics or bulk-memory may have had its atomics or + // thread-local data lowered to nonatomic operations or non-thread-local + // data. In that case, we mark the pseudo-feature "shared-mem" as disallowed + // to tell the linker that it would be unsafe to allow this code ot be used + // in a module with shared memory. + if (Stripped) { + M.addModuleFlag(Module::ModFlagBehavior::Error, "wasm-feature-shared-mem", + wasm::WASM_FEATURE_PREFIX_DISALLOWED); + } } }; char CoalesceFeaturesAndStripAtomics::ID = 0; diff --git a/llvm/test/CodeGen/WebAssembly/target-features-tls.ll b/llvm/test/CodeGen/WebAssembly/target-features-tls.ll --- a/llvm/test/CodeGen/WebAssembly/target-features-tls.ll +++ b/llvm/test/CodeGen/WebAssembly/target-features-tls.ll @@ -13,8 +13,8 @@ ; NO-BULK-MEM-LABEL: .custom_section.target_features,"",@ ; NO-BULK-MEM-NEXT: .int8 1 ; NO-BULK-MEM-NEXT: .int8 45 -; NO-BULK-MEM-NEXT: .int8 7 -; NO-BULK-MEM-NEXT: .ascii "atomics" +; NO-BULK-MEM-NEXT: .int8 10 +; NO-BULK-MEM-NEXT: .ascii "shared-mem" ; NO-BULK-MEM-NEXT: .bss.foo,"",@ ; +bulk-memory