Skip to content

Commit 44d5bed

Browse files
authored
Fix stack-use-after-return in Stack initialization (#868)
Problem Stack::default() was taking a raw pointer to self.sentinel and storing it in every StackEntry, then returning self by value. Because Rust moved the struct to the caller's location on return, all stored pointers were left pointing to the now-freed stack frame of default(). This would have caused silent data corruption or a crash whenever contcorrhist/conthist was dereferenced, most likely under thread contention(?). The bug was probably invisible in normal builds because the freed stack memory was rarely overwritten before being read, but caught it thanks to ASan. Solution Fixed by heap-allocating Stack via Box in a new Stack::new() constructor, making sure that the address of sentinel is stable before the pointer is stored. Bench: 2981881
1 parent d55e30d commit 44d5bed

File tree

4 files changed

+21
-20
lines changed

4 files changed

+21
-20
lines changed

src/search.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ pub fn start(td: &mut ThreadData, report: Report, thread_count: usize) {
129129
td.optimism[!td.board.side_to_move()] = -td.optimism[td.board.side_to_move()];
130130

131131
loop {
132-
td.stack = Stack::default();
132+
td.stack = Stack::new();
133133
td.root_delta = beta - alpha;
134134

135135
// Root Search

src/stack.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,9 @@ impl Stack {
1111
pub fn sentinel(&mut self) -> &mut StackEntry {
1212
unsafe { self.data.get_unchecked_mut(0) }
1313
}
14-
}
15-
16-
impl Default for Stack {
17-
fn default() -> Self {
18-
let mut stack = Self {
19-
data: [StackEntry::default(); MAX_PLY + 16],
20-
sentinel: [[0; 64]; 13],
21-
};
2214

15+
pub fn new() -> Box<Self> {
16+
let mut stack = Box::new(Self::default());
2317
let ptr = &raw mut stack.sentinel;
2418
for entry in &mut stack.data {
2519
entry.conthist = ptr;
@@ -29,6 +23,15 @@ impl Default for Stack {
2923
}
3024
}
3125

26+
impl Default for Stack {
27+
fn default() -> Self {
28+
Self {
29+
data: [StackEntry::default(); MAX_PLY + 16],
30+
sentinel: [[0; 64]; 13],
31+
}
32+
}
33+
}
34+
3235
#[derive(Copy, Clone)]
3336
pub struct StackEntry {
3437
pub mv: Move,

src/thread.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ pub struct ThreadData {
131131
pub shared: Arc<SharedContext>,
132132
pub board: Board,
133133
pub time_manager: TimeManager,
134-
pub stack: Stack,
134+
pub stack: Box<Stack>,
135135
pub nnue: Network,
136136
pub root_moves: Vec<RootMove>,
137137
pub pv_table: PrincipalVariationTable,
@@ -162,7 +162,7 @@ impl ThreadData {
162162
shared,
163163
board: Board::starting_position(),
164164
time_manager: TimeManager::new(Limits::Infinite, 0, 0),
165-
stack: Stack::default(),
165+
stack: Stack::new(),
166166
nnue: Network::default(),
167167
root_moves: Vec::new(),
168168
pv_table: PrincipalVariationTable::default(),

src/threadpool.rs

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::{
1717

1818
pub struct ThreadPool {
1919
pub workers: Vec<WorkerThread>,
20-
pub vector: Vec<Box<ThreadData>>,
20+
pub vector: Vec<ThreadData>,
2121
}
2222

2323
impl ThreadPool {
@@ -57,11 +57,11 @@ impl ThreadPool {
5757
self.vector.len()
5858
}
5959

60-
pub fn iter(&self) -> impl Iterator<Item = &Box<ThreadData>> {
60+
pub fn iter(&self) -> impl Iterator<Item = &ThreadData> {
6161
self.vector.iter()
6262
}
6363

64-
pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut Box<ThreadData>> {
64+
pub fn iter_mut(&mut self) -> impl Iterator<Item = &mut ThreadData> {
6565
self.vector.iter_mut()
6666
}
6767

@@ -260,10 +260,8 @@ fn make_worker_threads(num_threads: usize) -> Vec<WorkerThread> {
260260
}
261261
}
262262

263-
fn make_thread_data(
264-
shared: Arc<SharedContext>, worker_threads: &[WorkerThread], board: Arc<Board>,
265-
) -> Vec<Box<ThreadData>> {
266-
std::thread::scope(|scope| -> Vec<Box<ThreadData>> {
263+
fn make_thread_data(shared: Arc<SharedContext>, worker_threads: &[WorkerThread], board: Arc<Board>) -> Vec<ThreadData> {
264+
std::thread::scope(|scope| -> Vec<ThreadData> {
267265
let handles = worker_threads
268266
.iter()
269267
.map(|worker| {
@@ -282,10 +280,10 @@ fn make_thread_data(
282280
})
283281
.collect::<Vec<_>>();
284282

285-
let mut thread_data: Vec<Box<ThreadData>> = Vec::with_capacity(handles.len());
283+
let mut thread_data: Vec<ThreadData> = Vec::with_capacity(handles.len());
286284
for (rx, handle) in handles {
287285
let td = rx.recv().unwrap();
288-
thread_data.push(td);
286+
thread_data.push(*td);
289287
handle.join();
290288
}
291289

0 commit comments

Comments
 (0)