Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Description • 14 years ago

function f2(c) {
  return {
    g: c.match(/:/) & c.match(/:/) & c.match(/f/) & c.m & c.a & c.match(/f/) &
        c.match(/c/) & c.match(/a/) & (/s/) & c.match(/e/) & c.match(/s/) &
        (c.match(/./) & c.i) & (c.match(/\*/)) & (c.match(/n/)),
    y: c.a & c.a & c.match(/\)/) & c.match(/\)/) & c.match(/\)/)
        & c.match(/\\/),
    c: c.match(/f/)
  }
}
function f1(c) {
  t = f2(c.replace(/s/))
  try {
    eval(c)
  } catch(e) {}
  f4();
  if (c.indexOf("<") == -1 || c.indexOf())
  try {} catch(e) {}
  try {
    try {
      l
    } catch(e) {}
    if ("unwatch" in this) {}
    g
  } catch(e) {}
  try {} catch(p) {}
}
function f4() {
  try {} catch(e) {} {
    try {
      eval(s + "")
    } catch(e) {}
  }
} [{}]
s = [{},{}];
(function(){}())
s[{},{},{}] = [function(){}]
a = [{},{},{}].concat([{},{}]) 
f1("") 
f1("") 
f1("n") 
f1("") 
f1("(__proto__=null)") 
f1("for(var z=0;z<1;z++){gc()(*::*)}") 
f1("gc()") 
f1("<")


crashes js debug shell on JM changeset 0b7f385f3f59 with -m at 0xdadadade with js::methodjit::JaegerShot near the top of the stack. I tested this on 32-bit Mac debug build off changeset 0b7f385f3f59 because tip didn't compile for me. :(

(Not exactly the smallest) regression window:

http://hg.mozilla.org/users/danderson_mozilla.com/jaegermonkey/pushloghtml?fromchange=0075ab7ae8fe&tochange=0b7f385f3f59


Stack:

Exception Type:  EXC_BAD_ACCESS (SIGSEGV)
Exception Codes: KERN_INVALID_ADDRESS at 0x00000000dadadade
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   ???                           	0x003ec4b9 0 + 4113593
1   js-dbg-32-jm-darwin           	0x001e2e82 js::methodjit::JaegerShot(JSContext*) + 408
2   js-dbg-32-jm-darwin           	0x000a0f04 js_RunScript + 158
3   js-dbg-32-jm-darwin           	0x000a1453 js_Execute + 1253
4   js-dbg-32-jm-darwin           	0x00012255 JS_ExecuteScript + 54
5   js-dbg-32-jm-darwin           	0x0000aaf5 Process(JSContext*, JSObject*, char*, int) + 458 (js.cpp:450)
6   js-dbg-32-jm-darwin           	0x0000b862 ProcessArgs(JSContext*, JSObject*, char**, int) + 2326 (js.cpp:870)
7   js-dbg-32-jm-darwin           	0x0000bc2f main + 953 (js.cpp:4975)
8   js-dbg-32-jm-darwin           	0x00002881 _start + 208
9   js-dbg-32-jm-darwin           	0x000027b0 start + 40

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 1 • 14 years ago

Reproducible with JM tip.

	David Mandelin [:dmandelin] Assignee
	Comment 2 • 14 years ago

The cause: For the |indexOf| operations on line 17, we bake in the identity of the String class prototype. Then, GC apparently collects it and we start navigating through an invalid object the next time we use the PIC on line 17.

This could be solved by invalidating any primitive-string PICs on every GC. That's painful, though. It seems like it would be better to bake in the pointer to the string method, and then have some guard or invalidation to detect monkey-patching of String.prototype over standard methods.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Comment 3 • 14 years ago

This seems to causing most of the current js::methodjit::JaegerShot crashes in JM now, when running jsfunfuzz. :(

	David Mandelin [:dmandelin] Assignee
	Comment 4 • 14 years ago

OK, I'll bump up the priority on this. I do want to finish an investigation of the micro-level perf of the PICs first.

	David Mandelin [:dmandelin] Assignee
	Comment 5 • 14 years ago

The bug is in jsapi.cpp, so I want to land the fix on TM. It's pretty clear the bug is just a typo.

My earlier guess that |String.prototype| was getting GC'd was correct. The detailed cause is this:

1. At some point, String.prototype is initialized and stored in slot 12 of the global object, which is its normal home.
2. Evaluating |"unwatch" in this| calls the global resolver hook. This in turn calls JS_ResolveStandardClass. JS_ResolveStandardClass finds the right entry, but due to a typo it reads out the wrong name element, which causes it to call the init function for the string standard class. This overwrites the global slot where String.prototype is stored.
3. A GC gets run, and the old String.prototype gets collected.
4. A JM PIC stub that assumes that String.prototype cannot change (per ECMA-262 5e, 15.5.3.1) crashes.

	Jeff Walden [:Waldo]
	Comment 6 • 14 years ago

Comment on attachment 440080 [details] [diff] [review]
Patch

Goes back to 2006 at least, definitely in stable releases with TM, guessing probably triggerable somehow...should be fixed on branches too.

	Jeff Walden [:Waldo]
	Comment 7 • 14 years ago

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsapi.c&rev=3.453
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&root=/cvsroot&subdir=mozilla/js/src&command=DIFF_FRAMESET&file=jsapi.c&rev2=3.266&rev1=3.265

	David Mandelin [:dmandelin] Assignee
	Comment 8 • 14 years ago

http://hg.mozilla.org/tracemonkey?pushloghtml

	Robert Sayre
	Comment 9 • 14 years ago

http://hg.mozilla.org/mozilla-central/rev/d0e7da895292

	Daniel Veditz [:dveditz]
	Comment 10 • 14 years ago

Nominating for branches based on comment 6. Can't reproduce the crash using the given testcase but the code has the same problem.

	David Mandelin [:dmandelin] Assignee
	Comment 11 • 14 years ago

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d1179bed506f
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/cc28ca4d7ca2

	Al Billings [:abillings - ex-MoCo]
	Comment 12 • 14 years ago

(In reply to comment #10)
> Nominating for branches based on comment 6. Can't reproduce the crash using the
> given testcase but the code has the same problem.

No way for QA to verify this fix in the branch releases.

	Ed Morley [:emorley]
	Comment 13 • 12 years ago

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