1639896 - [MIPS] Support unaligned reads by baseline-interpreter

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P5)

Description

[Ted Campbell [:tcampbell]]

The BaselineInterpreter makes unaligned access to bytecode data. For platforms that care about this (MIPS?) we can add a couple extra MacroAssembler methods to unaligned loads and call them for https://searchfox.org/mozilla-central/rev/a40ef31fc9af34a99ceda6d65cdc4573d52b83d2/js/src/jit/BaselineCodeGen.cpp#368-389

For almost all platforms, it would just be a wrapper of normal masm. For MIPS, we can use the special MIPS rotation instructions for 24/32-bit loads. For unaligned 16-bit loads, we would just load the two bytes.

Comment 1

[Ted Campbell [:tcampbell]]

(This is a P5 issue and will not be fixed without someone volunteering :)

Comment 2

[André Bargull [:anba]]

Bug 1065894 already added load16UnalignedSignExtend, load16UnalignedZeroExtend, load32Unaligned, and load64Unaligned to the MacroAssembler (https://phabricator.services.mozilla.com/D74621). Jan mentioned in the review that BaselineCodeGen could switch to use those methods. (The methods themselves aren't currently implemented for MIPS, though.)

Comment 3

[Zhao Jiazhong]

Hi, I'm from Loongson Technology, and we are trying to maintain firefox on mips64el platform. I may submit a patch to fix this issue after some time, but we don't have 32-bit products, so I can't guarantee it works on mips32.

Comment 4

[Ted Campbell [:tcampbell]]

That would be great.

Generally when I test MIPS I use our simulator build on amd64 host. These builds use host machine for the C++ code, but run the JITs to target MIPS instruction set. When SpiderMonkey tries to execute the JIT code, it uses a simulator. This is far from perfect (eg. the canonical NaN stuff could not be tested), but it makes it simpler for people without hardware access.

Example moz-config I use for mips32:

ac_add_options --enable-application=js

ac_add_options --enable-debug
ac_add_options --enable-optimize

# Specify 32-bit target to use a 32-bit simulator. This still runs on 64-bit linux host
ac_add_options --target=i686-pc-linux
ac_add_options --enable-simulator=mips32

See https://firefox-source-docs.mozilla.org/js/build.html for the workflow the team currently uses to build with mach

Comment 5

[Zhao Jiazhong]

Attachment: Bug 1639896 - [MIPS] Add unaligned load and store functions to the assemblers. r=tcampbell

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7b24bbc87f56
[MIPS] Add unaligned load and store functions to the assemblers. r=tcampbell

Comment 7

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/7b24bbc87f56

Comment 8

[Zhao Jiazhong]

Comment on attachment 9154873 [details]
Bug 1639896 - [MIPS] Add unaligned load and store functions to the assemblers. r=tcampbell

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Some web sites like twitter crashed on mips64 platform due to this bug.
• User impact if declined: js or browser will crash if the program needs unaligned memory access in baseline jit.
• Fix Landed on Version: firefox79
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It only affects mips platform.
• String or UUID changes made by this patch:

Comment 9

[Ted Campbell [:tcampbell]]

This has no impact on non-MIPS builds and has been on central for a while anyways. Uplift to ESR seems quite reasonable to me.

Comment 10

[(PTO June 19-July 7) Ryan VanderMeulen]

Comment on attachment 9154873 [details]
Bug 1639896 - [MIPS] Add unaligned load and store functions to the assemblers. r=tcampbell

MIPS-only. Approved for 78.6esr.

Comment 11

[(PTO June 19-July 7) Ryan VanderMeulen]

https://hg.mozilla.org/releases/mozilla-esr78/rev/e2d8906f12b2