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

[crater experiment] Enable LLVM assertions #101591

Closed
wants to merge 1 commit into from

Conversation

saethlin
Copy link
Member

@saethlin saethlin commented Sep 8, 2022

r? @ghost

Enabling LLVM assertions on Linux so that we can do a crater run to look for issues similar to #101585

@rustbot rustbot added the T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. label Sep 8, 2022
@saethlin
Copy link
Member Author

saethlin commented Sep 8, 2022

@bors try

@bors
Copy link
Contributor

bors commented Sep 8, 2022

@saethlin: 🔑 Insufficient privileges: not in try users

@tmiasko
Copy link
Contributor

tmiasko commented Sep 8, 2022

@bors try

@bors
Copy link
Contributor

bors commented Sep 8, 2022

⌛ Trying commit 1e7d884 with merge 434cce50d8e9a9de0ddaaccaca51e3e3f4bbb275...

@matthiaskrgr
Copy link
Member

matthiaskrgr commented Sep 8, 2022

since we are doing this, should we also enable rustc debug assertions at the same time? :D

@saethlin
Copy link
Member Author

saethlin commented Sep 8, 2022

@matthiaskrgr If you're volunteering to help look through what comes out, then yes. Otherwise I have little confidence in my ability to understand the Rust ICEs. They all look the same to me.

@matthiaskrgr
Copy link
Member

finding interesting crater results can be fairly automatized; I made this script some time ago:

#!/bin/bash

EXP=$1

mkdir -p crater/$EXP
cd crater/$EXP
wget -c "https://crater-reports.s3.amazonaws.com/${EXP}/logs-archives/all.tar.gz"
tar -xzf all.tar.gz
echo "interesting findings:"
rg "LLVM ERROR"
rg "delay_span_bug"
rg "query stack during panic"
rg "internal compiler error"
#rg "RUST_BACKTRACE="

You just run main.sh pr-10147 and it will download and extract the crater logs (note: this easily by 5-10 gb when extracted 😅 ) and search for anything that looks like a crash and print it to the terminal.

( should probably add something like "Assertion `" for llvm assertions...)

@saethlin
Copy link
Member Author

saethlin commented Sep 8, 2022

Finding the interesting logs has never been my issue, it's understanding what they mean.

For now I think we should just see how many regressions we get with LLVM assertions. If it's just a few (which I think is unlikely) we can then look for additional regressions with the Rust debug assertions. It seems to me like the crater queue isn't very busy.

@matthiaskrgr matthiaskrgr changed the title Enable LLVM assertions [crater experiment] Enable LLVM assertions Sep 8, 2022
@matthiaskrgr
Copy link
Member

Hmm 😅 well usually I just check the bugtracker and if I don't see something that looks similar i just open an issue 🙃

@bors
Copy link
Contributor

bors commented Sep 8, 2022

☀️ Try build successful - checks-actions
Build commit: 434cce50d8e9a9de0ddaaccaca51e3e3f4bbb275 (434cce50d8e9a9de0ddaaccaca51e3e3f4bbb275)

@est31
Copy link
Member

est31 commented Sep 9, 2022

FTR they were originally enabled by default, but then they were disabled by default in #45810, leaving "alt" builds for users who want to test builds with LLVM assertions enabled. Later alt builds were removed entirely however. Then, #75199 has reenabled assertions again for some CI builders (but not binaries that are shipped from my understanding).

@tmiasko
Copy link
Contributor

tmiasko commented Sep 9, 2022

@craterbot run mode=build-only

@craterbot
Copy link
Collaborator

👌 Experiment pr-101591 created and queued.
🤖 Automatically detected try build 434cce50d8e9a9de0ddaaccaca51e3e3f4bbb275
🔍 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

@craterbot craterbot added the S-waiting-on-crater Status: Waiting on a crater run to be completed. label Sep 9, 2022
@craterbot
Copy link
Collaborator

🚧 Experiment pr-101591 is now running

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

@craterbot
Copy link
Collaborator

🎉 Experiment pr-101591 is completed!
📊 60 regressed and 8 fixed (243080 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

@craterbot 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 11, 2022
@bjorn3
Copy link
Member

bjorn3 commented Sep 11, 2022

At least one assertion failure:

@saethlin
Copy link
Member Author

saethlin commented Sep 11, 2022

This same assertion is hit in 21 crates, but crater only detects 2 of those cases as regressions, because the other 19 crates always die in crater due to what looks like OOM. I'm guessing that crater can't tell the difference between a SIGKILL due to OOM and a SIGKILL due to an LLVM assertion.

In any case, that looks like the same assertion detecting the same bug which prompted me to open this PR. I see a fix for it has been merged so I'll manually see if those crates are fixed by it and report back.

@saethlin
Copy link
Member Author

I've checked a few of the crates that have failed, and none hit the assertion anymore. I'm calling this closed.

@saethlin saethlin closed this Sep 15, 2022
@saethlin saethlin deleted the llvm-assertions branch September 21, 2022 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants