Last Comment Bug 758428 - shell swallows compiler bugs
: shell swallows compiler bugs
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: :Benjamin Peterson
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-24 15:59 PDT by :Benjamin Peterson
Modified: 2012-06-01 08:12 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
complains when the compiler returns NULL and doesn't report an error (3.05 KB, patch)
2012-05-24 16:25 PDT, :Benjamin Peterson
jorendorff: review+
Details | Diff | Splinter Review
address review comments (3.18 KB, patch)
2012-05-30 13:23 PDT, :Benjamin Peterson
no flags Details | Diff | Splinter Review
use JS_ASSERT_IF (3.16 KB, patch)
2012-05-30 13:24 PDT, :Benjamin Peterson
jorendorff: review+
Details | Diff | Splinter Review

Description :Benjamin Peterson 2012-05-24 15:59:38 PDT
If frontend::CompileScript returns NULL because of OOM, Process() in js.cpp will continue merrily on its way and no one ever calls JS_ReportOutOfMemory().

Is it the responsibility of the JS API to call JS_ReportOutOfMemory() or the shell?
Comment 1 :Benjamin Peterson 2012-05-24 16:15:16 PDT
This is not as clear cut as a thought. Parts of the compiler that allocate things do call js_ReportOutOfMemory(). However, if you accidentally return NULL from the compiler with no error set, nothing gets spit out. It would be nice to have something fail when that happens. I'll try to devise a patch.
Comment 2 :Benjamin Peterson 2012-05-24 16:25:20 PDT
Created attachment 627012 [details] [diff] [review]
complains when the compiler returns NULL and doesn't report an error
Comment 3 Jason Orendorff [:jorendorff] 2012-05-30 12:33:51 PDT
Comment on attachment 627012 [details] [diff] [review]
complains when the compiler returns NULL and doesn't report an error

>+JSBool gGotError = JS_FALSE;

In new code, use C++ bool, false, and true.

(unless you have to match a function signature, or matching is the nicest way to avoid stupid C++ warnings, but that's not an issue here)


>+        if (!script)
>+            JS_ASSERT(gGotError);

        JS_ASSERT_IF(!script, gGotError);
Comment 4 :Benjamin Peterson 2012-05-30 13:23:46 PDT
Created attachment 628437 [details] [diff] [review]
address review comments
Comment 5 :Benjamin Peterson 2012-05-30 13:24:59 PDT
Created attachment 628439 [details] [diff] [review]
use JS_ASSERT_IF
Comment 6 Jason Orendorff [:jorendorff] 2012-05-31 14:46:24 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c7a1e5ac5bd
Comment 7 Ed Morley [:emorley] 2012-06-01 08:12:13 PDT
https://hg.mozilla.org/mozilla-central/rev/2c7a1e5ac5bd

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