Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now) Reporter
	Description • 14 years ago

(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

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 1 • 14 years ago

Chris, is it true that argv[-1] should == fp->thisv? Do they get updated in sync when we compute |this|?

	Chris Leary [:cdleary] (not checking bugmail)
	Comment 2 • 14 years ago

I think the patch in bug 587809 fixes it.

	Chris Leary [:cdleary] (not checking bugmail)
	Comment 3 • 14 years ago

Wait... dmandelin said he landed that already.

	Chris Leary [:cdleary] (not checking bugmail)
	Comment 4 • 14 years ago

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].

	Jesse Ruderman
	Comment 5 • 14 years ago

(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

	David Mandelin [:dmandelin]
	Comment 6 • 14 years ago

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?

	David Anderson [:dvander] - inactive, e-mail if emergency
	Comment 7 • 14 years ago

dbaron hit this today - Chris, had a chance to see comment #6?

	Luke Wagner [:luke]
	Comment 9 • 14 years ago

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.

	Jesse Ruderman
	Comment 10 • 14 years ago

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)

	Luke Wagner [:luke]
	Comment 11 • 14 years ago

Hah.  Yeah, that assertion totally had it coming.

	Christian Holler (:decoder)
	Comment 12 • 12 years ago

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929