IonMonkey: OOM Testing: Crash [@ js::ion::CodeGeneratorX86Shared::visitOutOfLineBailout]

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: decoder, Assigned: dvander)

Tracking

(Blocks: 1 bug, {crash, testcase})

Other Branch
x86_64
Linux
crash, testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
The following command crashes on ionmonkey revision 8c54899dae82 (dbg build):

js  -e 'const libdir = "js/src/jit-test/lib/";' -A 9445 -f js/src/jit-test/tests/debug/Object-parameterNames.js
Created attachment 625303 [details] [diff] [review]
fix

IonMonkey relies on infallible allocations for small objects, and we ensure this with a ballast that has remained unimplemented. This patch implements it with some LifoAlloc trickery.
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #625303 - Flags: review?(luke)

Comment 2

5 years ago
Comment on attachment 625303 [details] [diff] [review]
fix

Review of attachment 625303 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/ds/LifoAlloc.h
@@ +241,5 @@
>  
> +    // Ensures that enough space exists to satisfy N bytes worth of
> +    // allocation requests, not necessarily contiguous.
> +    JS_ALWAYS_INLINE
> +    bool ensureUnused(size_t n) {

iiuc, this isn't strictly true for the fast-path algorithm below since, if a chunk reports k bytes of unused space and an allocation is made for >k bytes, that chunk's unused space can't be used.  However, this doesn't really matter for the purpose of the ballast since n only has to be approximate.  So my only request is to rename this to ensureUnusedApproximate and update the comment accordingly.
Attachment #625303 - Flags: review?(luke) → review+
Created attachment 625314 [details] [diff] [review]
follow-up

Quick follow-up patch to make it so OOM testing can't inject OOMs into our infallible allocations.
Attachment #625314 - Flags: review?(luke)

Updated

5 years ago
Attachment #625314 - Flags: review?(luke) → review+
Whoa, fast reviews! http://hg.mozilla.org/projects/ionmonkey/rev/88ea2e529609

Christian, would you mind having the OOM tester re-test all the bugs it filed today? I ran through them and could not reproduce them after implementing the ballast.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756605
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756609
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756611
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756613
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756614
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756617
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756621
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756622
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756623
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756624
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756626
(Reporter)

Updated

5 years ago
Duplicate of this bug: 756627
You need to log in before you can comment on or make changes to this bug.