Closed Bug 1299121 Opened 5 years ago Closed 5 years ago

Crash [@ js::Proxy::set] or Hit MOZ_CRASH(bad AccessResult) at js/src/vm/EnvironmentObject.cpp:2040

Categories

(Core :: JavaScript Engine, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: gkw, Assigned: shu)

References

Details

(Keywords: bugmon, crash, testcase, Whiteboard: [jsbugmon:update])

Crash Data

Attachments

(2 files, 1 obsolete file)

The following testcase crashes on mozilla-central revision 4f72b1d05267 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

// Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1130768.js
var g = newGlobal();
g.parent = this;
g.eval("(" + function() {
    var dbg = new Debugger(parent);
    dbg.onExceptionUnwind = function(frame) {
        frame.eval("h = 3");
    };
} + ")()");
// jsfunfuzz-generated
g = function h() {
    a
}
g();


Backtrace:

0   js-dbg-64-dm-clang-darwin-4f72b1d05267	0x000000010b649753 (anonymous namespace)::DebugEnvironmentProxyHandler::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) const + 995 (EnvironmentObject.cpp:2040)
1   js-dbg-64-dm-clang-darwin-4f72b1d05267	0x000000010b576a52 js::Proxy::set(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) + 466 (RootingAPI.h:706)
2   js-dbg-64-dm-clang-darwin-4f72b1d05267	0x000000010b4f50bd JSObject::nonNativeSetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::ObjectOpResult&) + 205 (RootingAPI.h:706)
3   js-dbg-64-dm-clang-darwin-4f72b1d05267	0x000000010b6ff1f3 js::SetNameOperation(JSContext*, JSScript*, unsigned char*, JS::Handle<JSObject*>, JS::Handle<JS::Value>) + 627 (NativeObject.h:1495)
4   js-dbg-64-dm-clang-darwin-4f72b1d05267	0x000000010b6bc5df Interpret(JSContext*, js::RunState&) + 31263 (Interpreter.cpp:2691)
/snip

For detailed crash information, see attachment.
Due to skipped revisions, the first bad revision could be any of:

changeset:   https://hg.mozilla.org/mozilla-central/rev/cb6fc6d38f8d
user:        Shu-yu Guo
date:        Thu Aug 25 01:28:47 2016 -0700
summary:     Bug 1263355 - Rewrite the frontend: bindings. (r=jorendorff,Waldo)

changeset:   https://hg.mozilla.org/mozilla-central/rev/18bec78f348e
user:        Shu-yu Guo
date:        Thu Aug 25 01:28:47 2016 -0700
summary:     Bug 1263355 - Report memory metrics for Scopes. (r=njn)

Shu-yu, is bug 1263355 a likely regressor?
Blocks: 1263355
Flags: needinfo?(shu)
Crash Signature: [@ js::Proxy::set]
Summary: Hit MOZ_CRASH(bad AccessResult) at js/src/vm/EnvironmentObject.cpp:2040 → Crash [@ js::Proxy::set] or Hit MOZ_CRASH(bad AccessResult) at js/src/vm/EnvironmentObject.cpp:2040
Comment on attachment 8786556 [details] [diff] [review]
Only report ACCESS_LOST for named lambda callees when getting.

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

You need to explain, in a comment, why the GET check is necessary. Otherwise, other people working on this code will be mystified.
Attachment #8786556 - Flags: review?(jimb)
Assignee: nobody → shu
(In reply to Jim Blandy :jimb from comment #4)
> Comment on attachment 8786556 [details] [diff] [review]
> Only report ACCESS_LOST for named lambda callees when getting.
> 
> Review of attachment 8786556 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You need to explain, in a comment, why the GET check is necessary.
> Otherwise, other people working on this code will be mystified.

Where would you like this comment? It's an preexisting invariant that ACCESS_LOST is only reported when action == GET; see the other sites that do *accessResult = ACCESS_LOST. I suppose it could go above the enum?
Flags: needinfo?(shu)
(In reply to Shu-yu Guo [:shu] from comment #5)
> (In reply to Jim Blandy :jimb from comment #4)
> > Comment on attachment 8786556 [details] [diff] [review]
> > Only report ACCESS_LOST for named lambda callees when getting.
> > 
> > Review of attachment 8786556 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > You need to explain, in a comment, why the GET check is necessary.
> > Otherwise, other people working on this code will be mystified.
> 
> Where would you like this comment? It's an preexisting invariant that
> ACCESS_LOST is only reported when action == GET; see the other sites that do
> *accessResult = ACCESS_LOST. I suppose it could go above the enum?

Yes, a comment on ACCESS_LOST, saying that it is only returned for GET, would be perfect.
Better comments.
Attachment #8787432 - Flags: review?(jimb)
Attachment #8786556 - Attachment is obsolete: true
Comment on attachment 8787432 [details] [diff] [review]
Only report ACCESS_LOST for named lambda callees when getting.

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

Thanks very much for updating the comments; this is great.
Attachment #8787432 - Flags: review?(jimb) → review+
Pushed by shu@rfrn.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/339602d23c40
Only report ACCESS_LOST for named lambda callees when getting. (r=jimb)
https://hg.mozilla.org/mozilla-central/rev/339602d23c40
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.