Closed Bug 591418 Opened 11 years ago Closed 11 years ago

JM: "Assertion failure: fp->argv[-1] == fp->getThisValue(),"

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: gkw, Unassigned)

References

Details

(Keywords: assertion, regression, testcase)

(function() {
  for each(let z in [0, 0, 0, 0]) {
    eval("\
      for(var a = 0; a < 1; ++a) {\
        this\
      }\
    ")
  }
})()

asserts js debug shell on JM changeset 33b05dd43cd4 with -j at Assertion failure: fp->argv[-1] == fp->getThisValue(), at ../jstracer.cpp:10090
Chris, is it true that argv[-1] should == fp->thisv? Do they get updated in sync when we compute |this|?
I think the patch in bug 587809 fixes it.
Depends on: 587809
Wait... dmandelin said he landed that already.
thisv is |null| and argv[-1] is the global object -- I have to look into where recording of the JSOP_THIS occurs with respect to the interpreter executing it, because a |null| thisv indicates lazy compute-and-set of argv[-1].
(In reply to comment #2)
> I think the patch in bug 587809 fixes it.

autobisect tells me the opposite: that the patch in bug 587809 caused it.

The first bad revision is:
changeset:   50705:305b00ec07e7
user:        Chris Leary <cdleary@mozilla.com>
date:        Wed Aug 18 18:17:30 2010 -0700
summary:     [JAEGER] Bug 587809 pre-landing: land tracer changes for eager this so we can see what it does with X64 on tinderbox
This comment in jsinterp.h suggest that the assert is bogus here:

     * Usually if argv != NULL then thisv == argv[-1], but natives may
     * assign to argv[-1]. Also, obj_eval can trigger a special case
     * where two stack frames have the same argv. If one of the frames fills
     * in both argv[-1] and thisv, the other frame's thisv is left null.

The assertion is:

    JS_ASSERT_IF(fp->argv, fp->argv[-1] == fp->getThisValue());

The comment suggests that when eval is in play, it is OK for fp->getThisValue() to return NULL. 

Chris, what do you think? Should we just:

    JS_ASSERT_IF(fp->argv && !fp->getThisValue().isNull, 
                 fp->argv[-1] == fp->getThisValue());

Or should we be looking for fp->argv == fp->down->argv or some such?
dbaron hit this today - Chris, had a chance to see comment #6?
blocking2.0: --- → ?
blocking2.0: ? → final+
I see someone else has already removed the presumed-bogus assert.  I'd dup the bug, but its hard to find when a line was deleted using hg annotate.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Yes, looks like *someone* already removed the bogus assert ;)  hg grep ftw.

hg cat -r 33b05dd43cd4 jstracer.cpp | edit

line 10090 is
JS_ASSERT_IF(fp->argv, fp->argv[-1] == fp->getThisValue());

hg grep --all "JS_ASSERT_IF\(fp" jstracer.cpp

changeset:   8721b595e7ab
user:        Luke Wagner
date:        Mon Aug 09 22:43:33 2010 -0700
summary:     Bug 539144 - Make formal args a jit-time const offset from fp; rm argv/argc/thisv/script/callobj (r=brendan,dvander)
Resolution: WORKSFORME → FIXED
Hah.  Yeah, that assertion totally had it coming.
Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.