[CodeGen][ObjC] Initial WebAssembly Support for GNUstep #169043
Conversation
llvmbot
added
backend:WebAssembly
clang:driver
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-backend-webassembly Author: Hugo Melder (hmelder) ChangesThis pull request adds initial support for compiling Objective-C to WebAssembly. I tested my changes with libobjc2 and the swift-corelibs-blocksruntime. There are two outstanding issues, which I cannot fix as deeper knowledge of the subsystems is required:
First IssueEmscripten is processing the generated The core of the problem is that symbols with the I'm currently hacking around this by not exporting no-strip symbols. This is the default behaviour for Wasm. Second IssueHere is a minimal example that triggers the crash. #include<stdio.h>
int main(void) {
int ret = 0;
@<!-- -->try {
}
@<!-- -->catch (id a)
{
ret = 1;
puts("abc");
}
return ret;
}The following assertion is triggered: Here is the crash report main-c3884.zip. You can use emcc -fobjc-runtime=gnustep-2.2 -fwasm-exceptions -c main.mor just invoke clang directly: /home/vm/llvm-build-wasm/bin/clang -target wasm32-unknown-emscripten -mllvm -combiner-global-alias-analysis=false -mllvm -wasm-enable-sjlj -mllvm -wasm-use-legacy-eh=false -mllvm -disable-lsr --sysroot=/home/vm/emsdk/upstream/emscripten/cache/sysroot -DEMSCRIPTEN -fobjc-runtime=gnustep-2.2 -fwasm-exceptions -c main.mBuilding libobjc2 and the BlocksRuntimeBuilding the BlocksRuntimecmake -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_INSTALL_PREFIX=/home/vm/demo-install -DCMAKE_BUILD_TYPE=Debug -B build -G NinjaBuilding libobjc2cmake -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_INSTALL_PREFIX=/home/vm/demo-install -DBlocksRuntime_LIBRARIES=/home/vm/demo-install/lib/libBlocksRuntime.a -DBlocksRuntime_INCLUDE_DIR=/home/vm/demo-install/include/BlocksRuntime -DEMBEDDED_BLOCKS_RUNTIME=OFF -DTESTS=OFF -B build -DCMAKE_BUILD_TYPE=Debug -G NinjaFull diff: https://github.com/llvm/llvm-project/pull/169043.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 06643d4bdc211..3b9f9f306829d 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -179,8 +179,15 @@ class CGObjCGNU : public CGObjCRuntime {
(R.getVersion() >= VersionTuple(major, minor));
}
- std::string ManglePublicSymbol(StringRef Name) {
- return (StringRef(CGM.getTriple().isOSBinFormatCOFF() ? "$_" : "._") + Name).str();
+ const std::string ManglePublicSymbol(StringRef Name) {
+ auto triple = CGM.getTriple();
+
+ // Exported symbols in Emscripten must be a valid Javascript identifier.
+ if (triple.isOSBinFormatCOFF() || triple.isOSBinFormatWasm()) {
+ return (StringRef("$_") + Name).str();
+ } else {
+ return (StringRef("._") + Name).str();
+ }
}
std::string SymbolForProtocol(Twine Name) {
@@ -4106,8 +4113,7 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() {
if (!ClassAliases.empty()) {
llvm::Type *ArgTypes[2] = {PtrTy, PtrToInt8Ty};
llvm::FunctionType *RegisterAliasTy =
- llvm::FunctionType::get(Builder.getVoidTy(),
- ArgTypes, false);
+ llvm::FunctionType::get(BoolTy, ArgTypes, false);
llvm::Function *RegisterAlias = llvm::Function::Create(
RegisterAliasTy,
llvm::GlobalValue::ExternalWeakLinkage, "class_registerAlias_np",
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 30d3e5293a31b..6cbec5e17ae1a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8001,7 +8001,8 @@ ObjCRuntime Clang::AddObjCRuntimeArgs(const ArgList &args,
if ((runtime.getKind() == ObjCRuntime::GNUstep) &&
(runtime.getVersion() >= VersionTuple(2, 0)))
if (!getToolChain().getTriple().isOSBinFormatELF() &&
- !getToolChain().getTriple().isOSBinFormatCOFF()) {
+ !getToolChain().getTriple().isOSBinFormatCOFF() &&
+ !getToolChain().getTriple().isOSBinFormatWasm()) {
getToolChain().getDriver().Diag(
diag::err_drv_gnustep_objc_runtime_incompatible_binary)
<< runtime.getVersion().getMajor();
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 15590b31fd07f..d882146e21b8a 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -1794,9 +1794,6 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
Flags |= wasm::WASM_SYMBOL_UNDEFINED;
if (WS.isNoStrip()) {
Flags |= wasm::WASM_SYMBOL_NO_STRIP;
- if (isEmscripten()) {
- Flags |= wasm::WASM_SYMBOL_EXPORTED;
- }
}
if (WS.hasImportName())
Flags |= wasm::WASM_SYMBOL_EXPLICIT_NAME;
|
|
@llvm/pr-subscribers-llvm-mc Author: Hugo Melder (hmelder) ChangesThis pull request adds initial support for compiling Objective-C to WebAssembly. I tested my changes with libobjc2 and the swift-corelibs-blocksruntime. There are two outstanding issues, which I cannot fix as deeper knowledge of the subsystems is required:
First IssueEmscripten is processing the generated The core of the problem is that symbols with the I'm currently hacking around this by not exporting no-strip symbols. This is the default behaviour for Wasm. Second IssueHere is a minimal example that triggers the crash. #include<stdio.h>
int main(void) {
int ret = 0;
@<!-- -->try {
}
@<!-- -->catch (id a)
{
ret = 1;
puts("abc");
}
return ret;
}The following assertion is triggered: Here is the crash report main-c3884.zip. You can use emcc -fobjc-runtime=gnustep-2.2 -fwasm-exceptions -c main.mor just invoke clang directly: /home/vm/llvm-build-wasm/bin/clang -target wasm32-unknown-emscripten -mllvm -combiner-global-alias-analysis=false -mllvm -wasm-enable-sjlj -mllvm -wasm-use-legacy-eh=false -mllvm -disable-lsr --sysroot=/home/vm/emsdk/upstream/emscripten/cache/sysroot -DEMSCRIPTEN -fobjc-runtime=gnustep-2.2 -fwasm-exceptions -c main.mBuilding libobjc2 and the BlocksRuntimeBuilding the BlocksRuntimecmake -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_INSTALL_PREFIX=/home/vm/demo-install -DCMAKE_BUILD_TYPE=Debug -B build -G NinjaBuilding libobjc2cmake -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_INSTALL_PREFIX=/home/vm/demo-install -DBlocksRuntime_LIBRARIES=/home/vm/demo-install/lib/libBlocksRuntime.a -DBlocksRuntime_INCLUDE_DIR=/home/vm/demo-install/include/BlocksRuntime -DEMBEDDED_BLOCKS_RUNTIME=OFF -DTESTS=OFF -B build -DCMAKE_BUILD_TYPE=Debug -G NinjaFull diff: https://github.com/llvm/llvm-project/pull/169043.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 06643d4bdc211..3b9f9f306829d 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -179,8 +179,15 @@ class CGObjCGNU : public CGObjCRuntime {
(R.getVersion() >= VersionTuple(major, minor));
}
- std::string ManglePublicSymbol(StringRef Name) {
- return (StringRef(CGM.getTriple().isOSBinFormatCOFF() ? "$_" : "._") + Name).str();
+ const std::string ManglePublicSymbol(StringRef Name) {
+ auto triple = CGM.getTriple();
+
+ // Exported symbols in Emscripten must be a valid Javascript identifier.
+ if (triple.isOSBinFormatCOFF() || triple.isOSBinFormatWasm()) {
+ return (StringRef("$_") + Name).str();
+ } else {
+ return (StringRef("._") + Name).str();
+ }
}
std::string SymbolForProtocol(Twine Name) {
@@ -4106,8 +4113,7 @@ llvm::Function *CGObjCGNU::ModuleInitFunction() {
if (!ClassAliases.empty()) {
llvm::Type *ArgTypes[2] = {PtrTy, PtrToInt8Ty};
llvm::FunctionType *RegisterAliasTy =
- llvm::FunctionType::get(Builder.getVoidTy(),
- ArgTypes, false);
+ llvm::FunctionType::get(BoolTy, ArgTypes, false);
llvm::Function *RegisterAlias = llvm::Function::Create(
RegisterAliasTy,
llvm::GlobalValue::ExternalWeakLinkage, "class_registerAlias_np",
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 30d3e5293a31b..6cbec5e17ae1a 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -8001,7 +8001,8 @@ ObjCRuntime Clang::AddObjCRuntimeArgs(const ArgList &args,
if ((runtime.getKind() == ObjCRuntime::GNUstep) &&
(runtime.getVersion() >= VersionTuple(2, 0)))
if (!getToolChain().getTriple().isOSBinFormatELF() &&
- !getToolChain().getTriple().isOSBinFormatCOFF()) {
+ !getToolChain().getTriple().isOSBinFormatCOFF() &&
+ !getToolChain().getTriple().isOSBinFormatWasm()) {
getToolChain().getDriver().Diag(
diag::err_drv_gnustep_objc_runtime_incompatible_binary)
<< runtime.getVersion().getMajor();
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index 15590b31fd07f..d882146e21b8a 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -1794,9 +1794,6 @@ uint64_t WasmObjectWriter::writeOneObject(MCAssembler &Asm,
Flags |= wasm::WASM_SYMBOL_UNDEFINED;
if (WS.isNoStrip()) {
Flags |= wasm::WASM_SYMBOL_NO_STRIP;
- if (isEmscripten()) {
- Flags |= wasm::WASM_SYMBOL_EXPORTED;
- }
}
if (WS.hasImportName())
Flags |= wasm::WASM_SYMBOL_EXPLICIT_NAME;
|
🐧 Linux x64 Test Results
|
|
@sunfishcode, I see that you are the original author of https://reviews.llvm.org/D62542. As @dschuff said in the review back then:
This was in 2019, is this hack still required now that fastcomp is deprecated? Then problem with the current behaviour is that hidden no-strip symbols, added during codegen, are exported. |
|
@davidchisnall the changes in codegen are trivial:
|
|
Assuming that the new WASM exception implementation implements the mandatory functions and data structure of the Itanium EH ABI correctly, not much needs to be done to get EH working with libobjc2. I just need to find the root course of the crash... |
davidchisnall
left a comment
davidchisnall
left a comment
There was a problem hiding this comment.
The Objective-C bits look fine to me, the MC bit possibly should be a separate PR.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
davidchisnall
left a comment
davidchisnall
left a comment
There was a problem hiding this comment.
Code LGTM once clang-format is happy.
We probably should have a test in tests/CodeGenObjC checking that the mangling is correct for WAsm.
Emscripten requires that exported symbols. See emscripten-core/emscripten#23563.
|
+cc @sbc100 about issue 1 and @aheejin about issue 2. Pardon my ignorance about objc here... When you say "explicitly hidden" what do you mean exactly? Do you mean something like On exception handling, I took a quick look at the IR output of your example. The cc1 command includes |
|
|
||
| // Exported symbols in Emscripten must be a valid Javascript identifier. | ||
| auto triple = CGM.getTriple(); | ||
| if (triple.isOSBinFormatCOFF() || triple.isOSBinFormatWasm()) { |
There was a problem hiding this comment.
The restriction on valid JS identifiers is specific to Emscripten rather than wasm as a whole, so you might want to check for isOSEmscripten here rather than the bin format. But if you want to have a common ABI across Emscripten and WASI, then this would be OK with me too.
There was a problem hiding this comment.
We should maybe fix emscripten to deal with these symbols instead of patching here?
There was a problem hiding this comment.
Yeah that makes sense. we could just do a similar prefix or substitution as the one here? If the export name is invalid, we could just mangle the symbol on the Module object? Or have an alias so Module[".realSymbol"] could keep working?
There was a problem hiding this comment.
Yes, we can do that. But remember that symbols are also available directly in the module scope as normal variables.
e.g. One can just write _malloc for symbols that are not exported on the Module. For exported symbols one can also write Module['_malloc']. So this change would just mean that symbol are that not valid JS symbol names would not be accessible via the first method... which is an odd difference but maybe better than "link failure" ?
There was a problem hiding this comment.
Yeah, better than link failure I think. And given that the map style accessors aren't going away, maybe it's sufficient to just leave it at that. where if you have invalid identifiers, you just need to use that method (as opposed to trying to mangle them and change the symbol name altogether?)
There was a problem hiding this comment.
OK, I'll look into fixing this now on the emscripten side. We have an open bug there already: emscripten-core/emscripten#24825
There was a problem hiding this comment.
@hmelder / @HendrikHuebner Would Sam's Emscripten change be useful for the objc use case?
There was a problem hiding this comment.
The restriction on valid JS identifiers is specific to Emscripten rather than wasm as a whole, so you might want to check for isOSEmscripten here rather than the bin format. But if you want to have a common ABI across Emscripten and WASI, then this would be OK with me too.
I think it does not really matter whether we use $ or . and I would prefer to not add more complexity into the conditional.
We should maybe fix emscripten to deal with these symbols instead of patching here?
[...]
So this change would just mean that symbol are that not valid JS symbol names would not be accessible via the first method... which is an odd difference but maybe better than "link failure" ?
We mangle the symbols so that they are not directly usable by the user in a C program, or at least this was the original intend with . on ELF. AFAIK there is an extension that allows $ to be used in a C identifier which is why Emscripten supports it in the first place.
You can change this in Emscripten but it does not really matter for our use case.
In this case we now generate a warning instead of an errors. Such symbols are not directly accessible in the module scope (since we cannot declare them there). They are only accessible via `wasmExports` or `Module` dictionary objects. See: llvm/llvm-project#169043 Fixes: emscripten-core#24825
|
@sbc100 I also re-read First Issue in OP, and it looks like (mangling aside) the underlying problem is that symbols are being exported that weren't intended to be in the first place. It looks like maybe OP is running into the issue of Emscripten's hack from the ancient times where EMSCRIPTEN_KEEPALIVE (which compiles to attribute("used") and then WASM_SYM_NO_STRIP) then also implies WASM_SYM_EXPORTED. IIUC they want to keep these symbols from being stripped but don't want them exported. Normally these unused exports are not necessarily a problem, they are just extra binary size that can removed with metadce. But when they cause linker errors because they are invalid C then everything breaks. So landing emscripten-core/emscripten#24825 will keep everything from breaking (we'd probably also want to suppress the warning), and it seems useful independently. But it might still be also useful to take another crack at the EMSCRIPTEN_KEEPALIVE problem. It seems sticky because there are likely users depending on the fact that attribute("used") causes exporting (although hopefully most are using EMSCRIPTEN_KEEPALIVE instead of directly). But there are probably other uses of attribute("used") that just want to prevent stripping, and we can't tell them apart. We could maybe just add 2 distinct attributes ("just-no-dead-strip" and "just-export-unless-dead") and deprecate "used" but keep it working ~forever. Not sure if there's a better way. |
In this case we now generate a warning instead of an errors. Such symbols are not directly accessible in the module scope (since we cannot declare them there). They are only accessible via `wasmExports` or `Module` dictionary objects. See: llvm/llvm-project#169043 Fixes: emscripten-core#24825
In this case we now generate a warning instead of an errors. Such symbols are not directly accessible in the module scope (since we cannot declare them there). They are only accessible via `wasmExports` or `Module` dictionary objects. See: llvm/llvm-project#169043 Fixes: emscripten-core#24825
|
Take llvm-project/clang/lib/CodeGen/CGObjCGNU.cpp Lines 1578 to 1580 in 917e458 which is placed into comdat and explicitly marked as hidden here llvm-project/clang/lib/CodeGen/CGObjCGNU.cpp Lines 1598 to 1599 in 917e458 When compiling a basic Objective-C application with emcc (
Which originates from for e in settings.EXPORTED_FUNCTIONS:
if not js_manipulation.isidentifier(e):
exit_with_error(f'invalid export name: "{e}"')The hidden symbol is exported because the no-strip attributed was added implicitly, causing it to be exported in llvm-project/llvm/lib/MC/WasmObjectWriter.cpp Lines 1795 to 1800 in 2aa3450 |
Exactly this is the problem. |
|
There is also some work to be done to support other personality functions in WebAssembly. Basic code generation with the wrong personality function works with this patch.
The personality I would like to avoid defining two new personality functions for ObjC and ObjCXX on WASM. Can't we just differentiate and emit code based on the target triple? |
I don't understand why we need |
It seems like there are actually two different issues:
We already have a fix for (2) in flight which works around this issue for now I believe. The fix for (1) is harder, but I would like to get this fixed eventually. The problem stems from the fact that we have an |
If objc follows exactly the same ABI and runtime as C++ then maybe it can just use the same personality function. I think we do want to support different personality functions, but maybe the test is which parts of the runtime code they want to share. IIRC we also have a separate personality function for using wasm EH for setjmp and longjmp (or maybe that's just a different tag?). I'd want to get @aheejin's opinion about the best design for this. But we can certainly make this work one way or another. |
|
I don't know why the GNUStep runtime needs different ObjC and ObjC++ personality functions. The ObjC++ personality has to support the complete superset of exception clauses (a single call site can have handlers for both C++ and ObjC exception types). It should therefore still be usable in either pure ObjC or pure C++. In theory, the ObjC++ personality needs to be able to distinguish ObjC and C++ exceptions, so using it instead of a single-language personality could require a less compact LSDA format. In practice, they use the exact same format, and AFAIK the exception types are always distinguished in a more subtle way. I don't know the exact details of what GNUStep does, but the Apple ObjC runtime just provides a v-table for ObjC exception RTTI objects that implements the private exception-matching virtual methods on type_info differently. (I'm not even sure LLVM supports emitting a different LSDA format for different personalities.) |
In this case we now generate a warning instead of an errors. Such symbols are not directly accessible in the module scope (since we cannot declare them there). They are only accessible via `wasmExports` or `Module` dictionary objects. See: llvm/llvm-project#169043 Fixes: #24825, #23560
|
Sorry for the late reply. My email filters weren't set up right...
Calls to llvm-project/llvm/lib/CodeGen/WasmEHPrepare.cpp Lines 370 to 372 in f440b5c
I think the name of the personality function can be either the same or different, depending on your preference. Even if the name is the same, the library you link will be different (i.e., it's not going to be libc++abi), so as long as the signature matches the name doesn't matter much. It would be slightly simpler if your personality function has the same name because we don't need to fix WasmEHPrepare though. EDIT: I assumed you can't share libc++abi, but if you can, things can be simpler. |
It's a long ago, but I think I made it that way in order to minimize the changes to libc++abi so that we share most of the code with the other platforms. Also the intention was to hide Wasm-specific low-level details in libunwind, such as the use of the variable
It's more of our convention than ABI, so I don't think it's doable if we really need to, but for our libraries I think the current code works fine. You can either
If 2 is not too much burden, I think it would help make WasmEHPrepare simpler. |
We don't use a personality function to handle SjLj. I think what you're talking about might be this? llvm-project/libcxxabi/src/cxa_personality.cpp Lines 1029 to 1030 in f440b5c This is used in SjLj exception handling, which I believe is a kind of EH used in x86. |
If ObjC can share libc++abi (with little |
This is because EH handling has not been implemented. llvm-project/llvm/lib/CodeGen/MachineFunction.cpp Lines 237 to 240 in d502ff0 llvm-project/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp Lines 330 to 351 in d502ff0 If we decide to use the same personality function (or the same name), I think it will work. Or, if the name is different, we should make it be classified as a Wasm personality function. |
8 participants
This pull request adds initial support for compiling Objective-C to WebAssembly. I tested my changes with libobjc2 and the swift-corelibs-blocksruntime.
There are two outstanding issues, which I cannot fix as deeper knowledge of the subsystems is required:
-fwasm-exceptionsFirst Issue
Emscripten is processing the generated
.wasmfile inemscripten.pyand checks if all exported symbols are valid javascript identifiers (tools/js_manipulation.py#L104). However, hidden symbols such as.objc_initare intentionally an invalid C identifier.The core of the problem is that symbols with the
WASM_SYMBOL_NO_STRIPattribute are exported when targeting Emscripten (https://reviews.llvm.org/D62542). This attribute is added to the symbol during relocation inWasmObjectWriter::recordRelocation. So we are accidentally exporting a lot of hidden symbols and not only ones generated by ObjC CG...I'm currently hacking around this by not exporting no-strip symbols. This is the default behaviour for Wasm.
Second Issue
Here is a minimal example that triggers the crash.
The following assertion is triggered:
Here is the crash report main-c3884.zip.
You can use
emccwith a modified LLVM build by exportingEM_LLVM_ROOTbefore sourcingemsdk/emsdk_env.sh:or just invoke clang directly:
Building libobjc2 and the BlocksRuntime
Building the BlocksRuntime
cmake -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_INSTALL_PREFIX=/home/vm/demo-install -DCMAKE_BUILD_TYPE=Debug -B build -G NinjaBuilding libobjc2
cmake -DCMAKE_TOOLCHAIN_FILE=$EMSDK/upstream/emscripten/cmake/Modules/Platform/Emscripten.cmake -DCMAKE_INSTALL_PREFIX=/home/vm/demo-install -DBlocksRuntime_LIBRARIES=/home/vm/demo-install/lib/libBlocksRuntime.a -DBlocksRuntime_INCLUDE_DIR=/home/vm/demo-install/include/BlocksRuntime -DEMBEDDED_BLOCKS_RUNTIME=OFF -DTESTS=OFF -B build -DCMAKE_BUILD_TYPE=Debug -G Ninja