Pleco icon indicating copy to clipboard operation
Pleco copied to clipboard

Unsound problems in `pleco_engine` crate

Open safe4u opened this issue 1 year ago • 1 comments
trafficstars

Hi, thanks for your contribution to this great engine.

However, we have found some unsound problems in the crate pleco_engine. For example, insert_score_depth and insert_score use get_unchecked_mut to access target RootMove without boundary checking for performance consideration. These functions are actually unsafe since they ask the caller to guarantee the index is in-bound and the compiler fails to check this.

It is certainly ok to mark the unsafe functions as safe and use them with care in 'pleco'. But considering the crate pleco_engine is available in crates.io and these functions are public, we think it's still necessary to make these functions more sound.

Suggestions

Here are some action choices we suggested:

  1. Mark these functions as 'unsafe' which would lead to large code changes.
  2. (recommended) Add assert! or at least debug_assert! macros to validate that the index is in-bound.

safe4u avatar Jul 07 '24 07:07 safe4u

Thanks for the feedback! I would lean towards option 2. Please feel free to raise a PR for this 😃

chase-manning avatar Jul 07 '24 11:07 chase-manning