Open
Bug 1486426
Opened 6 years ago
Updated 1 year ago
Prevent callVM usage in non-terminal CacheIR Ops
Categories
(Core :: JavaScript Engine: JIT, enhancement, P3)
Core
JavaScript Engine: JIT
Tracking
()
NEW
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
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•