All users were logged out of Bugzilla on October 13th, 2018

TM: Crash [@ TraceRecorder::test_property_cache]

VERIFIED FIXED in mozilla1.9.2a1

Status

()

P2
critical
VERIFIED FIXED
10 years ago
8 years ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {crash, testcase, verified1.9.1})

Trunk
mozilla1.9.2a1
crash, testcase, verified1.9.1
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
function f() {
  for ( foobar in (function() {
    for (var x = 0; x < 2; ++x) {
      if (x % 2 == 1) { yield (this.z.z) = function(){} }
      eval("this", false);
    }
  })())
  function(){}
}
f();

crashes both js opt and dbg shells with -j at TraceRecorder::test_property_cache near null. Seems to work as expected without -j.

Not sure if this warrants blocking1.9.1 but nominating just-in-case.

===

Full verbose output:

$ ./js-dbg-tm-intelmac -j
js> function f() {
  for ( foobar in (function() {
    for (var x = 0; x < 2; ++x) {
      if (x % 2 == 1) { yield (this.z.z) = function(){} }
      eval("this", false);
    }
  })())
  function(){}
}
Flushing fragments for JSScript 0x30ef30.
js> f();
capture stack type callee0: 0=O
capture stack type this0: 0=O
capture stack type vars0: 1=I
recording starting from typein:4@9
globalObj=0x29f000, shape=184
import vp=0x30ec80 name=$callee0 type=object flags=0
import vp=0x30ec84 name=$this0 type=object flags=0
import vp=0x30ec88 name=$<anonymous>.x type=int flags=0
    start
    state = param 0 ecx
    0
    sp = ld state[0]
    4
    rp = ld state[4]
    12
    cx = ld state[12]
    8
    gp = ld state[8]
    16
    eos = ld state[16]
    20
    eor = ld state[20]
    40
    globalObj = ld state[40]
    -24
    $callee0 = ld sp[-24]
    -16
    $this0 = ld sp[-16]
    -8
    ld1 = ld sp[-8]
    $<anonymous>.x = i2f ld1
    ld2 = ld cx[0]
    eq1 = eq ld2, 0
    xf1: xf eq1 -> pc=0x30ee65 imacpc=0x0 sp+0 rp+0

00009:   4  getlocal 0
00012:   4  int8 2
00014:   4  mod
    sti sp[0] = ld1
    #40000000:0 /* 2 */
    2
    sti sp[8] = 2
    js_imod1 = js_imod ( ld1 2 )

    -1
    eq2 = eq js_imod1, -1
    xt1: xt eq2 -> pc=0x30ee6a imacpc=0x0 sp+16 rp+0

00015:   4  one
00016:   4  eq
    i2f1 = i2f js_imod1
    sti sp[0] = js_imod1
    #3FF00000:0 /* 1 */
    1
    sti sp[8] = 1
    eq3 = eq js_imod1, 1
    xf2: xf eq3 -> pc=0x30ee6c imacpc=0x0 sp+16 rp+0

00020:   4  getthisprop "z"
    sti sp[0] = eq3
    eq4 = eq $this0, 0
    xt2: xt eq4 -> pc=0x30ee70 imacpc=0x0 sp+0 rp+0

    eq5 = eq $this0, globalObj
    xt3: xt eq5 -> pc=0x30ee70 imacpc=0x0 sp+0 rp+0

Bus error
Flags: blocking1.9.1?

Comment 1

10 years ago
Created attachment 363847 [details] [diff] [review]
patch

The comments above the return is not correct. getThis() return false when cx->fp->argv[-1] is null. Normally cx->fp->argv[-1] is the same as cx->fp->thisp. But there are still possibility that cx->fp->thisp is not exactly cx->fp->argv[-1]. So we still need to check cx->fp->thisp here.
Attachment #363847 - Flags: review?(gal)

Updated

10 years ago
OS: Mac OS X → All
Hardware: x86 → All
Attachment #363847 - Attachment is patch: true
Attachment #363847 - Attachment mime type: application/octet-stream → text/plain

Updated

10 years ago
Attachment #363847 - Flags: review?(gal) → review?(brendan)

Updated

10 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2

Updated

10 years ago
Attachment #363847 - Flags: review?(brendan) → review-
Comment on attachment 363847 [details] [diff] [review]
patch

>diff --git a/js/src/jstracer.cpp b/js/src/jstracer.cpp
>--- a/js/src/jstracer.cpp
>+++ b/js/src/jstracer.cpp
>@@ -9112,9 +9112,7 @@
> {
>     LIns* this_ins;
> 
>-    /* its safe to just use cx->fp->thisp here because getThis() returns false if thisp
>-       is not available */
>-    return getThis(this_ins) && getProp(cx->fp->thisp, this_ins);
>+    return getThis(this_ins) && cx->fp->thisp && getProp(cx->fp->thisp, this_ins);

This isn't right, we need to call js_ComputeThis under TraceRecorder::getThis if !(cx->fp->flags & JSFRAME_COMPUTED_THIS). See the COMPUTE_THIS macro in jsinterp.cpp.

/be
Leon, thanks for the patch (I didn't mean to sound ungrateful). If getThis calls js_ComputeThis, then I think the bug should be fixed, although more testing would be needed. Do you want to take this bug and patch it? It's a blocker and it needs to be assigned. If not, sayrer can find an owner.

/be

Comment 4

10 years ago
I can not see this bug in the latest tracemonkey tree and mozilla-central. The changeset http://hg.mozilla.org/tracemonkey/rev/5c7fe2d7c24d avoids the crash. Maybe the problem is just hidden by this check-in. Please assign this to other people or just close this bug.
(Reporter)

Comment 5

10 years ago
(In reply to comment #4)
> I can not see this bug in the latest tracemonkey tree and mozilla-central. The
> changeset http://hg.mozilla.org/tracemonkey/rev/5c7fe2d7c24d avoids the crash.
> Maybe the problem is just hidden by this check-in. Please assign this to other
> people or just close this bug.

Confirming WFM - I don't know whether that patch indeed fixes this bug, thus leaving resolution to someone else who might know.

Comment 6

10 years ago
http://hg.mozilla.org/tracemonkey/rev/5c7fe2d7c24d is not a fix. It might have change the behavior sufficiently to not trigger the problem in this particular testcase.
(Reporter)

Comment 7

10 years ago
-> WFM then.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: in-testsuite?
Resolution: --- → WORKSFORME
Please read bug 452498 comment 123, where I cited one of your tests and wrote

> Requires -j:
> =====
> (function(){ eval("for (x in ['', {}, '', {}]) { this; }" )})()
> 
> Assertion failure: fp->thisp == JSVAL_TO_OBJECT(fp->argv[-1]), at
> ../jstracer.cpp:4172

I think this is bug 479740.

Andreas, could you field this one? Should be easy to fix via js_ComputeThis, so long as that being called at recording time makes all the trace-execution-time assumptions true.

/be
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Oops, sorry about that -- should have read ahead in bugmail before commenting. I still do not like marking a blocker WFM, especially if the patch that seemed to "fix" the bug didn't do anything related to the crash site, by any theory of the crash we can think of.

But I can't think of a better way to resolve this bug. Can we leave it open for a bit in case the symptom reappears?

/be

Comment 10

10 years ago
Isn't this a dup of 457065?
(Reporter)

Comment 11

10 years ago
This seems to have been fixed by:

changeset:   25410:094c2e9de304
user:        Andreas Gal
date:        Wed Feb 25 18:47:22 2009 -0800
summary:     Improve blacklisting (479109, r=graydon).

I haven't yet verified this, I was using the script(s) created in bug 482536, and haven't tested the script fully yet.

Updated

10 years ago
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Comment 12

10 years ago
http://hg.mozilla.org/tracemonkey/rev/10f3c054d958
/cvsroot/mozilla/js/tests/js1_8/regress/regress-479740.js,v  <--  regress-479740.js
initial revision: 1.1
Flags: in-testsuite? → in-testsuite+
(Reporter)

Comment 13

10 years ago
(In reply to comment #11)
> This seems to have been fixed by:
> 
> changeset:   25410:094c2e9de304
> user:        Andreas Gal
> date:        Wed Feb 25 18:47:22 2009 -0800
> summary:     Improve blacklisting (479109, r=graydon).
> 
> I haven't yet verified this, I was using the script(s) created in bug 482536,
> and haven't tested the script fully yet.

I have manually verified that the fix in bug 479109 does indeed fix this issue as well.
Status: RESOLVED → VERIFIED
(Reporter)

Updated

10 years ago
Depends on: 479109

Updated

10 years ago
Keywords: fixed1.9.1
Verified fixed with testcase in comment 0 with the following debug builds:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre)
Gecko/20090522 Minefield/3.6a1pre ID:20090522133810

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre)
Gecko/20090522 Shiretoko/3.5pre ID:20090522153422
Keywords: fixed1.9.1 → verified1.9.1
Target Milestone: --- → mozilla1.9.2a1
Crash Signature: [@ TraceRecorder::test_property_cache]
You need to log in before you can comment on or make changes to this bug.