592962 - "Assertion failure: simsp <= i.sp()"

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

Testcase:
  default xml namespace = Proxy.create({get: Proxy.create});

Result:
  Assertion failure: simsp <= i.sp(), at jsfun.cpp:3099

Regression from:
  changeset:   52720:66c8ad02543b
  user:        Luke Wagner <[e-mail]>
  date:        Mon Aug 16 12:35:04 2010 -0700
  pushed:      Wed Sep 01 14:37:24 2010 -0700
  summary:     Bug 581263 - remove slow natives (r=waldo,mrbkap)

Result prior to regression:
  TypeError: getPropertyDescriptor is not a function

Comment 1

[Luke Wagner [:luke]]

Attachment: fix

This assertion just checks something I thought should be true: that the real stack pointer is at least as high as ReconstructStackDepth says it is.  However, the subsequent condition does not depend on this.

To make sure something more sinister wasn't afoot, I tracked down the bug and I think the issue is that defxmlns is popping before calling the function that ultimately calls js_ReconstructStackDepth.  I guess our error-reporting engine invariant is that you must report errors before consuming any of your nuses?  This patch seems to fix things.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 471547 [details] [diff] [review]
fix

(In reply to comment #1)
> invariant is that you must report errors before consuming any of your nuses? 

Yes, exactly. JSOP_DEFXMLNS blew it, besides sucking inherently ("default xml namespace = ...", blech!).

> BEGIN_CASE(JSOP_DEFXMLNS)
> {
>-    Value rval;
>-    POP_COPY_TO(rval);
>+    Value rval = regs.sp[-1];
>     if (!js_SetDefaultXMLNamespace(cx, rval))

js_SetDefaultXMLNamespace's final param is const Value &v, so you don't need rval any longer (or the braces). Why not just pass regs.sp[-1]?

r=me with that addressed. Thanks,

/be

Comment 3

[Brendan Eich [:brendan]]

(In reply to comment #1)
> invariant is that you must report errors before consuming any of your nuses? 

This invariant was required for GC safety prior to conservative stack scanning.

/be

Comment 4

[Luke Wagner [:luke]]

http://hg.mozilla.org/tracemonkey/rev/17a5586c96e1

Comment 5

[travis fisher]

arewefastyet.com/individual.php?machine=6 shows (if I read it right) that the tracing-jit-only time for bitwise-and more than doubled when this change landed.  I don't see how this can possibly be related however, but I'm just a spectator.  Any idea?

Comment 6

[Luke Wagner [:luke]]

Heh, nope -- SS is definitely not executing JSOP_DEFXMLNS.

Comment 7

[Jesse Ruderman]

Sounds like this was fixed in September.