1438502 - Missing exception when CacheEntry_getBytecode returns false

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P3)

Description

[André Bargull [:anba] [back from break; working through mail backlog]]

Test case 1:
---
var code = cacheEntry("");
oomTest(function() {
    offThreadDecodeScript(code);
});
---

Test case 2:
---
var code = cacheEntry("");
oomTest(function() {
    evaluate(code, {loadBytecode: true});
});
---

Both test cases assert with
---
Assertion failure: cx->isExceptionPending() (Thunk execution failed but no exception was raised - missing call to js::ReportOutOfMemory()?)
---

because no error is thrown when CacheEntry_getBytecode(...) returns nullptr.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Add error reports to CacheEntry internal methods.

Comment 2

[André Bargull [:anba] [back from break; working through mail backlog]]

Comment on attachment 8951248 [details] [diff] [review]
Add error reports to CacheEntry internal methods.

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

Thanks!

::: js/src/shell/js.cpp
@@ +1590,5 @@
>      ArrayBufferObject::BufferContents contents =
>          ArrayBufferObject::BufferContents::create<ArrayBufferObject::PLAIN>(buffer);
>      Rooted<ArrayBufferObject*> arrayBuffer(cx, ArrayBufferObject::create(cx, length, contents));
> +    if (!arrayBuffer) {
> +        JS_ReportErrorASCII(cx, "CacheEntry_setBytecode: Unable to create an array buffer to hold the bytecode");

ArrayBufferObject::create(...) will already have reported an error if |!arrayBuffer| is true, so this change is not needed.

@@ +1663,5 @@
>      if (args[0].isString()) {
>          code = args[0].toString();
>      } else if (args[0].isObject() && CacheEntry_isCacheEntry(&args[0].toObject())) {
>          cacheEntry = &args[0].toObject();
> +        code = CacheEntry_getSource(cx, cacheEntry);

Needs |if (!code) return false;| otherwise the pending error will be overwritten a few lines below.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cfd98003fc0f
Add error reports to CacheEntry internal methods. r=anba

Comment 4

[Andreea Pavel [:apavel]]

Backed out for spidermonkey build bustages at /builds/worker/workspace/sm-package/mozjs-60.0a1.0/js/src/jit-test/tests/self-test/cacheEntry.js:9

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=cfd98003fc0f68673f6053d2800e3d0fa99868a8

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=162441697&repo=mozilla-inbound&lineNumber=136944

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/5626666b65bb27fe974d42b0997c47ec4e1a1daa

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e48b1c8130b9
Add error reports to CacheEntry internal methods. r=anba

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/e48b1c8130b9