TM: Crash [@ 0x000fdf5b]

RESOLVED FIXED in mozilla1.9.1b4

Status

()

Core
JavaScript Engine
P1
critical
RESOLVED FIXED
9 years ago
5 years ago

People

(Reporter: gkw, Assigned: gal)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla1.9.1b4
x86
Mac OS X
crash, fixed1.9.1, regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey, crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
eval("\
  (function(){\
    for(y in [{},{},{}])\
    ((function(){\
      for(let y in [0])\
      y = this\
    })())\
  })()\
");

crashes opt and debug js shells with -j at a scary location (setting security-sensitive).

autoBisect shows this is probably related to bug 488203 :

The first bad revision is:
changeset:   27199:bf37cd061e3c
user:        Andreas Gal
date:        Tue Apr 14 19:52:09 2009 -0700
summary:     Properly compute 'this' object on trace and wrap if necessary (488203, r=mrbkap).

===

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

Thread 0 Crashed:
0   ???                           	0x000fdf5b 0 + 1040219
1   ???                           	0xbfffed98 0 + 3221220760
2   js-opt-tm-intelmac            	0x000a825b js_RecordLoopEdge(JSContext*, TraceRecorder*, unsigned int&) + 519
3   js-opt-tm-intelmac            	0x000a836e js_MonitorLoopEdge(JSContext*, unsigned int&) + 58
4   js-opt-tm-intelmac            	0x0003ecef js_Interpret + 43919
5   js-opt-tm-intelmac            	0x00043796 js_Execute + 572
6   js-opt-tm-intelmac            	0x0004f121 obj_eval(JSContext*, JSObject*, unsigned int, long*, long*) + 1911
7   js-opt-tm-intelmac            	0x00043f81 js_Invoke + 1719
8   js-opt-tm-intelmac            	0x0003479e js_Interpret + 1598
9   js-opt-tm-intelmac            	0x00043796 js_Execute + 572
10  js-opt-tm-intelmac            	0x0000c90a JS_ExecuteScript + 60
11  js-opt-tm-intelmac            	0x00004257 Process(JSContext*, JSObject*, char*, int) + 1559
12  js-opt-tm-intelmac            	0x0000680f main + 863
13  js-opt-tm-intelmac            	0x0000210b _start + 209
14  js-opt-tm-intelmac            	0x00002039 start + 41
Flags: blocking1.9.1?
(Assignee)

Updated

9 years ago
Assignee: general → gal
(Assignee)

Comment 1

9 years ago
Confirmed with TM tip. Good test case + reduction. Great work Gary.
(Assignee)

Updated

9 years ago
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1b4
(Assignee)

Comment 2

9 years ago
Created attachment 373414 [details] [diff] [review]
patch
(Assignee)

Comment 3

9 years ago
Created attachment 373415 [details] [diff] [review]
slightly less broken english in the comments
Attachment #373414 - Attachment is obsolete: true
Attachment #373415 - Flags: review?(brendan)

Updated

9 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Comment on attachment 373415 [details] [diff] [review]
slightly less broken english in the comments

>+    /*
>+     * In global code, bake in the global object as 'this' object.
>+     */
>+    if (!cx->fp->callee) {
>         JS_ASSERT(callDepth == 0);
>+        JSObject* thisObj = js_ComputeThisForFrame(cx, cx->fp);
>+        if (!thisObj)
>+            ABORT_TRACE("js_ComputeThis failed");
>+        JS_ASSERT(JSVAL_IS_OBJECT(cx->fp->argv[-1]));
>         this_ins = INS_CONSTPTR(thisObj);

Blank line here.

>+        /*
>+         * We don't have argv[-1] in global code, so we don't update the tracker here.
>+         */
>+        return true;
>+    }

Blank line here.

>+    /*
>+     * Traces type-specialize between null and objects, so if we currently see a null
>+     * value in argv[-1], this trace will only match if we see null at runtime as well.
>+     * Bake in the global object as 'this' object, updating the tracker as well. We
>+     * can only detect this condition prior to calling js_ComputeThisForFrame, since it
>+     * updates the interpreter's copy of argv[-1].
>+     */
>+    if (JSVAL_IS_NULL(cx->fp->argv[-1])) {
>+        JSObject* thisObj = js_ComputeThisForFrame(cx, cx->fp);
>+        if (!thisObj)
>+            ABORT_TRACE("js_ComputeThis failed");

Where's that bug on propagating errors to the interpreter? Graydon mentioned the control flow graph complexity of jstracer.cpp being very high, when looking into the general problem.

>+        JS_ASSERT(JSVAL_IS_OBJECT(cx->fp->argv[-1]));

You want to exclude null here, so !JSVAL_IS_PRIMITIVE(cx->fp->argv[-1]).

>+        JS_ASSERT(thisObj == globalObj);
>+        this_ins = INS_CONSTPTR(thisObj);
>+        set(&cx->fp->argv[-1], this_ins);
>         return true;
>     }
>     this_ins = get(&cx->fp->argv[-1]);

Blank line here.

/be
(Assignee)

Comment 5

9 years ago
Created attachment 373422 [details] [diff] [review]
v3
Attachment #373415 - Attachment is obsolete: true
Attachment #373422 - Flags: review?(brendan)
Attachment #373415 - Flags: review?(brendan)
Attachment #373422 - Flags: review?(brendan) → review+
(Assignee)

Comment 6

9 years ago
http://hg.mozilla.org/tracemonkey/rev/aae39925c259

Please remove security flag. The underlying patch never made it into trunk.
Whiteboard: fixed-in-tracemonkey
Group: core-security

Comment 7

9 years ago
http://hg.mozilla.org/mozilla-central/rev/aae39925c259
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Blocks: 489007
No longer blocks: 489007
Depends on: 489007
(Reporter)

Updated

9 years ago
Flags: in-testsuite?
Crash Signature: [@ 0x000fdf5b]
Bug in removed tracer code, setting in-testsuite- flag.
Flags: in-testsuite? → in-testsuite-
You need to log in before you can comment on or make changes to this bug.