DelSkayn / rquickjs

High level bindings to the quickjs javascript engine
MIT License
434 stars 59 forks source link

fix UB on TypedArray::as_typed_aray #244

Closed rbran closed 5 months ago

rbran commented 6 months ago

tldr: the code inside bool::then_some is always executed, but the code in the closure from bool::then is only executed if is true.

This is UB, because the bool::then_some is a function that takes a bool and a generic value, because of that, the value inside of it is always evaluated, independent if the bool is true/false.

it's clear if the desugarize the syntax, the code bellow is equivalent to the function as_typed_array:

// first we get the true/false for the condition
let cond: bool = self.is_typed_array::<T>();
// then we evaluate the expression to pass the value as parameter
// UB here, the is executed always, independent of the cond being true/false
let value: &TypedArray<T> = unsafe { self.ref_typed_array() };
// then we call the function `bool::then_some`.
bool::then_some(cond, value)

The solution is simply change it to bool::then, because the parameter it's a closure, it's lazy evaluated, and only executed if the condition is true.

As mentioned at https://github.com/rust-lang/rust/blob/5cb2e7dfc362662b0036faad3bab88d73027fd05/src/tools/clippy/clippy_lints/src/transmute/mod.rs#L468-L520

And presented at https://youtu.be/hBjQ3HqCfxs?si=nrIz6ZHnxEHO6Pik&t=53

DelSkayn commented 5 months ago

Good catch! Thanks for the PR!