1618880 - Intermittent Assertion failure: hasScript(), at /builds/worker/workspace/build/src/js/src/vm/JSFunction.h:751

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: ncsoregi [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=290966285&repo=autoland
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/KvDGc8BBTcyXjTc1xXDBBw/runs/0/artifacts/public/logs/live_backing.log

---

[task 2020-02-28T12:34:38.663Z] TEST-PASS | js/src/jit-test/tests/basic/bug-1133377.js | Success (code 0, args "--ion-eager --ion-offthread-compile=off --more-compartments") [0.6 s]
[task 2020-02-28T12:34:38.694Z] TEST-PASS | js/src/jit-test/tests/basic/bug-1133377.js | Success (code 0, args "--baseline-eager") [0.6 s]
[task 2020-02-28T12:34:38.712Z] Assertion failure: hasScript(), at /builds/worker/workspace/build/src/js/src/vm/JSFunction.h:751
[task 2020-02-28T12:34:38.712Z] Exit code: -11
[task 2020-02-28T12:34:38.712Z] FAIL - basic/bug-1198090.js
[task 2020-02-28T12:34:38.712Z] TEST-UNEXPECTED-FAIL | js/src/jit-test/tests/basic/bug-1198090.js | Assertion failure: hasScript(), at /builds/worker/workspace/build/src/js/src/vm/JSFunction.h:751 (code -11, args "--baseline-eager") [0.5 s]
[task 2020-02-28T12:34:38.712Z] INFO exit-status : -11
[task 2020-02-28T12:34:38.712Z] INFO timed-out : False
[task 2020-02-28T12:34:38.712Z] INFO stderr 2> Assertion failure: hasScript(), at /builds/worker/workspace/build/src/js/src/vm/JSFunction.h:751
[task 2020-02-28T12:34:38.726Z] TEST-PASS | js/src/jit-test/tests/basic/bug-1198090.js | Success (code 3, args "") [0.6 s]

Comment 2

[Jon Coppeard (:jonco)]

Crash during invalidation during compacting GC. Probably due to bug 1618131.

[task 2020-02-28T13:36:08.434Z] Thread 0 (crashed)
[task 2020-02-28T13:36:08.434Z]  0  js!js::jit::ScriptFromCalleeToken [JSFunction.h:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 751 + 0x0]
[task 2020-02-28T13:36:08.434Z]     rax = 0x0000555557d3c1e0   rdx = 0x0000000000000000
[task 2020-02-28T13:36:08.434Z]     rcx = 0x0000555556c7df7d   rbx = 0x00007fffffffb770
[task 2020-02-28T13:36:08.434Z]     rsi = 0x0000000000000000   rdi = 0x00007ffff72fd880
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffb720   rsp = 0x00007fffffffb720
[task 2020-02-28T13:36:08.434Z]      r8 = 0x00007ffff7e02ce0    r9 = 0x0000000000000001
[task 2020-02-28T13:36:08.434Z]     r10 = 0x0000000000000000   r11 = 0x00007ffff6fdcea0
[task 2020-02-28T13:36:08.434Z]     r12 = 0x0000000000000002   r13 = 0x0000555556de02c8
[task 2020-02-28T13:36:08.434Z]     r14 = 0x00007ffff4edb978   r15 = 0x0000555556dbd0d2
[task 2020-02-28T13:36:08.434Z]     rip = 0x00005555563c2139
[task 2020-02-28T13:36:08.434Z]     Found by: given as instruction pointer in context
[task 2020-02-28T13:36:08.434Z]  1  js!InvalidateActivation [Ion.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 2433 + 0x13]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffb810   rsp = 0x00007fffffffb730
[task 2020-02-28T13:36:08.434Z]     rip = 0x000055555636ae9d
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  2  js!js::jit::InvalidateAll [Ion.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 2572 + 0xd]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffb880   rsp = 0x00007fffffffb820
[task 2020-02-28T13:36:08.434Z]     rip = 0x00005555563a5e34
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  3  js!JS::Zone::discardJitCode [Zone.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 412 + 0xb]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffb950   rsp = 0x00007fffffffb890
[task 2020-02-28T13:36:08.434Z]     rip = 0x000055555614acb9
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  4  js!js::AutoClearTypeInferenceStateOnOOM::~AutoClearTypeInferenceStateOnOOM [TypeInference.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 4585 + 0x10]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffba20   rsp = 0x00007fffffffb960
[task 2020-02-28T13:36:08.434Z]     rip = 0x0000555555d3c138
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  5  js!js::gc::GCRuntime::sweepTypesAfterCompacting [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 2125 + 0x8]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffbb20   rsp = 0x00007fffffffba30
[task 2020-02-28T13:36:08.434Z]     rip = 0x0000555556086829
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer[task 2020-02-28T13:36:08.434Z]  6  js!js::gc::GCRuntime::sweepZoneAfterCompacting [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 2146 + 0xb]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffbb90   rsp = 0x00007fffffffbb30
[task 2020-02-28T13:36:08.434Z]     rip = 0x00005555560ce074
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  7  js!js::gc::GCRuntime::updateZonePointersToRelocatedCells [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 2501 + 0xe]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffbc40   rsp = 0x00007fffffffbba0
[task 2020-02-28T13:36:08.434Z]     rip = 0x00005555560ce4f2
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  8  js!js::gc::GCRuntime::compactPhase [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 6171 + 0xb]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffbcf0   rsp = 0x00007fffffffbc50
[task 2020-02-28T13:36:08.434Z]     rip = 0x00005555560ce771
[task 2020-02-28T13:36:08.434Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.434Z]  9  js!js::gc::GCRuntime::incrementalSlice [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 6618 + 0x15]
[task 2020-02-28T13:36:08.434Z]     rbp = 0x00007fffffffbe20   rsp = 0x00007fffffffbd00
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560cf472
[task 2020-02-28T13:36:08.435Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.435Z] 10  js!js::gc::GCRuntime::gcCycle [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 6979 + 0x1c]
[task 2020-02-28T13:36:08.435Z]     rbp = 0x00007fffffffbed0   rsp = 0x00007fffffffbe30
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560cfbec
[task 2020-02-28T13:36:08.435Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.435Z] 11  js!js::gc::GCRuntime::collect [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 7162 + 0x20]
[task 2020-02-28T13:36:08.435Z]     rbp = 0x00007fffffffbfe0   rsp = 0x00007fffffffbee0
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560d01c9
[task 2020-02-28T13:36:08.435Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.435Z] 12  js!js::gc::GCRuntime::runDebugGC [GC.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 7734 + 0x1e]
[task 2020-02-28T13:36:08.435Z]     rbp = 0x00007fffffffc090   rsp = 0x00007fffffffbff0
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560d2821
[task 2020-02-28T13:36:08.435Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.435Z] 13  js!js::gc::GCRuntime::gcIfNeededAtAllocation [Allocator.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 440 + 0x8]
[task 2020-02-28T13:36:08.435Z]     rbp = 0x00007fffffffc0c0   rsp = 0x00007fffffffc0a0
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560d2b2b
[task 2020-02-28T13:36:08.435Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.435Z] 14  js!js::gc::GCRuntime::checkAllocatorState<(js::AllowGC)1> [Allocator.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 402 + 0x5]
[task 2020-02-28T13:36:08.435Z]     rbp = 0x00007fffffffc0f0   rsp = 0x00007fffffffc0d0
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560d2be9
[task 2020-02-28T13:36:08.435Z]     Found by: previous frame's frame pointer
[task 2020-02-28T13:36:08.435Z] 15  js!js::AllocateObject<(js::AllowGC)1> [Allocator.cpp:fc9d28ae4655254a6ef8a57ac6577545b2a42ca7 : 61 + 0x1f]
[task 2020-02-28T13:36:08.435Z]     rbp = 0x00007fffffffc140   rsp = 0x00007fffffffc100
[task 2020-02-28T13:36:08.435Z]     rip = 0x00005555560d2e9c

Comment 3

[Intermittent Failures Robot]

2 failures in 5233 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 2

Platform breakdown:

• linux64: 1
• windows2012-64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1618880&startday=2020-02-24&endday=2020-03-01&tree=all

Comment 4

[Intermittent Failures Robot]

1 failures in 5483 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform breakdown:

• windows2012-64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1618880&startday=2020-03-02&endday=2020-03-08&tree=all

Comment 5

[Ted Campbell [:tcampbell]]

(That see-also is unrelated)

Comment 7

[Ted Campbell [:tcampbell]]

This now has the following signature:
Intermittent Assertion failure: hasBaseScript(), at z:/task_1583939485/src/js/src\vm/JSFunction.h:746

Comment 9

[Ted Campbell [:tcampbell]]

Here is an interesting instance:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=292754196&repo=autoland&lineNumber=11251

The two tests that seem to intermittently fail are basic/bug-1198090.js and ion/scalar-replacement-oom.js. These both have OOMs. Note that some failures of bug-1198090.js happen with --baseline-eager on the simple test so this isn't IonMonkey related.

It seems like discardJitCode is happening at a bad time.

Marking as S-S until analysis is finished because this is a surprising JIT/GC interaction.

Comment 10

[Ted Campbell [:tcampbell]]

By some amount of dumb luck, I've finally captured a form of this in rr.

The bug that in [1], we call MaybeForwarded on the JSScript*, but don't call it on the JSFunction* before we access JSFunction::nonLazyScript(). This results in corrupted reads.

In the test cases, we start relocating arena data in [2] which moves JSFunctions in zone and then we trigger a discardJitCode in which uses the stale pointer in the callstack shown in Comment 2. The proper update to the callee doesn't happen until [3]. While the precise stack is in debug-only code, it looks like other paths can end up with wild-ptr reads and writes in release.

It looks the the MaybeForwarded that was added in Bug 1618131 is incomplete since it we need to forward the JSFunction callee before we access nonLazyScript.

[1] https://searchfox.org/mozilla-central/rev/278046367dab878316f60f0bd7f740cf73f3c447/js/src/jit/Ion.cpp#2433,2473
[2] https://searchfox.org/mozilla-central/rev/278046367dab878316f60f0bd7f740cf73f3c447/js/src/gc/GC.cpp#6177
[3] https://searchfox.org/mozilla-central/rev/278046367dab878316f60f0bd7f740cf73f3c447/js/src/gc/GC.cpp#6189

Comment 11

[Ted Campbell [:tcampbell]]

Attachment: Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

Comment 12

[Ted Campbell [:tcampbell]]

It isn't clear what the security risk of this tricky GC hazard is, but the concern was already exposed in Bug 1618131 which landed two weeks ago. This is also causing failures on autoland and central so I'd like to land it. I also think it should be uplifted to Beta to follow Bug 1618131 so we don't have spurious assertion failures in automation riding the trains.

A question remains if this (and Bug 1618131) should go to ESR.

Comment 14

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/0bd7b6fc23db7c5a9536dce865e742e7d6f7f7e8
https://hg.mozilla.org/mozilla-central/rev/0bd7b6fc23db

Comment 15

[Ted Campbell [:tcampbell]]

Comment on attachment 9133274 [details]
Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

Beta/Release Uplift Approval Request

• User impact if declined: Frequent intermitents in test. Potential gc security hazard. The incomplete fix for this is already in Beta and this patch completes it.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The underlying issue was in production for a while and these fixes are a straightforward GC fix strategy we've used before.
• String changes made/needed:

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9133274 [details]
Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

Fixes frequent intermittent failures in GC. Approved for 75.0b5.

Comment 17

[Ted Campbell [:tcampbell]]

Jon, does it make sense to put both of these in ESR? It seems like they might be pre-existing hazards based on my reading of your patch for Bug 1618131.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/74eab483bc31

Comment 19

[Jon Coppeard (:jonco)]

(In reply to Ted Campbell [:tcampbell] from comment #17)
It's probably fine but I think we should play it safe and uplift this to ESR.

Comment 20

[Jon Coppeard (:jonco)]

Comment on attachment 9133274 [details]
Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fix for possible GC security hazard.
• User impact if declined: Possible crash / security vulnerability.
• Fix Landed on Version: 76
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a relatively straightforward change that checks whether pointers have been forwarded before dereferencing this.
• String or UUID changes made by this patch: None.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9133274 [details]
Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

Needs a rebased patch for ESR.

Comment 22

[Jon Coppeard (:jonco)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #21)
My bad - we need one of the patches from bug 1618131 to land first.

Comment 23

[Jon Coppeard (:jonco)]

Comment on attachment 9133274 [details]
Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fix for possible GC security hazard.
• User impact if declined: Possible crash / security vulnerability.
• Fix Landed on Version: 76
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a relatively straightforward change that checks whether pointers have been forwarded before dereferencing this.
• String or UUID changes made by this patch: None

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9133274 [details]
Bug 1618880 - Fix GC hazard in jit::InvalidateActivation. r?jonco

Fixes a GC hazard. Approved for 68.7esr.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr68/rev/2fac01c54524