-
-
Notifications
You must be signed in to change notification settings - Fork 897
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
explore using AddressSanitizer in CI instead of (or in addition to) Valgrind #2067
Comments
I'm trying to find how process shutdown works in Ruby, but I wouldn't be surprised if some objects are just left to leak as we know the OS will clean the memory up. Lots of these look like they originate from what I'm guessing are regex literals. They live for the life of the byte code (IOW the entire life of the process). |
Update to above process for Ubuntu 24.04 and Ruby 3.4.0-dev:
And output is much better! This is closer to something that would be useful:
|
Context
Today we run the test suite under Valgrind (e.g., https://ci.nokogiri.org/teams/nokogiri-core/pipelines/nokogiri/jobs/cruby-2.7/builds/9#L5f0f8e55:1) to look for illegal memory addresses. Valgrind is great in that it takes pretty much any executable code and instruments it without additional compile- or link-time flags. Valgrind is slow, though, and I'd like faster feedback loops (and a second opinion?) if possible.
Idea
I'd like to explore using AddressSanitizer (via gcc's
-fsanitizer=address
) to find the same types of illegal memory usage (use-after-free, etc.), as it will be ~10x faster than Valgrind. But there are drawbacks:Also, we should verify that we can write and apply suppressions which have been valuable so far, see:
Somebody should spend some time to look into how we can quiet the noise and get more signal from it!
Initial Exploration
Here's what I did to get something working:
But the output looks like a lot of warnings like:
For Comparison
If someone other than me picks this up, you can compare this behavior to valgrind by running:
Pings
cc @tenderlove and @peterzhu2118 because we were talking about this.
The text was updated successfully, but these errors were encountered: