search

pleco-rs / Pleco

A Rust-based re-write of the Stockfish Chess Engine
https://crates.io/crates/pleco
GNU General Public License v3.0
362 stars 39 forks source link

Unsound problems in `pleco_engine` crate #158

Open safe4u opened 1 week ago

safe4u commented 1 week ago

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.
chase-manning commented 1 week ago

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