[WIP] rustc_mir: disallow global mutable state in proc macros. by eddyb · Pull Request #63831 · rust-lang/rust

eddyb · 2019-08-23T14:10:33Z

Along the lines of #63809, this PR attempts to get rid of the main (or only?) place proc_macro handles could be leaked to, and further disallow/discourage sharing (other) state between invocations.

The approach of banning (interior-)mutable statics was most recently mentioned in #63804 (comment), but it's likely been brought up several times, we just never tried it.
(Note that this is not foolproof: one would have to scan all dependencies for such statics, modulo proc_macro/std, and even then it's possible there would be a lot of false positives)

So this is mostly for a check-only crater run, to see what (if anything) breaks.

rust-highfive · 2019-08-23T14:10:37Z

r? @petrochenkov

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

eddyb · 2019-08-23T14:10:41Z

@bors try

bors · 2019-08-23T14:10:53Z

⌛ Trying commit ee8492f with merge 8c9c32a...

[WIP] rustc_mir: disallow global mutable state in proc macros. Along the lines of #63809, this PR attempts to get rid of the main (or only?) place `proc_macro` handles could be leaked to, *and* further disallow/discourage sharing (other) state between invocations. The approach of banning (interior-)mutable `static`s was most recently mentioned in #63804 (comment), but it's likely been brought up several times, we just never tried it. (Note that this is not foolproof: one would have to scan all dependencies for such `static`s, modulo `proc_macro`/`std`, and even then it's possible there would be a lot of false positives) So this is mostly for a check-only crater run, to see what (if anything) breaks.

bors · 2019-08-23T16:52:22Z

☀️ Try build successful - checks-azure
Build commit: 8c9c32a

eddyb · 2019-08-23T17:13:50Z

@craterbot run mode=check-only

eddyb · 2019-08-27T12:34:31Z

@craterbot run mode=check-only

craterbot · 2019-08-27T12:34:34Z

👌 Experiment pr-63831 created and queued.
🤖 Automatically detected try build 8c9c32a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

emilyalbini · 2019-09-08T11:27:52Z

@craterbot crates=full

craterbot · 2019-09-08T11:27:54Z

📝 Configuration of the pr-63831 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

eddyb · 2019-09-12T12:06:35Z

@craterbot abort
(in favor of #64398)

craterbot · 2019-09-12T12:06:39Z

🗑️ Experiment pr-63831 deleted!

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

60: core: Remove usage of mutable global state r=taiki-e a=taiki-e It may be a warning in the future. So, use the hash value of the input AST instead of PRNG. See rust-lang/rust#64398 (comment) and rust-lang/rust#63831. Co-authored-by: Taiki Endo <te316e89@gmail.com>

petrochenkov · 2019-09-29T12:04:41Z

Crater completed in #64398, marking as waiting on author to decide what to do (or not to do) next.

ecstatic-morse · 2019-10-05T00:50:28Z

Regressed crates list is here.

ecstatic-morse · 2019-10-07T16:50:27Z

The new functionality around crate lists has been deployed @eddyb. The following invocation should work (I don't have craterbot permissions):

@craterbot check crates=https://gist.githubusercontent.com/ecstatic-morse/ca6fe943de6937db635143472358d90d/raw/177739189815b3c52a7f69b494dbb91ea2d25e1d/gistfile1.txt

craterbot · 2019-10-07T16:50:30Z

🔒 Error: you're not allowed to interact with this bot.

🔑 If you are a member of the Rust team and need access, add yourself to the whitelist.
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Mark-Simulacrum · 2019-10-07T16:56:00Z

@craterbot check crates=https://gist.githubusercontent.com/ecstatic-morse/ca6fe943de6937db635143472358d90d/raw/177739189815b3c52a7f69b494dbb91ea2d25e1d/gistfile1.txt

craterbot · 2019-10-07T16:56:02Z

👌 Experiment pr-63831 created and queued.
🤖 Automatically detected try build 8c9c32a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Mark-Simulacrum · 2019-10-07T16:56:41Z

@craterbot p=1 since this is such a small run

craterbot · 2019-10-07T16:56:43Z

🚨 Error: failed to parse the command

🆘 If you have any trouble with Crater please ping @rust-lang/infra!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

emilyalbini · 2019-10-07T16:57:14Z

@craterbot p=1

craterbot · 2019-10-07T16:57:15Z

📝 Configuration of the pr-63831 experiment changed.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2019-10-09T11:40:08Z

🚧 Experiment pr-63831 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

craterbot · 2019-10-10T12:39:31Z

🎉 Experiment pr-63831 is completed!
📊 627 regressed and 0 fixed (656 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

petrochenkov · 2019-10-10T17:51:52Z

Marking as waiting on author to triage the regressions.
(I'm not too interested in driving this myself.)

JohnCSimon · 2019-10-26T01:55:56Z

Ping from triage
@eddyb Can you please address the regressions in this PR?
cc: @petrochenkov

Thanks

eddyb · 2019-10-30T17:27:55Z

I can't think of a good way to categorize uses of interior mutability in statics (e.g. lazy_static! is fine, RNGs are not), and proc_macro handles can't be used across invocations anyway.

We probably can't do much here, and sandboxing (e.g. via WASM) will likely be opt-in.
Hopefully the situation isn't too bad if we ever try to put proc macro expansion through incremental.

rust-highfive assigned petrochenkov Aug 23, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 23, 2019

petrochenkov added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 23, 2019

rustc_mir: disallow global mutable state in proc macros.

55f3acf

eddyb force-pushed the proc-macro-statecheck branch from ee8492f to 55f3acf Compare September 12, 2019 07:22

eddyb mentioned this pull request Sep 12, 2019

[WIP] Crater rollup of 3 PRs (#63809, #63812, #63831). #64398

Closed

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Sep 12, 2019

petrochenkov added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 12, 2019

taiki-e mentioned this pull request Sep 20, 2019

core: Remove usage of mutable global state taiki-e/auto_enums#60

Merged

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 29, 2019

craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2019

craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Oct 10, 2019

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 10, 2019

This was referenced Oct 14, 2019

rustc_mir: double-check const-promotion candidates for sanity. #63812

Merged

[WIP] proc_macro: check non-interned handles for "leaks" between/after invocations. #63809

Closed

eddyb closed this Oct 30, 2019

Uh oh!

Conversation

eddyb commented Aug 23, 2019

Uh oh!

rust-highfive commented Aug 23, 2019

Uh oh!

eddyb commented Aug 23, 2019

Uh oh!

bors commented Aug 23, 2019

Uh oh!

bors commented Aug 23, 2019

Uh oh!

eddyb commented Aug 23, 2019

Uh oh!

eddyb commented Aug 27, 2019

Uh oh!

craterbot commented Aug 27, 2019

Uh oh!

emilyalbini commented Sep 8, 2019

Uh oh!

craterbot commented Sep 8, 2019

Uh oh!

eddyb commented Sep 12, 2019

Uh oh!

craterbot commented Sep 12, 2019

Uh oh!

petrochenkov commented Sep 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ecstatic-morse commented Oct 5, 2019

Uh oh!

ecstatic-morse commented Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

craterbot commented Oct 7, 2019

Uh oh!

Mark-Simulacrum commented Oct 7, 2019

Uh oh!

craterbot commented Oct 7, 2019

Uh oh!

Mark-Simulacrum commented Oct 7, 2019

Uh oh!

craterbot commented Oct 7, 2019

Uh oh!

emilyalbini commented Oct 7, 2019

Uh oh!

craterbot commented Oct 7, 2019

Uh oh!

craterbot commented Oct 9, 2019

Uh oh!

craterbot commented Oct 10, 2019

Uh oh!

petrochenkov commented Oct 10, 2019

Uh oh!

JohnCSimon commented Oct 26, 2019

Uh oh!

eddyb commented Oct 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

petrochenkov commented Sep 29, 2019 •

edited

Loading

ecstatic-morse commented Oct 7, 2019 •

edited

Loading