The following testcase asserts on x64 with: Assertion failure: live->empty(), at ion/LinearScan.cpp:514

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?

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.

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.

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?