-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
LibJS: JS::Value
NaN-boxing only works correctly on x86
#15290
Comments
Yes if ARM doesn't work like x86 in that regard this is indeed incorrect. Make sure that you also make a special check in the heap stack scan at https://github.com/SerenityOS/serenity/blob/master/Userland/Libraries/LibJS/Heap/Heap.cpp#L145 (I can also have a look at this but I see you have already assigned yourself, and I don't have an ARM machine to test this) |
JS::Value stores 48 bit pointers to separately allocated objects in its payload. On x86-64, canonical addresses have their top 16 bits set to the same value as bit 47, effectively meaning that the value has to be sign-extended to get the pointer. AArch64, however, expects the topmost bits to be all zeros. This commit gates sign extension behind `#if ARCH(X86_64)`, and adds an `#error` for unsupported architectures, so that we do not forget to choose the correct behavior when porting to a new architecture. Fixes SerenityOS#15290 Fixes LadybirdBrowser/ancient-history#56
It's interesting to know that this wasn't causing an issue on a M1 (also aarch64!), I was able to ran ladybird fine on that platform. Maybe Linux uses a different virtual memory layout than MacOS? Or the M1 supports a larger address space than the machine of the person reporting the issue? |
It's possible that the M1 just tolerates these incorrect pointers, or doesn't give out much memory with the 47th bit set 🤷. |
This is what I'm suspecting. Linux places the stack at the top of the address space, that's where the high pointers are coming from. |
JS::Value stores 48 bit pointers to separately allocated objects in its payload. On x86-64, canonical addresses have their top 16 bits set to the same value as bit 47, effectively meaning that the value has to be sign-extended to get the pointer. AArch64, however, expects the topmost bits to be all zeros. This commit gates sign extension behind `#if ARCH(X86_64)`, and adds an `#error` for unsupported architectures, so that we do not forget to think about pointer handling when porting to a new architecture. Fixes SerenityOS#15290 Fixes LadybirdBrowser/ancient-history#56
JS::Value stores 48 bit pointers to separately allocated objects in its payload. On x86-64, canonical addresses have their top 16 bits set to the same value as bit 47, effectively meaning that the value has to be sign-extended to get the pointer. AArch64, however, expects the topmost bits to be all zeros. This commit gates sign extension behind `#if ARCH(X86_64)`, and adds an `#error` for unsupported architectures, so that we do not forget to think about pointer handling when porting to a new architecture. Fixes SerenityOS#15290 Fixes LadybirdBrowser/ancient-history#56
JS::Value stores 48 bit pointers to separately allocated objects in its payload. On x86-64, canonical addresses have their top 16 bits set to the same value as bit 47, effectively meaning that the value has to be sign-extended to get the pointer. AArch64, however, expects the topmost bits to be all zeros. This commit gates sign extension behind `#if ARCH(X86_64)`, and adds an `#error` for unsupported architectures, so that we do not forget to think about pointer handling when porting to a new architecture. Fixes #15290 Fixes LadybirdBrowser/ancient-history#56
JS::Value stores 48 bit pointers to separately allocated objects in its payload. On x86-64, canonical addresses have their top 16 bits set to the same value as bit 47, effectively meaning that the value has to be sign-extended to get the pointer. AArch64, however, expects the topmost bits to be all zeros. This commit gates sign extension behind `#if ARCH(X86_64)`, and adds an `#error` for unsupported architectures, so that we do not forget to think about pointer handling when porting to a new architecture. Fixes SerenityOS#15290 Fixes LadybirdBrowser/ancient-history#56
JS::Value stores 48 bit pointers to separately allocated objects in its payload. On x86-64, canonical addresses have their top 16 bits set to the same value as bit 47, effectively meaning that the value has to be sign-extended to get the pointer. AArch64, however, expects the topmost bits to be all zeros. This commit gates sign extension behind `#if ARCH(X86_64)`, and adds an `#error` for unsupported architectures, so that we do not forget to think about pointer handling when porting to a new architecture. Fixes SerenityOS#15290 Fixes LadybirdBrowser/ancient-history#56
On 64-bit platforms, NaN payloads are sign-extended to get object pointers:
serenity/Userland/Libraries/LibJS/Runtime/Value.h
Lines 452 to 454 in 2302260
This is an x86-specific behavior, and we might not have to do this for other architectures.
Action items:
#error
out on unsupported architectures instead of silently making incorrect assumptionscc @davidot
The text was updated successfully, but these errors were encountered: