Replace all `UniquePtr<CompilationStencil>` by `RefPtr<CompilationStencil>`
Categories
(Core :: JavaScript Engine, task, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox97 | --- | fixed |
People
(Reporter: nbp, Assigned: nbp)
References
Details
Attachments
(3 files)
Today, most method for returning a CompilationStencil are returning it as a UniquePtr<CompilationStencil>. This is problematic as we are adding caches to store CompilationStencils as RefPtr<CompilationStencil>.
As we are attempting to skip the compilation, we check for cache hits and return the value of the cache entry which is found. However, a RefPtr<…> cannot be converted to a UniquePtr<…>, even if we remove it from the cache.
The alternative is not pleasing either, as the only safe way to return a UniquePtr<…>, is to clone it. Thus making a copy of the memory and all data structure inside it.
Borrowing is not an option either, as the cache might be cleared (especially when Web Workers would want to play with caches as well) while we might be instantiating form the CompilationStencil which is stored in the cache.
Therefore, the easiest solution remaining is to only manipulate CompilationStencil using a RefPtr<…> or a const CompilationStencil& where the Stencil is garanteed to be ref-counted.
|
Assignee | |
Comment 1•2 years ago
|
||
RefPtr<CompilationStencil> would be used for caching the result of compilation
which might be reused by other functions inside SpiderMonkey. However, many
compilation functions are returning UniquePtr<CompilationStencil>.
Converting a RefPtr<..> to a UniquePtr<..>, when the ref-count is not equal to
1, or when the value still exists in a cache will cause some UAF. This assertion
prevents such mistake to happen in future patches.
|
|
Assignee | |
Comment 2•2 years ago
|
||
Currently, CompilationStencil are used both as UniquePtr and RefPtr. This is
risky and might lead to unwanted UAF. This patch replaces all uses of UniquePtr
by RefPtr, and already_addRefed, such that we either have or not reference
counted Stencils.
|
|
Assignee | |
Comment 3•2 years ago
|
||
(This patch will conflict with other changes, but the spirit would remain)
Previously, we were using UniquePtr<..> for CompilationStencil, which is ok
to move, as we have the garantee that we have a single entity.
However, as we replaced the UniquePtr<..> by RefPtr<..>, using a move can be
considered harmful if more than one reference exists. This patch changes usage
of CompilationStencil&& by RefPtr<CompilationStencil>&&, and check the
number of references before moving the content out of the CompilationStencil.
|
||
Updated•2 years ago
|
|
|
||
Updated•2 years ago
|
Pushed by npierron@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/07c96a9ed553 part 1 - Add assertion to avoid mixing UniquePtr with RefPtr. r=arai https://hg.mozilla.org/integration/autoland/rev/eab364304ebc part 2 - Use RefPtr<..> for CompilationStencil. r=arai https://hg.mozilla.org/integration/autoland/rev/bf75d7c17975 part 3 - Use RefPtr<..>&& for CompilationStencil. r=arai
|
||
Comment 5•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/07c96a9ed553
https://hg.mozilla.org/mozilla-central/rev/eab364304ebc
https://hg.mozilla.org/mozilla-central/rev/bf75d7c17975
Description
•