Skip to content

rustc_trans: don't apply noalias on returned references. - #46253

Merged
bors merged 1 commit into
rust-lang:masterfrom
eddyb:return-aliasing
Nov 26, 2017
Merged

rustc_trans: don't apply noalias on returned references.#46253
bors merged 1 commit into
rust-lang:masterfrom
eddyb:return-aliasing

Conversation

@eddyb

Copy link
Copy Markdown
Contributor

In #45225 frozen returned &T were accidentally maked noalias, unlike &mut T.
Return value noalias is only sound for functions that return dynamic allocations, e.g. Box, and using it on anything else can lead to miscompilation, as LLVM assumes certain usage patterns.
Fixes#46239.

@eddybeddyb added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 25, 2017
@rust-highfive

Copy link
Copy Markdown
Contributor

r? @michaelwoerister

(rust_highfive has picked a reviewer for you, use r? to override)

@nagisa

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Nov 25, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 5eed95e has been approved by nagisa

@kennytmkennytm added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 25, 2017
@kennytm

kennytm commented Nov 26, 2017

Copy link
Copy Markdown
Member

@bors p=1 — may be needed by #46270 as well.

@bors

bors commented Nov 26, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 5eed95e with merge 71b21ed...

bors added a commit that referenced this pull request Nov 26, 2017
rustc_trans: don't apply noalias on returned references.
In #45225 frozen returned `&T` were accidentally maked `noalias`, unlike `&mut T`.
Return value `noalias` is only sound for functions that return dynamic allocations, e.g. `Box`, and using it on anything else can lead to miscompilation, as LLVM assumes certain usage patterns.
Fixes#46239.
@bors

bors commented Nov 26, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: nagisa
Pushing 71b21ed to master...

@michaelwoerister

Copy link
Copy Markdown
Member

As per the IRC discussion with @eddyb on Friday, I'll tentatively mark this as beta-accepted. @rust-lang/compiler, please veto within the next 15 hours if you don't think this is a good idea. I'll do the backport tomorrow.

@michaelwoeristermichaelwoerister added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Dec 4, 2017
bors added a commit that referenced this pull request Dec 7, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beta-acceptedAccepted for backporting to the compiler in the beta channel.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@eddyb@rust-highfive@nagisa@bors@kennytm@michaelwoerister