1483183 - Assertion failure: !JS_IsExceptionPending(cx), at js/src/jsexn.h:130 with stackTest

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision bf79440c1376 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --baseline-eager):

stackTest(new Function(`
  newGlobal({
    sameZoneAs: []
  }).frame;
`));


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x000000000079a4d0 in js::AutoAssertNoPendingException::~AutoAssertNoPendingException (this=<optimized out>, __in_chrg=<optimized out>) at js/src/jsexn.h:130
#0  0x000000000079a4d0 in js::AutoAssertNoPendingException::~AutoAssertNoPendingException (this=<optimized out>, __in_chrg=<optimized out>) at js/src/jsexn.h:130
#1  0x0000000000798c7b in js::jit::GetPropIRGenerator::tryAttachStub (this=this@entry=0x7fffffffb690) at js/src/jit/CacheIR.cpp:166
#2  0x0000000000705e15 in js::jit::DoGetPropFallback (cx=<optimized out>, frame=0x7fffffffb908, stub_=<optimized out>, val=..., res=...) at js/src/jit/BaselineIC.cpp:1522
#3  0x00000c4f40e341a8 in ?? ()
[...]
#24 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7fffffffb510	140737488336144
rcx	0x7ffff6c1c2dd	140737333281501
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7fffffffb4d0	140737488336080
rsp	0x7fffffffb4d0	140737488336080
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6780	140737354033024
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7fffffffb540	140737488336192
r13	0xf4d99201	4107899393
r14	0xf4d99201	4107899393
r15	0x7fffffffb690	140737488336528
rip	0x79a4d0 <js::AutoAssertNoPendingException::~AutoAssertNoPendingException()+48>
=> 0x79a4d0 <js::AutoAssertNoPendingException::~AutoAssertNoPendingException()+48>:	movl   $0x0,0x0
   0x79a4db <js::AutoAssertNoPendingException::~AutoAssertNoPendingException()+59>:	ud2

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/6ffbdf27a930
user:        Matthew Gaudet
date:        Mon Apr 23 17:25:38 2018 -0400
summary:     Bug 1438556: [Part 1] Use shape guards on the prototype chain for cross-compartment ICs r=tcampbell

This iteration took 294.694 seconds to run.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Check stack before attempting to create CCW in GetPropIRGenerator

To be perfectly frank, I have no idea if this is the right answer.  It passes
the test case, but all I have really done is hoisted the stack check that
exists in Compartment::wrap into tryAttachCrossCompartment wrapper, and
replaced with the DontReport variant.

(Alternative answers could include changing the check in Compartment::wrap to
be a non-reporting version)

Comment 3

[Ted Campbell [:tcampbell]]

Comment on attachment 9001418 [details] [diff] [review]
Check stack before attempting to create CCW in GetPropIRGenerator

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

So, the issue here is actually that classic SpiderMonkey problem where bool return value can indicate either predicate-false or operation-failed-and-pending-exception set. The problem here is that tryAttach functions return false to indicate a failure to attach, while the wrap() call returns false to indicate an exception. Here is example of what to do:

https://searchfox.org/mozilla-central/rev/5dbfd833bbb114afe758db4d4bdbc5b13bcc33ef/js/src/jit/CacheIR.cpp#214-217

(We really need to boil the ocean and use type system to mark return values that indicate pending exceptions)

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Discard pending exception during CCW GetProp IC failure

Thanks for the pointer! That's very helpful.

Comment 5

[Ted Campbell [:tcampbell]]

Comment on attachment 9001449 [details] [diff] [review]
Discard pending exception during CCW GetProp IC failure

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

Include testcase too.

Comment 6

[Ted Campbell [:tcampbell]]

Can you also make a patch to add AutoAssertNoPendingException to any CacheIR generators that are missing it? (Probably move the CallIC ones to the top tryAttachStub too)

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Complete covereage with AutoAssertNoPendingException in CacheIR stub attach code

Should we maybe just add a AutoAssertNoPendingException member to the IRGenerator base class?

Comment 8

[Ted Campbell [:tcampbell]]

Comment on attachment 9002330 [details] [diff] [review]
Complete covereage with AutoAssertNoPendingException in CacheIR stub attach code

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

Thanks for doing this. I think the tryAttach methods are probably the right place to do it since we use these to communicate and test that tryAttach (which are main interface boundary from CacheIR.cpp to actual instantiations) mean |return false| as failed-to-attach and not operation-failed-with-exception.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4e417850332a
Discard pending exception during CCW GetProp IC failure r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/9370dc79ddfc
Complete coverage with AutoAssertNoPendingException in CacheIR stub attach code r=tcampbell

Comment 10

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on bug1483183.js

Push link: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=9370dc79ddfc94fdb20f3ae6036c9fcbd3c3d6c1
Backout link: https://hg.mozilla.org/integration/mozilla-inbound/rev/56856b94af05f97c1ef66b0bbb71ce8be890a156
Log link: https://treeherder.mozilla.org/logviewer.html#?job_id=194875448&repo=mozilla-inbound&lineNumber=33755

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/79082cb95664
Discard pending exception during CCW GetProp IC failure r=tcampbell
https://hg.mozilla.org/integration/mozilla-inbound/rev/a6c928f38da7
Complete coverage with AutoAssertNoPendingException in CacheIR stub attach code r=tcampbell

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/79082cb95664
https://hg.mozilla.org/mozilla-central/rev/a6c928f38da7

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Is there a user impact which justifies backport consideration here or can this ride the trains?

Comment 14

[Matthew Gaudet (he/him) [:mgaudet]]

Probably isn't worth uplifiting.