Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What are the semantics of '#[thread_local] static ...' without 'mut'? #47053

Closed
EdSchouten opened this issue Dec 28, 2017 · 2 comments
Closed
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@EdSchouten
Copy link
Contributor

EdSchouten commented Dec 28, 2017

Background:

I am currently working on porting Rust's libstd over to the CloudABI sandboxed runtime environment. I'm pretty close to getting it working and hope to send out a pull request in a couple of days.

As CloudABI's userspace <-> kernel ABI is documented and we generate Rust bindings automatically, it's really attractive to implement libstd on top of kernel primitives as much as possible. libstd's locking primitives (mutexes, etc.) are simpler than the POSIX ones (no shared address space support, no configurable clocks, etc). This allows us to shrink condvars, mutexes and rwlocks to four bytes each.

The rwlocks implementation I wrote needs to keep track of the number of read locks acquired by the current thread as follows:

#[thread_local]
static mut RDLOCKS_ACQUIRED: u32 = 0;

The reason I'm filing this bug report:

An earlier version of my code had this:

#[thread_local]
static RDLOCKS_ACQUIRED: u32 = 0;

This seemed to make the code build, but for some reason, this caused some weird behaviour, where this counter was not always incremented or decremented, causing assertions in my code to fail.

Is #[thread_local] static without mut supposed to do anything meaningful in the first place? If not, should we add a compiler warning/error for this?

@Amanieu
Copy link
Member

Amanieu commented Jan 5, 2018

I use atomic thread-local variables in my code when I need to communicate between a thread and a signal handler running on that thread.

@EdSchouten
Copy link
Contributor Author

EdSchouten commented Jan 14, 2018

I suspect that this is an issue with this code:

// `#[thread_local]` statics may not outlive the current function.
for attr in &self.tcx.get_attrs(def_id)[..] {
if attr.check_name("thread_local") {
return Ok(self.cat_rvalue_node(id, span, expr_ty));
}
}

By calling into cat_rvalue_node(), we ignore the mutbl argument. That function always usesMcDeclared, even though McImmutable should be used instead.

@kennytm kennytm added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 14, 2018
@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 22, 2018
kennytm added a commit to kennytm/rust that referenced this issue Jan 23, 2018
…sakis

Properly pass down immutability info for thread-locals.

For thread-locals we call into cat_rvalue_node() to create a CMT
(Category, Mutability, Type) that always has McDeclared. This is
incorrect for thread-locals that don't have the 'mut' keyword; we should
use McImmutable there.

Extend cat_rvalue_node() to have an additional mutability parameter. Fix
up all the callers to make use of that function. Also extend one of the
existing unit tests to cover this.

Fixes: rust-lang#47053
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants