Skip to content

wasm2c: cleanup big-endian to use non-reversed memory - #2839

Open
shravanrn wants to merge 1 commit into
WebAssembly:mainfrom
UT-Security:fix-be
Open

wasm2c: cleanup big-endian to use non-reversed memory#2839
shravanrn wants to merge 1 commit into
WebAssembly:mainfrom
UT-Security:fix-be

Conversation

@shravanrn

@shravanrn shravanrn commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Wasm2c currently generates code for big endian that swaps the direction of linear memory (it grows towards lower addresses like a stack). This complicates a lot of runtime code which now has to be aware of this linear memory direction swap. This PR implements an alternate & simpler approach of bitswapping values when reading/writing to/from linear memory, and keeping the linear memory growth in the conventional direction. This helps eliminate a lot of conditional code in the runtime, keeping things much simpler for the portions of wasm2c that change more often, while isolating the big-endian changes to the insides of memory instruction implementations which change less frequently.

@shravanrn
shravanrn requested review from keithw and sbc100 August 27, 2026 03:59
@shravanrn

shravanrn commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@keithw fyi, this is a necessary step for cleaning up the Wasm mmap/malloc, guard-page/bounds-check configuration code in wasm2c, which in turn should be done prior to moving to compile time flags for configs

@keithw

keithw commented Aug 27, 2026

Copy link
Copy Markdown
Member

This is certainly fine with me -- the tradeoff seems to be a reduction in complexity in exchange for a (possible) slowdown on big-endian platforms. I know @SoniEx2 put a lot of effort into this fancy BE implementation, so let's give them a chance to weigh in if they choose, but at least for me personally I suspect our actual users on BE platforms are essentially nonexistent so if streamlining helps you, let's do it.

@keithw

keithw commented Aug 27, 2026

Copy link
Copy Markdown
Member

I guess if we do care about conformance on BE platforms, we should also figure out why the s390x test is not running in CI...

@shravanrn

shravanrn commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@keithw Sounds good, happy to wait a few days to give @SoniEx2 a chance to weigh in. Separately, if the change looks alright, could you ok the code review?

Re the s390x failure - Looks like this is tracked in #2655 I did test locally on qemu that all the s390x tests pass, so this change should be fine, but I can try to see if I can fix #2655 as a separate follow up

I suspect our actual users on BE platforms are essentially nonexistent

I can confirm at least that Firefox does not use this because RLBox is incompatible with Wasm in BE platforms.

I guess if we do care about conformance on BE platforms

Well, this PR should make it easy to keep the BE support in place without too much work. So in this form, continued conformance should be ok.

for a (possible) slowdown

Fwiw, it does mean that some of the optimization work i did for general LE wasm2c (#2804) now applies to BE too since the memory layout is the same. So unclear what the overall performance implications are, given that qemu won't tell me anything about real performance.

@sbc100

sbc100 commented Aug 27, 2026

Copy link
Copy Markdown
Member

@shravanrn are you using wasm2c with bigendian targets in production?

@shravanrn

shravanrn commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@shravanrn are you using wasm2c with bigendian targets in production?

@sbc100 No and no plans to do so. This is just a reworking of the existing support so it becomes less painful to improve wasm2c without having to constantly consider behavioral differences between LE vs BE targets going forward.

@shravanrn
shravanrn requested a review from SoniEx2 August 27, 2026 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants