The default bug view has changed. See this FAQ.

shell swallows compiler bugs

RESOLVED FIXED in mozilla15

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Benjamin, Assigned: Benjamin)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

5 years ago
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?
(Assignee)

Comment 1

5 years ago
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.
Summary: shell swallows OOM errors from the compiler → shell swallows compiler bugs
(Assignee)

Comment 2

5 years ago
Created attachment 627012 [details] [diff] [review]
complains when the compiler returns NULL and doesn't report an error
Attachment #627012 - Flags: review?(jorendorff)
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);
Attachment #627012 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 4

5 years ago
Created attachment 628437 [details] [diff] [review]
address review comments
Assignee: general → bpeterson
Attachment #627012 - Attachment is obsolete: true
(Assignee)

Comment 5

5 years ago
Created attachment 628439 [details] [diff] [review]
use JS_ASSERT_IF
Attachment #628437 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
Attachment #628439 - Flags: review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/2c7a1e5ac5bd
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/2c7a1e5ac5bd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.