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

Do not allow moving out of thread local under ast borrowck #55150

Conversation

pnkfelix
Copy link
Member

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix #47215
Fix #54797

…e not quite rvalues

(and of course they are not quite statics either).

Namely, they *do* have a restricted region (like rvalues), but they
also cannot be moved out of (like statics).
@rust-highfive
Copy link
Collaborator

r? @estebank

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2018
@pnkfelix
Copy link
Member Author

r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned estebank Oct 17, 2018
@pnkfelix
Copy link
Member Author

pnkfelix commented Oct 17, 2018

One might reasonably ask: "Why add new checks to AST-borrowck rather than focus on NLL."

I basically estimated the effort it would take to implementing reporting a "nice" error from drop-elaboration, and decided that this was a much better approach.

@pnkfelix pnkfelix added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 23, 2018
@pnkfelix
Copy link
Member Author

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned eddyb Oct 23, 2018
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Oct 23, 2018

📌 Commit 1d46ce5 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 23, 2018
…-from-moving-out-of-thread-local-under-ast-borrowck, r=nikomatsakis

Do not allow moving out of thread local under ast borrowck

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix rust-lang#47215
Fix rust-lang#54797
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Oct 25, 2018
…-from-moving-out-of-thread-local-under-ast-borrowck, r=nikomatsakis

Do not allow moving out of thread local under ast borrowck

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix rust-lang#47215
Fix rust-lang#54797
@bors
Copy link
Contributor

bors commented Oct 27, 2018

⌛ Testing commit 1d46ce5 with merge a5e6e159a64c676dc8a959db771d0ab5cc84f169...

@bors
Copy link
Contributor

bors commented Oct 27, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 27, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-distcheck of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
[02:29:34] warning: spurious network error (2 tries remaining): curl error: Could not resolve host: github.com
[02:29:34] ; class=Net (12)
[02:29:54] warning: spurious network error (1 tries remaining): curl error: Could not resolve host: github.com
[02:29:54] ; class=Net (12)
[02:30:15] error: failed to load source for a dependency on `rand`
[02:30:15] Caused by:
[02:30:15]   Unable to update registry `https://github.com/rust-lang/crates.io-index`
[02:30:15] 
[02:30:15] Caused by:
[02:30:15] Caused by:
[02:30:15]   failed to fetch `https://github.com/rust-lang/crates.io-index`
[02:30:15] 
[02:30:15] Caused by:
[02:30:15]   curl error: Could not resolve host: github.com
[02:30:15] ; class=Net (12)
[02:30:15] 
[02:30:15] 
[02:30:15] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "generate-lockfile" "--manifest-path" "/checkout/obj/build/tmp/distcheck-src/rust-src/lib/rustlib/src/rust/src/libstd/Cargo.toml"
[02:30:15] 
[02:30:15] 
[02:30:15] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test distcheck
[02:30:15] Build completed unsuccessfully in 2:27:11
---
travis_time:end:19180a7c:start=1540624280304249995,finish=1540624280314062198,duration=9812203
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04ad5080
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:031b17fa
travis_time:start:031b17fa
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:202dada1
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@pnkfelix
Copy link
Member Author

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 27, 2018
@bors
Copy link
Contributor

bors commented Oct 27, 2018

⌛ Testing commit 1d46ce5 with merge f32f111...

bors added a commit that referenced this pull request Oct 27, 2018
…g-out-of-thread-local-under-ast-borrowck, r=nikomatsakis

Do not allow moving out of thread local under ast borrowck

AST borrowck failed to prevent moving out of a thread-local static.

This was broken. And it also (sometimes?) caused an ICE during drop elaboration.

Fix #47215
Fix #54797
@bors
Copy link
Contributor

bors commented Oct 27, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing f32f111 to master...

@bors bors merged commit 1d46ce5 into rust-lang:master Oct 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant 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