TM: Crash [@ UpvarArgTraits::interp_get] or [@ GetUpvarArgOnTrace]

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
9 years ago
4 years ago

People

(Reporter: streetwolf, Assigned: dvander)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
crash, crashreportid, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ccbr][sg:critical?] fixed-in-tracemonkey, crash signature, URL)

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a1pre) Gecko/20091218 Minefield/3.7a1pre Firefox/3.5.5
Build Identifier: 20091218043634

Clicked on URL provided, page appeared briefly then crashed to desktop with the FF crash menu.

Reproducible: Always

Steps to Reproduce:
1. Go to URL provided
2.
3.
Actual Results:  
Crash to desktop

Expected Results:  
No crash

Crash Report:

http://crash-stats.mozilla.com/report/index/9da734e0-629e-4e04-bb54-f2c8e2091218
(Reporter)

Updated

9 years ago
Component: Disability Access APIs → DOM: Core & HTML
Version: unspecified → Trunk
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091218 Minefield/3.7a1pre.

But no crash with javascript.options.jit.content to false.
No crash with Namoroko so far.
This must have regressed in the last few days.
Assignee: nobody → general
Status: UNCONFIRMED → NEW
Component: DOM: Core & HTML → JavaScript Engine
Ever confirmed: true
Keywords: crash, crashreportid, regressionwindow-wanted
QA Contact: accessibility-apis → general
0  	mozjs.dll  	GetUpvarOnTrace<UpvarArgTraits>  	 js/src/jstracer.cpp:3179
1 	mozjs.dll 	GetUpvarArgOnTrace 	js/src/jstracer.cpp:3199
2 	mozjs.dll 	js_MonitorLoopEdge 	js/src/jstracer.cpp:7089
Summary: Crash after page appears for a second or two. → Crash after page appears for a second or two. [@ GetUpvarOnTrace<UpvarArgTraits>(JSContext*, unsigned long, long, unsigned long, double*) ]
Crash still present in the latest tracemonkey build.
I'm on this. After some investigation, it's actually crashing at:

http://www.evri.com/widget/javascripts/ZiffDavis.1.js

Reduced testcase coming right up.

Comment 5

9 years ago
Regression window is hard to find.

The last working TM build (and without the crash) was:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091205 Minefield/3.7a1pre
http://hg.mozilla.org/tracemonkey/rev/21b06416fa71

Then all TM builds (up to and including the 20091215 build) were unusable, crashing immediately on startup.  This started with:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091206 Minefield/3.7a1pre
http://hg.mozilla.org/tracemonkey/rev/ced680bb4404

Finally, TM build started working again and this crash now exists, but did it begin with the 20091206 build?  First TM build to load again without crashing is:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a1pre) Gecko/20091216 Minefield/3.7a1pre
http://hg.mozilla.org/tracemonkey/rev/6687fe3cda87
(function () {
    p = function () {
        Set()
    };
    var Set = function () {};
    for (var x = 0; x < 5; x++) {
        Set = function (z) {
            return function () {
                [z]
            }
        } (x)
    }
})()

crashes js debug shell at UpvarArgTraits::interp_get near null and js opt shell at GetUpvarArgOnTrace near null on TM tip with -j.

Huge pain to reduce, but it's eventually done. Setting security-sensitive because of memory addresses on stack, hopefully more-experienced eyes will re-diagnose this.

Debug stack:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000028
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   js-dbg-32-tm-darwin           	0x0016bae5 UpvarArgTraits::interp_get(JSStackFrame*, int) + 9
1   js-dbg-32-tm-darwin           	0x001729cd JSTraceType_ GetUpvarOnTrace<UpvarArgTraits>(JSContext*, unsigned int, int, unsigned int, double*) + 507
2   js-dbg-32-tm-darwin           	0x0014786b GetUpvarArgOnTrace(JSContext*, unsigned int, int, unsigned int, double*) + 51
3   ???                           	0x002e6ea0 0 + 3042976
4   js-dbg-32-tm-darwin           	0x00135b63 ExecuteTrace(JSContext*, nanojit::Fragment*, InterpState&) + 89
5   js-dbg-32-tm-darwin           	0x00168f90 ExecuteTree(JSContext*, TreeFragment*, unsigned int&, VMSideExit**) + 629
6   js-dbg-32-tm-darwin           	0x0016a2df js_MonitorLoopEdge(JSContext*, unsigned int&, RecordReason) + 995
7   js-dbg-32-tm-darwin           	0x0007d30a js_Interpret + 42491
8   js-dbg-32-tm-darwin           	0x0009c0b8 js_Execute + 1169
9   js-dbg-32-tm-darwin           	0x000116d2 JS_ExecuteScript + 54
10  js-dbg-32-tm-darwin           	0x0000a617 Process(JSContext*, JSObject*, char*, int) + 1347 (js.cpp:532)
11  js-dbg-32-tm-darwin           	0x0000afe8 ProcessArgs(JSContext*, JSObject*, char**, int) + 2281 (js.cpp:849)
12  js-dbg-32-tm-darwin           	0x0000b3b5 main + 953 (js.cpp:4862)
13  js-dbg-32-tm-darwin           	0x0000246d _start + 208
14  js-dbg-32-tm-darwin           	0x0000239c start + 40

Opt stack:

Exception Type:  EXC_BAD_ACCESS (SIGBUS)
Exception Codes: KERN_PROTECTION_FAILURE at 0x0000000000000028
Crashed Thread:  0  Dispatch queue: com.apple.main-thread

Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
0   js-opt-32-tm-darwin           	0x000f13cb GetUpvarArgOnTrace(JSContext*, unsigned int, int, unsigned int, double*) + 187
1   ???                           	0x003d2ea0 0 + 4009632
2   js-opt-32-tm-darwin           	0x000f9398 ExecuteTree(JSContext*, TreeFragment*, unsigned int&, VMSideExit**) + 744
3   js-opt-32-tm-darwin           	0x00114434 js_MonitorLoopEdge(JSContext*, unsigned int&, RecordReason) + 1172
4   js-opt-32-tm-darwin           	0x0005bb8c js_Interpret + 56972
5   js-opt-32-tm-darwin           	0x0005db2c js_Execute + 444
6   js-opt-32-tm-darwin           	0x0000d8dc JS_ExecuteScript + 60
7   js-opt-32-tm-darwin           	0x00004658 Process(JSContext*, JSObject*, char*, int) + 1336
8   js-opt-32-tm-darwin           	0x000086d6 main + 1734
9   js-opt-32-tm-darwin           	0x000025fd _start + 208
10  js-opt-32-tm-darwin           	0x0000252c start + 40
Group: core-security
Severity: major → critical
Keywords: regression, testcase
OS: Windows 7 → All
Hardware: x86_64 → All
Summary: Crash after page appears for a second or two. [@ GetUpvarOnTrace<UpvarArgTraits>(JSContext*, unsigned long, long, unsigned long, double*) ] → TM: Crash [@ UpvarArgTraits::interp_get] or [@ GetUpvarArgOnTrace]
Was tested on 10.6.2 and assuming [sg:critical?] unless otherwise noted.

AutoBisecting now...
Whiteboard: [ccbr][sg:critical?]
autoBisect shows this is probably related to bug 515749:

The first bad revision is:
changeset:   35427:538a07fdf3d2
user:        David Anderson
date:        Fri Dec 11 19:10:36 2009 -0800
summary:     Lazily import stack and global slots (bug 515749, original patch and r=gal).
Blocks: 515749
Keywords: regressionwindow-wanted
dvander would like this assigned to him... (over IRC)
Assignee: general → dvander
Status: NEW → ASSIGNED

Comment 10

9 years ago
Slightly simpler:

(function () {
    var Set;
    var p = function () {
        Set;
    };
    for (var x = 0; x < 5; x++) {
        Set = (function (z) {
            return function () {
                z;
            };
        }) (x)
    }
})()
Created attachment 418527 [details] [diff] [review]
fix

The meaning of TraceRecorder::known() is different now that we lazily import slots. I did not catch this case though, so it generates bad code.
Attachment #418527 - Flags: review?(dmandelin)
Attachment #418527 - Flags: review?(dmandelin) → review+
http://hg.mozilla.org/tracemonkey/rev/f7cff6dd16f1
Whiteboard: [ccbr][sg:critical?] → [ccbr][sg:critical?] fixed-in-tracemonkey
Backed out, get() asserts somewhere. http://hg.mozilla.org/tracemonkey/rev/28ba3d9a651f
Whiteboard: [ccbr][sg:critical?] fixed-in-tracemonkey → [ccbr][sg:critical?]
Created attachment 418750 [details] [diff] [review]
actual fix

Brain fart, of course get() expects an on-trace value, so it will always return something. This version passes closure tests.
Attachment #418527 - Attachment is obsolete: true
Attachment #418750 - Flags: review?(dmandelin)
Comment on attachment 418750 [details] [diff] [review]
actual fix

>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -12126,16 +12126,23 @@ TraceRecorder::upvar(JSScript* script, J
>      */
>     uint32 cookie = uva->vector[index];
>     jsval& vr = js_GetUpvar(cx, script->staticLevel, cookie);
>     v = vr;
> 
>     if (known(&vr))
>         return get(&vr);
> 
>+    /* If the variable was not known, it could require a lazy import. */
>+    CountSlotsVisitor countVisitor(&vr);
>+    VisitStackSlots(countVisitor, cx, callDepth);
>+
>+    if (countVisitor.stopped() || size_t(&vr - cx->fp->slots) < cx->fp->script->nslots)
>+        return get(&vr);
>+

How about:

bool TraceRecorder::can_get(jsval *vp)
{
    if (known(vp))
        return true;

    /* If the variable was not known, it may be available for lazy import. */
    CountSlotsVisitor countVisitor(&vr);
    VisitStackSlots(countVisitor, cx, callDepth);
    return countVisitor.stopped() || size_t(&vr - cx->fp->slots) < cx->fp->script->nslots;
}

and then

    if (can_get(&vr))
        return get(&vr);

It seems good for confusion reduction to have an direct API that says whether |get| will succeed.

Comment 16

9 years ago
also http://time.yandex.ru
Blocks: 532972

Comment 17

9 years ago
crash data also says

   2 http://www.washingtonpost.com/wp-dyn/content/article/2009/12/13/AR2009121300007.html?nav=hcmodule
   1 http://www.washingtonpost.com/wp-dyn/content/article/2009/12/18/AR2009121804042.html?hpid=topnews
   1 http://www.washingtonpost.com/wp-dyn/content/article/2009/12/13/AR2009121300007.html?waporef=ak

is this trunk only or are patches needed for 1.9.2?  the only reports I've seen in the crash database show 3.7a1pre and were logged on 12-18,19,20

20091217-crashdata 0 GetUpvarArgOnTrace
20091218-crashdata 14 GetUpvarArgOnTrace
20091219-crashdata 16 GetUpvarArgOnTrace
20091220-crashdata 9 GetUpvarArgOnTrace
This only affects trunk after 2009-12-11.
(In reply to comment #15)
> (From update of attachment 418750 [details] [diff] [review])
> bool TraceRecorder::can_get(jsval *vp)

Once upon a time we had a has() method, shorter name than known or can_get, and it avoids the underscore war against camelCaps (_ is righteous here, admittedly).

/be
Created attachment 418923 [details] [diff] [review]
v3
Attachment #418750 - Attachment is obsolete: true
Attachment #418923 - Flags: review?(dmandelin)
Attachment #418750 - Flags: review?(dmandelin)
Created attachment 418924 [details] [diff] [review]
v3 actual patch

attached the wrong patch.
Attachment #418923 - Attachment is obsolete: true
Attachment #418924 - Flags: review?(dmandelin)
Attachment #418923 - Flags: review?(dmandelin)
Attachment #418924 - Flags: review?(dmandelin) → review+
http://hg.mozilla.org/tracemonkey/rev/167467b7b439
Whiteboard: [ccbr][sg:critical?] → [ccbr][sg:critical?] fixed-in-tracemonkey
Duplicate of this bug: 536673

Comment 24

9 years ago
If it helps, I can reproduce this pretty reliably on many pages of washingtonpost.com

Comment 25

9 years ago
http://hg.mozilla.org/mozilla-central/rev/167467b7b439
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 536921
Crash Signature: [@ UpvarArgTraits::interp_get] [@ GetUpvarArgOnTrace]
Group: core-security
You need to log in before you can comment on or make changes to this bug.