Open Bug 1486426 Opened Last year

Prevent callVM usage in non-terminal CacheIR Ops

Categories

(Core :: JavaScript Engine: JIT, enhancement, P3)

enhancement

Tracking

()

Tracking Status
firefox63 --- affected

People

(Reporter: mgaudet, Unassigned)

References

(Blocks 1 open bug)

Details

In Bug 1467907 I discovered that there's a limitation in the usage of callVM in CacheIR that currently aren't detected statically. At the bare minimum, we should document these, but it would also be nice to try at catch mistakes here to help future humans exploring CacheIR. 

The Limitation:

Suppose you have a sequence of CacheIR ops: 

    FooOperation 0 
    BarOperation 1
    BazOperation 0 1 
    ReturnFromIC


You can only safely use callVM in the last operation before ReturnFromIC. This is because, as far as I can tell, there's no easy way to communicate the liveness constraints you may have to callVM to ensure live operands are preserved.

If, in this example, you tried to implement BarOperation with callVM, what would happen is that the 0-th operand may well be clobbered, meaning BazOperation is really working on garbage.

There is already -some- support to help make this kind of error show up: CacheRegisterAllocator::discardStack() currently preceeds every call to callVM. This marks all operand locations as 'uninitialized' in an attempt to prevent this kind of error. However, because it is a separate routine, it seems like it is optional and could be elided if you want to preserve the operand stack. This is not the case. 

Potential Solutions:

1. Inline discardStack in prepareVMCall() (Ion) and AutoStubFrame::enter() (Baseline) to ensure that operand locations are clearly marked as uninitialized whenever callVM works (see also Bug 1467191 about unifying these mechanisms)
2. Enhance callVM code to use the liveness analysis on operandLocations, indicating what operand locations are still live. This machinery already exists for freeing dead operand locations [1]
3. ??? 


[1]: https://searchfox.org/mozilla-central/rev/e126996d9b0a3d7653de205898517f4f5b632e7f/js/src/jit/CacheIRCompiler.cpp#318
You need to log in before you can comment on or make changes to this bug.