Fix setDatatype endianness when writing to Wasm memory#553
Open
vinayakray19 wants to merge 2 commits into
Open
Fix setDatatype endianness when writing to Wasm memory#553vinayakray19 wants to merge 2 commits into
vinayakray19 wants to merge 2 commits into
Conversation
vinayakray19
requested review from
PiotrSikora,
leonm1,
martijneken and
mpwarres
as code owners
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
vinayakray19
force-pushed
the
bugfix/le-to-be-conversion
branch
2 times, most recently
from
2d56183 to
ccb0e1d
Compare
Wasm linear memory is little-endian, but setDatatype wrote host-native integer bytes via setMemory. On big-endian hosts such as s390x this broke hostcalls that return primitives (for example proxy_get_log_level) and could cause plugin configure failures. Apply htowasm using the VM's usesWasmByteOrder flag, matching setWord. Add a regression test that asserts setDatatype stores little-endian bytes. Signed-off-by: vinayakray19 <vinayak.ray@ibm.com>
vinayakray19
force-pushed
the
bugfix/le-to-be-conversion
branch
from
ccb0e1d to
0e3cec0
Compare
Contributor
|
Thank you for your contribution! Could you add a test to test/endianness_test.cc that uses setDatatype? That way, we have an end-to-end test to ensure the bytes writen by the host are properly interpreted by the wasm module. |
Author
|
sure @leonm1 |
Extend endianness.wasm with test_host_set_datatype, which calls proxy_get_log_level and verifies the u32 written by the host via setDatatype. EndiannessContext returns 0x02000001 so the test fails on big-endian hosts if bytes are not converted to Wasm little-endian order. Signed-off-by: vinayakray19 <vinayak.ray@ibm.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
WasmBase::setDatatypeto convert integers to Wasm (little-endian) byte order viahtowasmbefore writing to linear memorySetDatatypeWritesLittleEndianregression test inwasm_vm_test.ccFixes #552
Problem
On big-endian hosts (s390x),
setDatatypewrote host-native integer bytes into Wasm memory. This broke hostcalls that return primitives via output pointers (e.g.proxy_get_log_level) and could causeproxy_on_configurefailures in Rust Wasm plugins.setWordalready handled endianness correctly;setDatatypedid not.Test Added
SetDatatypeWritesLittleEndianregression test (asserts LE bytes01 00 00 02for value0x02000001)