Closed Bug 678072 Opened 8 years ago Closed 8 years ago

IonMonkey: Assertion failure: live->empty(), at ion/LinearScan.cpp:514


(Core :: JavaScript Engine, defect)

Not set





(Reporter: h4writer, Assigned: adrake)




(2 files, 2 obsolete files)

Attached file testcase
The following testcase asserts on x64 with:
Assertion failure: live->empty(), at ion/LinearScan.cpp:514
Blocks: anion
Blocks: IonMonkey
No longer blocks: anion
Blocks: 677337
No longer blocks: IonMonkey
Attached patch Patch v0 (obsolete) — Splinter Review
Unbox instructions snapshot their own outputs, causing liveness analysis to show the output as live into the instruction. This patch adds a case to avoid propagating liveness from uses of just-defined values, with the assertion that they are snapshot inputs.

dvander: This seems kind of strange from a snapshot perspective -- should LIR snapshots really include values defined by their assigned instructions?
Assignee: general → adrake
Attachment #552307 - Flags: review?(dvander)
Snapshotting your own output is totally bad. We should look into that instead. Type analysis is somewhat careful to make sure that doesn't happen - but that would be the first place to look.
Attachment #552307 - Flags: review?(dvander)
Attached patch Patch v1 (obsolete) — Splinter Review
Explicitly include the "this" parameter when setting snapshots so we don't leave off the last argument. If we should be excluding the this slot, or if there's a less-hacky way than +1, I'll do that instead.
Attachment #552307 - Attachment is obsolete: true
Attachment #552329 - Flags: review?(dvander)
Attachment #552329 - Attachment is obsolete: true
Attachment #552329 - Flags: review?(dvander)
Attachment #552330 - Flags: review?(dvander)
Comment on attachment 552330 [details] [diff] [review]
Patch v1, now with 100% more patch

Review of attachment 552330 [details] [diff] [review]:

::: js/src/ion/IonBuilder.cpp
@@ +187,5 @@
>      current->makeStart(start);
>      // Attach start's snapshot to each parameter, so the type analyzer doesn't
>      // replace uses in the snapshot.
> +    for (uint32 i = 0; i < nargs() + 1; i++) {

Nice catch. Could you use i < CountArgSlots(fun()) instead?
Attachment #552330 - Flags: review?(dvander) → review+
Closed: 8 years ago
Resolution: --- → FIXED
Duplicate of this bug: 678550
You need to log in before you can comment on or make changes to this bug.