Last Comment Bug 948233 - Improper OOM check in DoCompareFallback
: Improper OOM check in DoCompareFallback
Status: RESOLVED FIXED
[qa-]
: crash
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86_64 Linux
-- critical (vote)
: mozilla29
Assigned To: Christian Holler (:decoder)
: general
: Jason Orendorff [:jorendorff]
Mentors:
: 914598 (view as bug list)
Depends on:
Blocks: langfuzz 912928
  Show dependency treegraph
 
Reported: 2013-12-09 17:51 PST by Christian Holler (:decoder)
Modified: 2013-12-17 14:52 PST (History)
6 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
js-setNext-oom.patch (1.12 KB, patch)
2013-12-09 17:51 PST, Christian Holler (:decoder)
jdemooij: review+
lukasblakk+bugs: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description User image Christian Holler (:decoder) 2013-12-09 17:51:43 PST
Created attachment 8345050 [details] [diff] [review]
js-setNext-oom.patch

In js::jit::DoCompareFallback we have the following code:

>    ICStub *doubleStub = compiler.getStub(compiler.getStubSpace(script));
>    if (!stub)
>        return false;

I think this is either a typo or some tasty copy-pasta. I changed stub to doubleStub and it fixed another OOM crasher for me. Jandem, can you review the attached patch since it's your code?
Comment 1 User image Christian Holler (:decoder) 2013-12-10 03:31:33 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/908680cb2773
Comment 2 User image Wes Kocher (:KWierso) 2013-12-10 16:49:44 PST
https://hg.mozilla.org/mozilla-central/rev/908680cb2773
Comment 3 User image Christian Holler (:decoder) 2013-12-11 08:18:16 PST
*** Bug 914598 has been marked as a duplicate of this bug. ***
Comment 4 User image Chris Peterson [:cpeterson] 2013-12-12 14:09:01 PST
Christian: should this OOM fix be uplifted to Aurora 28 and Beta 27?
Comment 5 User image Christian Holler (:decoder) 2013-12-12 16:18:49 PST
This one can safely be uplifted to Aurora at least.
Comment 6 User image Christian Holler (:decoder) 2013-12-12 16:21:33 PST
Comment on attachment 8345050 [details] [diff] [review]
js-setNext-oom.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: Crashes with OOM conditions
Testing completed (on m-c, etc.): A few days on mozilla-central
Risk to taking this patch (and alternatives if risky): Not risky, patch is just fixing a null check (fixing a typo).
String or IDL/UUID changes made by this patch: None
Comment 7 User image Christian Holler (:decoder) 2013-12-15 05:01:18 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/8f29c6506b23
Comment 8 User image Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-12-17 14:52:46 PST
I don't think this needs QA verification. If anyone thinks that's a mistake please remove the [qa-] whiteboard tag and add the verifyme keyword.

Note You need to log in before you can comment on or make changes to this bug.