From 9e0ff773ad5840af78ef0deeebb7da2f503eca32 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 8 Dec 2015 07:23:37 -0800 Subject: [PATCH] std: Use mem::replace in TLS initialization Due to #30228 it's not currently sound to do `*ptr = Some(value)`, so instead use `mem::replace` which fixes the soundness hole for now. --- src/libstd/thread/local.rs | 17 ++++++++- src/test/run-pass/tls-init-on-init.rs | 51 +++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 src/test/run-pass/tls-init-on-init.rs diff --git a/src/libstd/thread/local.rs b/src/libstd/thread/local.rs index 119429cc584f7..870247f7e82fc 100644 --- a/src/libstd/thread/local.rs +++ b/src/libstd/thread/local.rs @@ -13,6 +13,7 @@ #![unstable(feature = "thread_local_internals", issue = "0")] use cell::UnsafeCell; +use mem; // Sure wish we had macro hygiene, no? #[doc(hidden)] @@ -226,7 +227,21 @@ impl LocalKey { // just in case initialization fails. let value = (self.init)(); let ptr = slot.get(); - *ptr = Some(value); + + // note that this can in theory just be `*ptr = Some(value)`, but due to + // the compiler will currently codegen that pattern with something like: + // + // ptr::drop_in_place(ptr) + // ptr::write(ptr, Some(value)) + // + // Due to this pattern it's possible for the destructor of the value in + // `ptr` (e.g. if this is being recursively initialized) to re-access + // TLS, in which case there will be a `&` and `&mut` pointer to the same + // value (an aliasing violation). To avoid setting the "I'm running a + // destructor" flag we just use `mem::replace` which should sequence the + // operations a little differently and make this safe to call. + mem::replace(&mut *ptr, Some(value)); + (*ptr).as_ref().unwrap() } diff --git a/src/test/run-pass/tls-init-on-init.rs b/src/test/run-pass/tls-init-on-init.rs new file mode 100644 index 0000000000000..195b814492af1 --- /dev/null +++ b/src/test/run-pass/tls-init-on-init.rs @@ -0,0 +1,51 @@ +// Copyright 2015 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(thread_local_state)] + +use std::thread::{self, LocalKeyState}; +use std::sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT}; + +struct Foo { cnt: usize } + +thread_local!(static FOO: Foo = Foo::init()); + +static CNT: AtomicUsize = ATOMIC_USIZE_INIT; + +impl Foo { + fn init() -> Foo { + let cnt = CNT.fetch_add(1, Ordering::SeqCst); + if cnt == 0 { + FOO.with(|_| {}); + } + Foo { cnt: cnt } + } +} + +impl Drop for Foo { + fn drop(&mut self) { + if self.cnt == 1 { + FOO.with(|foo| assert_eq!(foo.cnt, 0)); + } else { + assert_eq!(self.cnt, 0); + match FOO.state() { + LocalKeyState::Valid => panic!("should not be in valid state"), + LocalKeyState::Uninitialized | + LocalKeyState::Destroyed => {} + } + } + } +} + +fn main() { + thread::spawn(|| { + FOO.with(|_| {}); + }).join().unwrap(); +}