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

LinkedList implementation violates aliasing rules #34417

Closed
Amanieu opened this issue Jun 22, 2016 · 3 comments
Closed

LinkedList implementation violates aliasing rules #34417

Amanieu opened this issue Jun 22, 2016 · 3 comments
Labels
P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@Amanieu
Copy link
Member

Amanieu commented Jun 22, 2016

The nodes in LinkedList look like this:

struct Node<T> {
    next: Option<Box<Node<T>>>,
    prev: Option<Shared<Node<T>>>,
    value: T,
}

Box uses Unique, which requires that it "the referent of the pointer should not be modified without a unique path to the Unique reference". This effectively means that Unique doesn't allow aliases, but this rule violated by the prev pointer which point to the same node as a Unique.

While Rust currently doesn't give noalias semantics to Unique, this is planned in the future and will silently break LinkedList.

@alexcrichton alexcrichton added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 22, 2016
@eefriedman
Copy link
Contributor

The declaration of Box in liballoc is basically informational at the moment; we don't actually use it for anything. So actually the problem is worse than it looks: we already mark Box<T> pointers noalias (but only if you pass one directly to a function... so it's sort of a similar situation to Ref<T> where it would break if we used noalias markings more aggressively).

It's not completely clear whether this particular use of unsafe will be considered undefined behavior under the new memory model (see rust-lang/rfcs#1643), but it's likely it will.

@nikomatsakis
Copy link
Contributor

I agree with @eefriedman that this usage is likely to turn out to be a problem -- because it uses Box. It's not obvious to me whether Unique ought to be "meaningful" to the Rust compiler in any sense. In any case, I believe the reason that Unique has that wording about uniqueness is because it implements Send, which is not a problem here, so perhaps it is the wording that is wrong (it's one way to make things thread-safe, but not the only way).

@alexcrichton alexcrichton added P-medium Medium priority and removed I-nominated labels Jun 27, 2016
@alexcrichton
Copy link
Member

triage: P-medium

Not actively causing breakage today, but we're likely to want to revisit this soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-medium Medium priority T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants