Closed Bug 1234428 Opened 6 years ago Closed 6 years ago

Crash [@ js::CompartmentChecker::fail] with findPath

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: decoder, Unassigned)

References

Details

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

Crash Data

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 388bdc46ba51 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug, run with --fuzzing-safe --no-threads --disable-oom-functions --ion-eager --ion-extra-checks):

g = newGlobal();
g.parent = this;
g.eval(`
 dbg = Debugger(parent);
 dbg.onIonCompilation = function () {}
`);
function foo()
eval(`
  var o = {};
  function C() {};
  new C;
  Match(findPath(o, o));
`)
foo();
var Match = Pattern.prototype = {};



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00000000005c7ff0 in js::CompartmentChecker::fail (c1=<optimized out>, c2=<optimized out>) at js/src/jscntxtinlines.h:49
#0  0x00000000005c7ff0 in js::CompartmentChecker::fail (c1=<optimized out>, c2=<optimized out>) at js/src/jscntxtinlines.h:49
#1  0x00000000005c8133 in check (c=<optimized out>, this=0x7fffffffa280) at js/src/jscntxtinlines.h:70
#2  check (obj=<optimized out>, this=0x7fffffffa280) at js/src/jscntxtinlines.h:81
#3  js::CompartmentChecker::check (this=0x7fffffffa280, v=...) at js/src/jscntxtinlines.h:101
#4  0x00000000008be363 in check<JS::Value> (handle=..., this=0x7fffffffa280) at js/src/jscntxtinlines.h:91
#5  assertSameCompartment<JS::Handle<JSObject*>, JS::Handle<jsid>, JS::Handle<JS::Value>, JSObject*, JSObject*> (t5=<optimized out>, t4=<optimized out>, t3=<synthetic pointer>, t2=<synthetic pointer>, t1=<synthetic pointer>, cx=0x7ffff6907400) at js/src/jscntxtinlines.h:217
#6  DefinePropertyById (cx=cx@entry=0x7ffff6907400, obj=..., obj@entry=..., id=..., id@entry=..., value=..., value@entry=..., get=..., set=..., attrs=attrs@entry=1, flags=0) at js/src/jsapi.cpp:2128
#7  0x00000000008bf192 in DefineProperty (cx=cx@entry=0x7ffff6907400, obj=..., name=name@entry=0xe6fc84 "node", value=..., getter=..., setter=..., attrs=attrs@entry=1, flags=0) at js/src/jsapi.cpp:2230
#8  0x00000000008bf245 in JS_DefineProperty (cx=cx@entry=0x7ffff6907400, obj=..., obj@entry=..., name=name@entry=0xe6fc84 "node", value=..., attrs=attrs@entry=1, getter=getter@entry=0x0, setter=setter@entry=0x0) at js/src/jsapi.cpp:2239
#9  0x0000000000a38303 in FindPath (cx=0x7ffff6907400, argc=<optimized out>, vp=0x7fffffffa8c8) at js/src/builtin/TestingFunctions.cpp:2458
#10 0x0000000000a7e002 in js::CallJSNative (cx=0x7ffff6907400, native=0xa37cf0 <FindPath(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:235
[...]
#40 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7fffffffa280	140737488331392
rcx	0x7ffff6ca53b0	140737333842864
rdx	0x0	0
rsi	0x7ffff6f7a9d0	140737336814032
rdi	0x7ffff6f791c0	140737336807872
rbp	0x7fffffffa1c0	140737488331200
rsp	0x7fffffffa1c0	140737488331200
r8	0x7ffff7fe0780	140737354008448
r9	0x6372732f736a2f6c	7165916604736876396
r10	0x7fffffff9f80	140737488330624
r11	0x7ffff6c27960	140737333328224
r12	0x1	1
r13	0x7fffffffa280	140737488331392
r14	0x7ffff6907400	140737330050048
r15	0x0	0
rip	0x5c7ff0 <js::CompartmentChecker::fail(JSCompartment*, JSCompartment*)+48>
=> 0x5c7ff0 <js::CompartmentChecker::fail(JSCompartment*, JSCompartment*)+48>:	movl   $0x31,0x0
   0x5c7ffb <js::CompartmentChecker::fail(JSCompartment*, JSCompartment*)+59>:	callq  0x4a3f80 <abort()>


Very likely a bug in the shell-only function findPath.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/19e2d95665e8
user:        Nicolas B. Pierron
date:        Thu May 28 19:26:55 2015 +0200
summary:     Bug 1147403 part 3 - Make IonSpewer work during off-thread compilation. r=h4writer

changeset:   https://hg.mozilla.org/mozilla-central/rev/898150157374
user:        Nicolas B. Pierron
date:        Thu May 28 19:26:55 2015 +0200
summary:     Bug 1147403 part 4 - Extract the printer from the serializer. r=h4writer

changeset:   https://hg.mozilla.org/mozilla-central/rev/960bbe18c960
user:        Nicolas B. Pierron
date:        Thu May 28 19:26:56 2015 +0200
summary:     Bug 1147403 part 5 - Add Debugger::onIonCompilation hook. r=shu

changeset:   https://hg.mozilla.org/mozilla-central/rev/396f78269a37
user:        Nicolas B. Pierron
date:        Thu May 28 19:26:56 2015 +0200
summary:     Bug 1147403 part 6 - Remove GetJitContext from serializing functions. r=h4writer

changeset:   https://hg.mozilla.org/mozilla-central/rev/bb3ab5214edd
user:        Nicolas B. Pierron
date:        Thu May 28 19:26:56 2015 +0200
summary:     Bug 1147403 part 7 - Fix inIon, only reset the counter when the function is executed. r=jandem

changeset:   https://hg.mozilla.org/mozilla-central/rev/8b4c06ddd51d
user:        Jonathan Kew
date:        Thu May 28 18:29:07 2015 +0100
summary:     Bug 1167930 - Handle direction:rtl in vertical modes when converting a LogicalMargin to physical. r=smontagu

changeset:   https://hg.mozilla.org/mozilla-central/rev/021356105e99
user:        Nicolas B. Pierron
date:        Thu May 28 19:46:53 2015 +0200
summary:     Bug 1147403 part 3.1 - Replace newly added IonSpewPass after KeepAlive transform. r=KWierso

This iteration took 121.000 seconds to run.
Nicolas, is bug 1147403 a likely regressor?
Blocks: 1147403
Flags: needinfo?(nicolas.b.pierron)
This patch fix findPath by assuming that the value can be from a different
compartment.

This patch also add a new way for running test cases such that we do not run
a test 6 times, while we are only interested in a single configuration. This
way we won't run --no-ion with --ion-eager, when specified.
Attachment #8706516 - Flags: review?(jcoppeard)
Flags: needinfo?(nicolas.b.pierron)
(In reply to Nicolas B. Pierron [:nbp] from comment #3)
> This patch also add a new way for running test cases such that we do not run
> a test 6 times, while we are only interested in a single configuration. This
> way we won't run --no-ion with --ion-eager, when specified.

Don't we want to make sure the test behaves in the interpreter, too?

Often when I work on a big patch, I get test failures that are completely unrelated to the original bug for which we added the test. That's extremely useful.

Or imagine we add a --jitv2 or --concurrentgc configuration to jit_test.py --tbpl, to make sure this configuration doesn't break while we're still working on it. At that point we want to run all tests with those flags, IMO. (I did this with --non-writable-jitcode for instance, to make sure it didn't regress before it was enabled by default.)
Comment on attachment 8706516 [details] [diff] [review]
findPath wrap cross-compartment objects.

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

I agree with Jan that we should run testcases on all configurations, so r=me without the jittest changes.

::: js/src/jit-test/tests/self-test/findPath-bug1234428.js
@@ +5,5 @@
> +// 2. Registering the onIonCompilation hook on the Debugger causes
> +//    the JSScript of the function C to be wrapped in the Debugger compartment.
> +// 3. The JSScript hold a pointer to its function C.
> +// 4. The function C, hold its environment.
> +// 5. The environment holds the Object o.

Thanks for adding an explanation of what's going on!
Attachment #8706516 - Flags: review?(jcoppeard) → review+
(In reply to Jan de Mooij [:jandem] from comment #4)
> (In reply to Nicolas B. Pierron [:nbp] from comment #3)
> > This patch also add a new way for running test cases such that we do not run
> > a test 6 times, while we are only interested in a single configuration. This
> > way we won't run --no-ion with --ion-eager, when specified.
> 
> Don't we want to make sure the test behaves in the interpreter, too?
> 
> Often when I work on a big patch, I get test failures that are completely
> unrelated to the original bug for which we added the test. That's extremely
> useful.

The problem I had was that findPath does not return the same result if onIonCompilation is not called, which implies running with --ion-eager enabled.  Thus we could run in the interpreter, but it would behave differently.

Also, Gary is now fuzzing by concatenating test cases.  Thus we would probably get a fuzzblocker from gary as soon as the new configuration is tested.

In the meantime, I will split the patch and land only the fix without the no-variant jit-test addition.
https://hg.mozilla.org/mozilla-central/rev/560d53d75ad6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.