1426385 - Crash [@ js::CompartmentChecker::fail] with evaluate

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Christian Holler (:decoder)]

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

var someObject = {};
var evalOpt = {
    envChainObject: someObject
};
test = (function() {})();
evalWithCache(test, __proto__);
function evalWithCache(code, ctx) {
    ctx.global = newGlobal({});
}
evaluate("assertEq(someVar, 1);", evalOpt);


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x000000000056fb58 in js::CompartmentChecker::fail (c2=<optimized out>, c1=<optimized out>) at js/src/jscntxtinlines.h:40
#0  0x000000000056fb58 in js::CompartmentChecker::fail (c2=<optimized out>, c1=<optimized out>) at js/src/jscntxtinlines.h:40
#1  js::CompartmentChecker::check (c=<optimized out>, this=0x7fffffffc390) at js/src/jscntxtinlines.h:59
#2  js::CompartmentChecker::check (this=this@entry=0x7fffffffc390, obj=0x7ffff448e120) at js/src/jscntxtinlines.h:72
#3  0x0000000000b5fdc8 in js::CompartmentChecker::check<JSObject*> (handle=..., this=0x7fffffffc390) at js/src/jscntxtinlines.h:88
#4  js::assertSameCompartment<JS::MutableHandle<JSObject*> > (t1=..., cx=0x7ffff5f16000) at js/src/jscntxtinlines.h:217
#5  js::CreateObjectsForEnvironmentChain (cx=0x7ffff5f16000, chain=..., terminatingEnv=..., envObj=...) at js/src/vm/EnvironmentObject.cpp:3177
#6  0x00000000009a83c5 in CreateNonSyntacticEnvironmentChain (cx=0x7ffff5f16000, envChain=..., env=..., scope=scope@entry=...) at js/src/jsapi.cpp:3621
#7  0x00000000009b1c1b in ExecuteScript (cx=0x7ffff5f16000, envChain=..., scriptArg=..., rval=0x7ffff404f090) at js/src/jsapi.cpp:4667
#8  0x00000000009b1d7a in JS_ExecuteScript (cx=<optimized out>, envChain=..., scriptArg=..., scriptArg@entry=..., rval=...) at js/src/jsapi.cpp:4699
#9  0x000000000046ace9 in Evaluate (cx=0x7ffff5f16000, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:1820
#10 0x0000000000575ed1 in js::CallJSNative (cx=0x7ffff5f16000, native=0x46a230 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291
[...]
#24 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:9141
rax	0x0	0
rbx	0x7fffffffc390	140737488339856
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffc330	140737488339760
rsp	0x7fffffffc310	140737488339728
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x0	0
r13	0x200ce20	33607200
r14	0x0	0
r15	0x7ffff5f16000	140737319624704
rip	0x56fb58 <js::CompartmentChecker::check(JSObject*)+216>
=> 0x56fb58 <js::CompartmentChecker::check(JSObject*)+216>:	movl   $0x0,0x0
   0x56fb63 <js::CompartmentChecker::check(JSObject*)+227>:	ud2

Comment 1

[Jason Orendorff [:jorendorff]]

Ugh. Ted, would you take a look please?

Comment 2

[Ted Campbell [:tcampbell]]

Will take a look this afternoon. Most likely we need to detect the problem earlier and throw an exception to user when they call evaluate. Hopefully the story is better once I finish Bug 1406153, but I should be able to land a small fix in the meantime.

Comment 3

[Ted Campbell [:tcampbell]]

Reduced testcase isn't scary at all. jsshell-only. Already fixed in my local patches for Bug 1406153. I may defer this unless it becomes a fuzz blocker since the evaluate code is ugly and I'm rewriting it anyways.

> var evalOpt = {
>     global: newGlobal(),
>     envChainObject: {}
> };
> evaluate("", evalOpt);

Will assign to myself to follow up on.

Comment 4

[Ted Campbell [:tcampbell]]

Dropping priority to since jsshell-only.

Comment 5

[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/40aafceb5e48
user:        Andrew McCreight
date:        Mon Aug 07 16:35:43 2017 -0700
summary:     Bug 1388191 - Add way to test evaluation with envChain in the shell. r=jorendorff

This iteration took 2.031 seconds to run.

Comment 6

[Andrew Overholt [:overholt]]

Given comment 4 I'd like to remove the regression keyword so this isn't tracked in the regression triage meeting.

I'll ni?(mccr8) based on comment 5.

Comment 7

[Andrew McCreight [:mccr8]]

My patch just added a function that the test case uses. It seems reasonable to not mark it as a regression.

Comment 9

[Fuzzing Team]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 3dc7d345da52).

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

var evalOpt = {
    global: newGlobal(),
    envChainObject: {}
};
evaluate("", evalOpt);

crashes js shell compiled with --enable-debug on m-c rev 8ec327de0ba7 using --fuzzing-safe --no-threads --no-baseline --no-ion --more-compartments at Hit MOZ_CRASH(*** Compartment mismatch 0x7f7c2939b9d0 vs. 0x7f7c2939b310 at argument 0) at js/src/vm/JSContext-inl.h:46

Comment 11

[Fuzzing Team]

JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.

Comment 12

[Neha Kochar [:neha]]

Bulk change for all regression bugs with status-firefox67 as 'fix-optional' to be marked 'affected' for status-firefox68.

Comment 15

[Ted Campbell [:tcampbell]]

Attachment: Bug 1426385 - Properly wrap envChainObject to jsshell evaluate. r?mgaudet

This is a shell-only helper method that vaguely represents some complex Gecko
use-cases. At the very least we should not crash with scary compartment
errors.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1a48b917af4a
Properly wrap envChainObject to jsshell evaluate. r=mgaudet

Comment 17

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/1a48b917af4a