817714 - IonMonkey: LookupSwitch does not check the result of the allocation of bodies (the FixedList)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

This bug might cause a crash in case of OOM of temp objects during the processing of the lookup switch. (which is more frequent than OOMs)

Comment 1

[Lukas Blakk [:lsblakk] use ?needinfo]

Nicolas - can you give a bit more information about this bug and anticipated user impact? How wide of an issue is this?

Comment 2

[Nicolas B. Pierron [:nbp]]

This is a OOM on temp objects, I never heard anything related to doing OOM there until yesterday, so I guess this won't be a noticeable user impact, but at the same time this is a one line patch with no implication.

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: Check allocation result in LookupSwitch.

Comment 4

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e2d7adbe497

Comment 5

[Alex Keybl [:akeybl]]

(In reply to Nicolas B. Pierron [:pierron] [:nbp] from comment #2)
> This is a OOM on temp objects, I never heard anything related to doing OOM
> there until yesterday, so I guess this won't be a noticeable user impact,
> but at the same time this is a one line patch with no implication.

Given the lack of known user impact, no need to track. We'd consider an uplift, however.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3e2d7adbe497

Comment 7

[Nicolas B. Pierron [:nbp]]

Comment on attachment 688039 [details] [diff] [review]
Check allocation result in LookupSwitch.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 701963

User impact if declined: Unlikely crashes.

Testing completed (on m-c, etc.): on m-c since 2012-12-04 19:04:10 PST

Risk to taking this patch (and alternatives if risky): Might increase the binary size by a 4 / 5 / 8 bytes, respectively on x86 / x64 / ARM.  Hopefully IonMonkey can still be disabled for B2G builds :)

String or UUID changes made by this patch: N/A

Comment 8

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 688039 [details] [diff] [review]
Check allocation result in LookupSwitch.

This sounds too risky for branches, let's ride the trains instead.