Last Comment Bug 688327 - spidermonkey shell (Debug build) runs code incorrectly when both -m -D are specified
: spidermonkey shell (Debug build) runs code incorrectly when both -m -D are sp...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla9
Assigned To: general
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-21 16:51 PDT by David Flanagan [:djf]
Modified: 2011-09-27 05:36 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case. Run with -D -m and pipe stdout to a file to hide all the -D output so you can see the TypeError (726 bytes, text/x-c)
2011-09-21 16:51 PDT, David Flanagan [:djf]
no flags Details
gdb stack trace (3.02 KB, text/plain)
2011-09-22 12:35 PDT, David Flanagan [:djf]
no flags Details
JM spew of problematic code sequence (6.38 KB, text/plain)
2011-09-22 14:44 PDT, Steve Fink [:sfink] [:s:] (PTO Sep23-28)
no flags Details
inline stub calls can keep more registers live (850 bytes, patch)
2011-09-22 15:11 PDT, Steve Fink [:sfink] [:s:] (PTO Sep23-28)
dvander: review+
dflanagan: feedback+
Details | Diff | Splinter Review

Description David Flanagan [:djf] 2011-09-21 16:51:38 PDT
Created attachment 561614 [details]
Test case. Run with -D -m and pipe stdout to a file to hide all the -D output so you can see the TypeError

The attached test calls Object.defineProperty() in some vaguely interesting ways, but doesn't actually do anything.  Invoke it in the spidermonkey shell and it runs to completion and exits.

But, if you run it with -D and -m it throws a TypeError.  It runs fine with either -D or -m alone, though, so it is some interaction between the PCCOUNTS and the method jit.  Note that the test case just calls the same function 17 times.  The 17th fails, and I'm guessing that the method jit kicks in after the 16th execution...
Comment 1 David Flanagan [:djf] 2011-09-22 12:35:56 PDT
Created attachment 561834 [details]
gdb stack trace

I generated this stack trace by setting breakpoint in JS_ReportErrorNumber and running a debugging build of the spidermonkey shell with -D and -m and the attached test case.

I was able to verify that at stack frame 5, the first argument to Object.defineProperty() call was not tagged as an object.  But in stack frame #10, before method jitting begins, args.base()[2] is indeed an object.

So the args are getting corrupted somewhere, but the code is too opaque for me to figure it out.  Perhaps at mystery stack frame #6?
Comment 2 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-09-22 14:44:28 PDT
Created attachment 561890 [details]
JM spew of problematic code sequence
Comment 3 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-09-22 15:11:21 PDT
Created attachment 561900 [details] [diff] [review]
inline stub calls can keep more registers live

djf, can you try this patch on your example? It fixes mine. bhackett looked at it and figured it out in no time.
Comment 4 David Flanagan [:djf] 2011-09-22 15:16:46 PDT
The patch fixes the issue for me.  Thanks!
Comment 5 Steve Fink [:sfink] [:s:] (PTO Sep23-28) 2011-09-23 10:21:37 PDT
Comment on attachment 561900 [details] [diff] [review]
inline stub calls can keep more registers live

We can carry callee-saved registers live across a FASTCALL, so using AvailRegs (aka SavedRegs | TempRegs) is incorrect.
Comment 6 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-09-27 05:36:33 PDT
https://hg.mozilla.org/mozilla-central/rev/ee8a3069dc4c

Note You need to log in before you can comment on or make changes to this bug.