Last Comment Bug 528644 - Crash [@ js_GetUpvar]
: Crash [@ js_GetUpvar]
Status: VERIFIED FIXED
[sg:critical][ccbr][3.6.x]fixed-in-tr...
: crash, regression, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: David Mandelin [:dmandelin]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks: jsfunfuzz 488034
  Show dependency treegraph
 
Reported: 2009-11-13 15:59 PST by Gary Kwong [:gkw] [:nth10sd]
Modified: 2012-03-23 16:55 PDT (History)
14 users (show)
sayrer: blocking1.9.2-
sayrer: wanted1.9.2+
dveditz: wanted1.9.0.x-
jruderman: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
needed
.7-fixed
needed
.11-fixed


Attachments
Patch (1.50 KB, patch)
2010-01-06 13:57 PST, David Mandelin [:dmandelin]
mrbkap: review+
christian: approval1.9.2.7+
christian: approval1.9.1.11+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2009-11-13 15:59:06 PST
function g(foo) {
  for (a in foo) function() {}
}
(g((eval("\
  function() {\
    for each(b in [0]) {\
      __defineGetter__(\"x\", \
        function(b) \
        eval(\"new function() { yield print(b) }\" ) \
      )\
    } \
    return x\
  }\
"))()))()


crashes js debug and opt shell at js_GetUpvar near null on TM tip on 10.5.8.

autoBisect shows this is probably related to bug 488034:

The first bad revision is:
changeset:   27186:70111870bcf8
user:        Brendan Eich
date:        Mon Apr 13 14:16:15 2009 -0700
summary:     Bug 488034 - Crash [@ js_GetUpvar] or "Assertion failure: (script)->upvarsOffset != 0, at ../jsinterp.cpp" (r=mrbkap).


Debug stack:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000018
Crashed Thread:  0

Thread 0 Crashed:
0   js-dbg-tm-darwin              	0x00098a46 js_GetUpvar + 76
1   js-dbg-tm-darwin              	0x0008d56c js_Interpret + 111234
2   js-dbg-tm-darwin              	0x0009e9ab __ZL15SendToGeneratorP9JSContext13JSGeneratorOpP8JSObjectP11JSGeneratorl + 561
3   js-dbg-tm-darwin              	0x0009ede8 __ZL12generator_opP9JSContext13JSGeneratorOpPlj + 610
4   js-dbg-tm-darwin              	0x0009ee91 __ZL14generator_nextP9JSContextjPl + 39
5   js-dbg-tm-darwin              	0x0009c1bb js_Invoke + 1503
6   js-dbg-tm-darwin              	0x0009cbb3 js_InternalInvoke + 151
7   js-dbg-tm-darwin              	0x0009fa60 js_CallIteratorNext + 360
8   js-dbg-tm-darwin              	0x000773a2 js_Interpret + 20664
9   js-dbg-tm-darwin              	0x0009b81b js_Execute + 1169
10  js-dbg-tm-darwin              	0x000111ba JS_ExecuteScript + 54
11  js-dbg-tm-darwin              	0x00009e5e __ZL7ProcessP9JSContextP8JSObjectPci + 458
12  js-dbg-tm-darwin              	0x0000aba0 __ZL11ProcessArgsP9JSContextP8JSObjectPPci + 2272
13  js-dbg-tm-darwin              	0x0000af6d main + 953
14  js-dbg-tm-darwin              	0x00001d6b _start + 209
15  js-dbg-tm-darwin              	0x00001c99 start + 41
Comment 1 Gary Kwong [:gkw] [:nth10sd] 2009-11-13 16:09:00 PST
(In reply to comment #0)
> crashes js debug and opt shell at js_GetUpvar near null on TM tip on 10.5.8.

... and occurs without -j.
Comment 2 Gary Kwong [:gkw] [:nth10sd] 2009-12-02 12:26:09 PST
Nominating blocking1.9.1? since it got turned security-sensitive and affects 1.9.1 branch.
Comment 3 Brendan Eich [:brendan] 2009-12-14 16:10:00 PST
Is this a nearly-null memory read, not exploitable?

/be
Comment 4 David Mandelin [:dmandelin] 2010-01-05 16:39:29 PST
I have the cause of the crash. The crash is in trying to access |b| in |yield print(b)|. This is compiled as a JSOP_GETUPVAR in a function at static level 5 getting a variable |x| defined at static level 3 (|function (b) ...|). But the function at level 3 has returned by the time we re-enter the function that accesses |b|, so we crash.

It seems that the generator function should not be compiled with JSOP_GETUPVAR at all, since it escapes. I'll have to dig in to the regressing patch and the closure compiler to figure out the exact cause and solution.
Comment 5 David Mandelin [:dmandelin] 2010-01-06 13:57:46 PST
Created attachment 420408 [details] [diff] [review]
Patch
Comment 6 David Mandelin [:dmandelin] 2010-01-06 14:34:50 PST
http://hg.mozilla.org/tracemonkey/rev/a578f79c7dd2
Comment 8 Daniel Veditz [:dveditz] 2010-06-25 12:47:04 PDT
This "ridealong" got left behind.
Comment 9 christian 2010-06-25 15:43:57 PDT
Comment on attachment 420408 [details] [diff] [review]
Patch

a=LegNeato for 1.9.2.6
Comment 10 christian 2010-06-25 15:45:21 PDT
Comment on attachment 420408 [details] [diff] [review]
Patch

Also approving for 1.9.1.11
Comment 11 David Mandelin [:dmandelin] 2010-06-25 16:27:29 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/65e8cc0b9c5c
Comment 12 David Mandelin [:dmandelin] 2010-06-25 16:30:56 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/dc2e2f77450e
Comment 13 Carsten Book [:Tomcat] 2010-07-01 13:49:35 PDT
gary: is there is a way to verify this fix in a debug build ?
Comment 14 Al Billings [:abillings] 2010-07-15 16:23:08 PDT
Any update on this? Gary?
Comment 15 Gary Kwong [:gkw] [:nth10sd] 2010-07-16 19:43:37 PDT
(In reply to comment #14)
> Any update on this? Gary?

I no longer crash using TM changeset 7cb520995757 on a 32-bit debug shell. Verified fixed.

Also verified on 1.9.2 changeset df760e6d4ddc and 1.9.1 changeset 2eb0724d74f8
Comment 16 Gary Kwong [:gkw] [:nth10sd] 2012-03-23 16:55:45 PDT
Verified per comment 15.

Note You need to log in before you can comment on or make changes to this bug.