Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modified jsglue so that it does not suffer from abi incompatibilities #9

Merged
merged 1 commit into from Mar 28, 2013

Conversation

@ILyoan
Copy link
Contributor

ILyoan commented Mar 22, 2013

Convert from jsval to uint64_t and vice versa.

Rust expected uint64_t while C side code uses jsval(64bit union type) but this kind of conversion is incompatible on some platform ABIs (like ARM EABI). So I added converting code that performs the converting safely.

@jdm
Copy link
Member

jdm commented Mar 22, 2013

Could you explain further why the existing conversion is incompatible? I'm having trouble understanding whether this is just a symptom of the way we currently hardcode jsval representation to work for x86-64 or a more general problem that we don't hit with Gecko on ARM.

@ILyoan
Copy link
Contributor Author

ILyoan commented Mar 22, 2013

AFAIK, different function signature between caller and calle can be dangerous.
I don't know much about X86 or X86_64 but in this case, ARM Procedure Call Standard(APCS) does not allow this kind of conversion. APCS says

  • A double-word sized Fundamental Data Type(e.g., long long, double and 64-bit containerized vectors) is returned in r0 and r1.
  • A Composite Type larger than 4 bytes, or whose size cannot be determined statically by both caller and callee, is stored in memory at an address passed as an extra argument when the function was called (§5.5, rule A.4). The memory to be used for the result may be modified at any point during the function call.

According to the existing conversion, Rust side expects jsval to be returned as u64(the first option on the above), while C returns it as 64-bit composite type(second one). In this case, the stack frame between two side are different(so callee side will not get argument correctly), and C side saves return value into stack but Rust side will check r0 and r1 register.

Passing argument as u64 and receiving it as jsval looks fine at least on ARM Architecture. but it can be harmful on other platforms so I also added uint64_to_jsval conversion too.

pcwalton added a commit that referenced this pull request Mar 28, 2013
 Modified jsglue so that it does not suffer from abi incompatibilities
@pcwalton pcwalton merged commit 5b246c1 into servo:master Mar 28, 2013
@sanxiyn sanxiyn mentioned this pull request Mar 28, 2013
mmatyas pushed a commit to mmatyas/rust-mozjs that referenced this pull request Jul 30, 2015
Update CompositionOp C bindings to fix tests
tschneidereit pushed a commit to tschneidereit/rust-mozjs that referenced this pull request Aug 26, 2017
Make the GC inhibition APIs use a counter rather than a boolean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.