Skip to content

Avoid exceptions in UCI spin option validation - #6738

Open
luckysolanki902 wants to merge 1 commit into
official-stockfish:masterfrom
luckysolanki902:fix-stoi-crash-ucioption
Open

Avoid exceptions in UCI spin option validation#6738
luckysolanki902 wants to merge 1 commit into
official-stockfish:masterfrom
luckysolanki902:fix-stoi-crash-ucioption

Conversation

@luckysolanki902

@luckysolanki902 luckysolanki902 commented Apr 17, 2026

Copy link
Copy Markdown

Summary

Malformed setoption input for a spin option is validated in Option::operator=, but using std::stoi for that check can throw std::invalid_argument or std::out_of_range. Because Stockfish is compiled with -fno-exceptions, malformed console input terminates the engine.

Examples on current master:

setoption name Hash value 999999999999999999  # -> std::out_of_range -> crash
setoption name Hash value notanumber          # -> std::invalid_argument -> crash

Confirmed live on stockfish-dev-20260415-b1fb50ae:

libc++abi: terminating due to uncaught exception of type std::out_of_range: stoi: out of range

Fix

Replace the throwing validation path in Option::operator= with std::strtol. This keeps validation non-throwing without depending on std::from_chars availability on older toolchains. Rejected values return early without mutating the option.

The new validation rejects:

  • non-numeric values (no conversion)
  • trailing garbage (partial conversion)
  • overflow / underflow
  • values outside [min, max]

Testing

Bench: 2984258

Malformed values are now rejected without terminating the engine.

@coderabbitai

coderabbitai Bot commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7513f6a5-e4f4-4e3a-9a23-1f27575fe09d

📥 Commits

Reviewing files that changed from the base of the PR and between 463afd4 and 74f97d0.

📒 Files selected for processing (1)
  • src/ucioption.cpp

📝 Walkthrough

Walkthrough

The Option::operator=(const std::string& v) implementation in src/ucioption.cpp was changed to handle spin-type validation explicitly. Instead of calling std::stoi inside the conditional, the code now parses v with std::strtol, verifies full consumption, checks errno != ERANGE, and enforces the [min, max] range. On parse or range failure the assignment returns early without updating currentValue or calling on_change. The <cerrno> header was added.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@luckysolanki902
luckysolanki902 force-pushed the fix-stoi-crash-ucioption branch from ef75adc to 463afd4 Compare April 17, 2026 01:28

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dd8e6d6f-3c47-4672-9afd-2e194f2e75bf

📥 Commits

Reviewing files that changed from the base of the PR and between ef75adc and 463afd4.

📒 Files selected for processing (1)
  • src/ucioption.cpp

Comment thread src/ucioption.cpp Outdated
@luckysolanki902
luckysolanki902 force-pushed the fix-stoi-crash-ucioption branch from 463afd4 to 26e9cb7 Compare April 17, 2026 01:37
@github-actions

github-actions Bot commented Apr 17, 2026

Copy link
Copy Markdown

clang-format 20 needs to be run on this PR.
If you do not have clang-format installed, the maintainer will run it when merging.
For the exact version please see https://packages.ubuntu.com/plucky/clang-format-20.

(execution 24610608033 / attempt 1)

@anematode

Copy link
Copy Markdown
Member

This one doesn't need a fishtest run (it doesn't affect the engine's play)

@Disservin

Copy link
Copy Markdown
Member

we avoided charconv because in the past workers on fishtest failed because it's missing

@vondele

vondele commented Apr 18, 2026

Copy link
Copy Markdown
Member

I recall this as well, but I do not longer recall which environment / compiler version was involved. For gcc, full support of charconv seems to be only available from 11+ and clang even a bit later. In CI our current set of supported compiler see to work fine, at least for the smoke test.

We're a bit on the same path as with the fens. In principle we rely on the gui to provide valid input.

@Disservin

Copy link
Copy Markdown
Member

iirc it was clang 13 from sebs machine and he had a somewhat special system where it was missing..

Malformed spin-option values are validated in Option::operator=, but
using std::stoi for that check can throw std::invalid_argument or
std::out_of_range. Because Stockfish is built with -fno-exceptions,
malformed setoption input terminates the engine.

Use std::strtol for spin-option validation instead. This keeps the
validation path non-throwing without depending on std::from_chars
support across older toolchains. Reject values when:
  - no conversion happens
  - trailing garbage remains
  - conversion overflows
  - result falls outside [min, max]

Bench: 2984258
@luckysolanki902
luckysolanki902 force-pushed the fix-stoi-crash-ucioption branch from 26e9cb7 to 74f97d0 Compare April 18, 2026 18:01
@luckysolanki902 luckysolanki902 changed the title Replace std::stoi with std::from_chars in UCI option parsing Avoid exceptions in UCI spin option validation Apr 18, 2026
@luckysolanki902

Copy link
Copy Markdown
Author

Addressed the charconv concern by switching the validation path from std::from_chars to std::strtol in the latest force-push. This keeps the fix non-throwing in -fno-exceptions builds without depending on charconv support on older toolchains. Bench is still unchanged at 2984258.

@Disservin

Copy link
Copy Markdown
Member

i'd also prefer if we simply clamp the value if it is outside the range

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants