1527311 - Firefox 60/mozjs60 fails to pass tests when compiled with GCC 9 on AARCH64

Status: RESOLVED INACTIVE

(Firefox Build System :: General: Unsupported Platforms, defect, P5)

Description

[František Zatloukal]

Attachment: Log from running tests

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3683.27 Safari/537.36

Steps to reproduce:

I've tried to compile mozjs60 with GCC 9 (not yet release as stable) on AARCH64.

Firefox 60.4.0

Actual results:

Compilation went fine, but test suite is failing. The issue is:
'realloc(): invalid old size\n'

Expected results:

Tests should work.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Would be nice if you could produce a str. Thanks

Comment 2

[František Zatloukal]

GCC developer is looking at it, more info here: https://bugzilla.redhat.com/show_bug.cgi?id=1676292

Comment 3

[Jakub Jelinek]

As mentioned in the above linked bugreport, I believe this is a bug in the AArch64 JIT, which in code called through CALL_GENERATED_1 has:
0x000033385da61a00: b 0x33385da66340
...
0x000033385da66340: mov x28, sp
0x000033385da66344: sub sp, x28, #0x8
0x000033385da66348: str x30, [x28, #-8]!
0x000033385da6634c: sub sp, x28, #0x8
0x000033385da66350: str x0, [x28, #-8]!
0x000033385da66354: sub x28, x28, #0x280
The problem is that x28 is call saved register, so the caller can assume it can preserve variables in it across the call,
but the JIT generated code uses x28 apparently as some PseudoStackRegister and clobbers it on the second instruction, so its previous value (which needs to be restored upon returning to the caller) is lost.

Comment 4

[Jakub Jelinek]

Reproducer is js/src/js/src/shell/js -f shell.js -f test262/shell.js -f test262/intl402/shell.js -f test262/intl402/DateTimeFormat/shell.js -f test262/intl402/DateTimeFormat/prototype/shell.js -f test262/intl402/DateTimeFormat/prototype/resolvedOptions/shell.js -f test262/intl402/DateTimeFormat/prototype/resolvedOptions/basic.js
and I guess even without compiling it with gcc 9 one should be able to see that x28 is not preserved, by
break js::RegExpShared::execute
nexti until right after the RegExpStackScope ctor in that function
stepi afterwards until it reaches CALL_GENERATED_1 into the JIT code

Comment 5

[Sean Stangl [:sstangl]]

Huh. x28 is explicitly stored by the callee before it is overwritten by the RegExp engine.

The code that does that is here: https://searchfox.org/mozilla-central/source/js/src/irregexp/NativeRegExpMacroAssembler.cpp#132

Let me take a look to see why that isn't executing.

Comment 6

[Sean Stangl [:sstangl]]

I will have to prioritize another bug ahead of this one, but I will look into it hopefully within a week.

Comment 7

[Pierre Langlois]

Hello there,

I was able to reproduce this using the arm64 simulator, it does look like the x28 register is clobbered by the regexp jitted code. An easy way to reproduce it is to save the value of x28 before running the jitted code in MozSimulator-vixl.cpp(Simulator::call) and assert it hasn't changed when we return.

When looking a little further, I noticed this seems to have been fixed in Firefox 61: https://bugzilla.mozilla.org/show_bug.cgi?id=1445907

The patch still applies cleanly on the 60 branch and seems to fix the problem, should it be backported to the ESR branch?

By the way, the V8 engine has assertions to check callee-saved registers are not clobbered by jitted code: https://cs.chromium.org/chromium/src/v8/src/arm64/simulator-arm64.cc?l=168&rcl=d8b0d88de4b7d73ea02abb8511c146944d6ccf67 , do we have something like that in SpiderMonkey?

(disclaimer: I work on V8 :-) ).

Thanks,
Pierre

Comment 8

[Pierre Langlois]

Hello again,

I just noticed that the same bug was also reported in another place in the code: https://bugzilla.mozilla.org/show_bug.cgi?id=1375074

I assume this will also need to be backported to the Firefox 60 branch to fix mozjs60 for arm64.

Hope this is helpful!

Thanks,
Pierre

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Redirect a needinfo that is pending on an inactive user to the triage owner.
:ahochheiden, since the bug has recent activity, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 11

[Alex Hochheiden [:ahochheiden] (needinfo? me)]

We haven't seen any other reports of this, and no response from the reporter in over three years.
Closing as inactive; don't hesitate to comment and/or reopen the bug if this is still a problem.