oxc
oxc copied to clipboard
`AstBuilder::copy` is unsound
https://github.com/oxc-project/oxc/blob/7f7b5ea9e83f37ff666d6f33fda7a9e27ea2aa07/crates/oxc_ast/src/ast_builder.rs#L71-L79
This method can be used correctly when a node is being removed from AST anyway, but there are multiple ways in which it can lead to unsound situations:
Aliasing violation:
fn do_bad_stuff(stmt: &Statement, ast: &AstBuilder) {
if let Statement::BlockStatement(block) = stmt {
let box1 = ast.copy(block);
let box2 = ast.copy(block);
// We now have two `Box<BlockStatement>` pointing to same `BlockStatement`
}
}
Mutation through immutable ref:
fn do_bad_stuff2(stmt: &Statement, ast: &AstBuilder) {
if let Statement::BlockStatement(block) = stmt {
let block = ast.copy(block);
// We can mutate, even though we only got an immutable ref
block.body.clear();
}
}
The double-free which #3481 fixes was likely caused by using copy
.
We could make the method unsafe
and caller will have to ensure it's only used when safe to do so. However, personally I think we should remove the API entirely because (1) it is difficult to reason about when it's safe to use and is therefore a footgun and (2) the performance hit from using safe alternatives is very minimal.
Unfortunately the alternatives are likely take
or mem::replace
which are not so ergonomic.