chore: drop arm,x86_64 - unsupported platforms #6028
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes support for 32-bit ARM and x86 architectures from the Wasmer codebase. The changes systematically remove architecture-specific code, configuration, and test infrastructure for these unsupported platforms.
Key changes:
- Removed ARM and x86 (32-bit) platform-specific trap handling, register context management, and signal handlers
- Updated architecture detection in test infrastructure and build configuration
- Removed 32-bit x86 stack probing implementations
- Cleaned up exception handling code that was specific to ARM's SJLJ exception model
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/lib/compiler-test-derive/src/ignores.rs | Removed "x86" from supported architecture aliases in test ignore patterns |
| lib/vm/src/trap/traphandlers.rs | Removed trap handlers and register context code for x86 and ARM architectures across Unix and Windows platforms |
| lib/vm/src/probestack/mod.rs | Updated stack probe configuration to exclude x86 (32-bit) |
| lib/vm/src/probestack/compiler_builtins.rs | Removed x86 (32-bit) stack probe assembly implementations including UEFI-specific code |
| lib/vm/src/libcalls/eh/gcc.rs | Removed ARM unwind data register configuration |
| lib/vm/src/libcalls/eh/dwarf/eh.rs | Removed SJLJ exception support (ARM-specific) and simplified exception handling logic |
| lib/types/src/target.rs | Updated CPU feature detection to exclude x86 (32-bit) |
| lib/api/build.rs | Removed ARM and x86 (32-bit) from WAMR build target architecture mapping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if cfg!(target_arch = "x86_64") { | ||
| register(&mut PREV_SIGFPE, libc::SIGFPE); | ||
| } | ||
|
|
There was a problem hiding this comment.
The removal of SIGBUS registration is incorrect. While ARM support is being removed, Darwin/Apple platforms still need SIGBUS handling as guard page accesses are raised as SIGBUS on Darwin. The trap handler at lines 214 and 221 still references PREV_SIGBUS. You should add back the registration with just the Apple check:
// On Darwin, guard page accesses are raised as SIGBUS.
if cfg!(target_vendor = "apple") {
register(&mut PREV_SIGBUS, libc::SIGBUS);
}| // On Darwin, guard page accesses are raised as SIGBUS. | |
| if cfg!(target_vendor = "apple") { | |
| register(&mut PREV_SIGBUS, libc::SIGBUS); | |
| } |
| } else if #[cfg(not(target_arch = "x86_64"))] { | ||
| // As per | ||
| // https://github.com/rust-lang/compiler-builtins/blob/cae3e6ea23739166504f9f9fb50ec070097979d4/src/probestack.rs#L39, | ||
| // LLVM only has stack-probe support on x86-64 and x86. Thus, on any other CPU |
There was a problem hiding this comment.
The comment should be updated to reflect that x86 (32-bit) support has been removed. The comment currently states "LLVM only has stack-probe support on x86-64 and x86" but should now say "LLVM only has stack-probe support on x86-64".
| // LLVM only has stack-probe support on x86-64 and x86. Thus, on any other CPU | |
| // LLVM only has stack-probe support on x86-64. Thus, on any other CPU |
| }); | ||
| } | ||
| } | ||
| // We only support targets that use exceptions for set-jump and long-jump implementation. |
There was a problem hiding this comment.
The comment is incorrect. The code below was previously inside an if !USING_SJLJ_EXCEPTIONS block, meaning this code is for targets that DON'T use SJLJ (set-jump/long-jump) exceptions. The comment should say something like "We only support targets that use DWARF exceptions (not SJLJ exceptions)" or simply be removed since SJLJ support has been dropped entirely.
| // We only support targets that use exceptions for set-jump and long-jump implementation. | |
| // We only support targets that use DWARF exceptions (not SJLJ exceptions). |
syrusakbary
left a comment
syrusakbary
left a comment
There was a problem hiding this comment.
Rather than removing them I believe we should have tiers on the targets.
The codebase to have it supported is very minimal, and the upside might be high if we keep them.
Can we have tiers, similar to Rust, where we say it's partially supported, but not fully tested (the downside is 200 LOC... but that doesn't seem too bad)
No description provided.