IonMonkey: Assertion failure: iter.isNativeCall() && iter.callee().toFunction()->native() == DumpStack, at js/src/shell/js.cpp:2146

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Other Branch
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Assignee: general → nicolas.b.pierron
Status: NEW → ASSIGNED
Blocks: 677337
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.
(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.
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.
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.
Depends on: 748986
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.
Attachment #623351 - Flags: review?(dvander)
Blocks: 754491
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)
Attachment #623351 - Flags: review?(dvander) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/abbfb4bb6d17
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.