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

save-analysis: Fix ICE when processing associated constant #60649

Merged
merged 3 commits into from
May 13, 2019

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented May 8, 2019

Closes #59134
Closes rust-lang/rls#1449

Thanks @swgillespie for helping tracking this down and fixing it!

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2019
@Xanewok
Copy link
Member Author

Xanewok commented May 8, 2019

@rust-lang/infra this fixes a widespread ICE (any crate that transitively depends on bitflags 1.0.5, which is quite a few, currently yanked for this reason) in the RLS - any chance this can be backported to stable or at least to beta?

cc @KodrAus

@KodrAus
Copy link
Contributor

KodrAus commented May 8, 2019

A back port to beta sounds good to me, I’m also happy to just sit on our current master branch in bitflags for a while until it is available on the latest stable.

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels May 8, 2019
@Centril
Copy link
Contributor

Centril commented May 8, 2019

cc @rust-lang/compiler @rust-lang/release

Beta & stable nominating based on #60649 (comment).

@pietroalbini
Copy link
Member

Note that a point release is likely not going to happen. We're two weeks from the next release and since this isn't merged yet the most likely target is May 16th, one week before 1.35 is out.

@Xanewok
Copy link
Member Author

Xanewok commented May 9, 2019

@pietroalbini ah, my release math was way off, sorry. Assuming this will be merged soon, is beta backport still on the table?

@pietroalbini
Copy link
Member

Yep, the deadline for beta backports to be merged is Sunday 19th.

@Xanewok Xanewok force-pushed the save-analysis-assoc-const-ice branch from bf36208 to e73ba21 Compare May 9, 2019 12:12
@Xanewok
Copy link
Member Author

Xanewok commented May 9, 2019

Added a test for both ICEs (when processing const with associated type and when processing assoc const with bogus field access).
I tried to apply the patch from #60649 (comment) but it didn't fix the ICEs so didn't include it here.

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 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.
travis_time:end:080c4090:start=1557404023920840570,finish=1557404110925580129,duration=87004739559
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:03:48] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:49] tidy error: /checkout/src/test/ui/save-analysis/issue-59134-1.rs: missing trailing newline
[00:03:49] tidy error: /checkout/src/test/ui/save-analysis/issue-59134-0.rs: missing trailing newline
[00:03:54] some tidy checks failed
[00:03:54] 
[00:03:54] 
[00:03:54] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:54] 
[00:03:54] 
[00:03:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:54] Build completed unsuccessfully in 0:01:05
[00:03:54] Build completed unsuccessfully in 0:01:05
[00:03:54] Makefile:67: recipe for target 'tidy' failed
[00:03:54] make: *** [tidy] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:089ef8cc
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Thu May  9 12:19:14 UTC 2019
---
travis_time:end:3cb30eb4:start=1557404355299172114,finish=1557404355304092990,duration=4920876
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:06c56d30
$ 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:30a0bdf8
travis_time:start:30a0bdf8
$ 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:0194b6e0
$ 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)

@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label May 9, 2019
@nikomatsakis
Copy link
Contributor

Discussed in compiler meeting. Marking as beta-accepted.

@nikomatsakis
Copy link
Contributor

Seems like a point release won't happen, so we didn't discuss that.

@Xanewok Xanewok removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label May 9, 2019
@Xanewok
Copy link
Member Author

Xanewok commented May 9, 2019

I'll drop the stable-nominated label then

@mouse07410
Copy link

mouse07410 commented May 12, 2019

Pardon a naive question, but when can we hope to see this fix added to the stable...?

@tesuji
Copy link
Contributor

tesuji commented May 12, 2019

@mouse07410 Probably next stable, 1.35.

@Xanewok
Copy link
Member Author

Xanewok commented May 12, 2019

@oli-obk what do you think?

EDIT: Feel like r+'ing so it has a chance to be backported in beta as well?

@oli-obk
Copy link
Contributor

oli-obk commented May 13, 2019

@bors r+

r? @oli-obk

@bors
Copy link
Contributor

bors commented May 13, 2019

📌 Commit 0af18ee has been approved by oli-obk

@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 May 13, 2019
@bors
Copy link
Contributor

bors commented May 13, 2019

⌛ Testing commit 0af18ee with merge a9ec99f...

bors added a commit that referenced this pull request May 13, 2019
save-analysis: Fix ICE when processing associated constant

Closes #59134
Closes rust-lang/rls#1449

Thanks @swgillespie for helping tracking this down and fixing it!

r? @eddyb
@bors
Copy link
Contributor

bors commented May 13, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: oli-obk
Pushing a9ec99f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 13, 2019
@bors bors merged commit 0af18ee into rust-lang:master May 13, 2019
@Xanewok Xanewok deleted the save-analysis-assoc-const-ice branch May 13, 2019 15:09
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. 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