Last Comment Bug 748188 - IonMonkey: Assertion failure: iter.isNativeCall() && iter.callee().toFunction()->native() == DumpStack, at js/src/shell/js.cpp:2146
: IonMonkey: Assertion failure: iter.isNativeCall() && iter.callee().toFunction...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 748986
Blocks: 677337 IonEager 754491
  Show dependency treegraph
 
Reported: 2012-04-23 17:42 PDT by Nicolas B. Pierron [:nbp]
Modified: 2012-05-11 22:40 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add support for native frames. (10.75 KB, patch)
2012-05-11 16:19 PDT, Nicolas B. Pierron [:nbp]
dvander: review+
Details | Diff | Splinter Review

Description Nicolas B. Pierron [:nbp] 2012-04-23 17:42:11 PDT
This bug is related to the lack of creation of frame for native functions when we leave IonCode to execute a native function.  Either the activation (by creating/modifying the exit frame) or a StackFrame should be created to represent this case and to avoid the failure of this assertion.

function f() {
  print(dumpStack());
}

function g() {
  f();
}

function h() {
  var x = [1];
  for (var i = 0; i < 100000; i++)
    x.map(g);
}

h();

This test case is skipping the map function, because StackIter is not aware of it.
Comment 1 David Anderson [:dvander] 2012-04-23 21:44:30 PDT
I don't think this is worth fixing right now - there's an assert still firing on tbpl (I think) and let's just disable that, and come back to this when the debugger needs it.
Comment 2 Nicolas B. Pierron [:nbp] 2012-04-24 09:44:21 PDT
(In reply to David Anderson [:dvander] from comment #1)
> I don't think this is worth fixing right now - there's an assert still
> firing on tbpl (I think) and let's just disable that, and come back to this
> when the debugger needs it.

I would agree if I was precisely aware of what was not going right, my guess is that there is some potential misbehaving corner cases related to this bug.  So I want to verify if they are worth fixing now or later.
Comment 3 Nicolas B. Pierron [:nbp] 2012-04-24 15:06:26 PDT
The problem is that StackIter does not know how to read an exit frame to make a difference between a native function call or any VMFunction.  This likely need a bit of instrumentation in CallNative code generation to either produce a different kind of exit frame or to add a flag to the exit frame such as the iterator can identify that a native function has been called.

In the mean time, I will just comment the assertion as this does not seems to cause any other issue except inside dumpStack() shell function.
Comment 4 David Anderson [:dvander] 2012-04-26 08:46:43 PDT
We might end up needing to solve that for marking VMWrapper arguments (though I think we need to mark native arguments as well). Splitting IonFrame_Exit into two types should do it.
Comment 5 Nicolas B. Pierron [:nbp] 2012-05-11 16:19:20 PDT
Created attachment 623351 [details] [diff] [review]
Add support for native frames.

This patch is based on attachment 623206 [details] [diff] [review] (Bug 748986), it add support to detect
native calls from an exit frame and stop StackIter if the exit frame correspond to a native call.
Comment 6 David Anderson [:dvander] 2012-05-11 17:46:28 PDT
Comment on attachment 623351 [details] [diff] [review]
Add support for native frames.

Review of attachment 623351 [details] [diff] [review]:
-----------------------------------------------------------------

Glad this ended up being pretty straightforward.

::: js/src/ion/arm/IonFrames-arm.h
@@ +183,5 @@
>      inline uint8 *argBase() {
>          uint8 *sp = reinterpret_cast<uint8 *>(this);
>          return sp + IonExitFrameLayout::Size();
>      }
> +    inline Value *nativeVp() {

Should this assert that ionCode == NULL? (and != for argsBase)
Comment 7 Nicolas B. Pierron [:nbp] 2012-05-11 22:40:15 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/abbfb4bb6d17

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