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

Add some details about using asan build. #654

Merged
merged 2 commits into from
Oct 11, 2024
Merged

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Oct 6, 2024

We could add links to examples and Ruby's documentation but I thought let's keep it short to start with.

@dentarg
Copy link

dentarg commented Oct 10, 2024

The asan build will always be built from head, right? Something to mention in the README?

README.md Outdated
ruby-version: asan
```

You should also compile your own code with ASan to get the full benefits of this feature.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might be automatic for C extensions with that build, @KJTsanaktsidis ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be automatic for extensions yes. If you’re linking in some other library maybe you want to compile that with ASAN too but that’s probably niche enough to to mention here?

README.md Outdated
@@ -220,6 +220,18 @@ It is also possible to cache gems manually, but this is not recommended because
There are many concerns which means using `actions/cache` is never enough for caching gems (e.g., incomplete cache key, cleaning old gems when restoring from another key, correctly hashing the lockfile if not checked in, OS versions, ABI compatibility for `ruby-head`, etc).
So, please use `bundler-cache: true` instead and report any issue.

### ASan Debugging

Ruby can be built with AddressSanitizer (ASan) for debugging memory issues. You can use the `asan` variant of Ruby for this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move it around lines 23? That's where we explain other similar variants.
There is no need for a YAML snippet, it should be obvious.
It's also ubuntu-24.04 only, we should mention that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmh, maybe we need its own section to document env vars and stuff, see #653 (comment)
But anyway I think we should mention asan near line 23 and that it's ubuntu-24.04 only

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, think it needs it's own section, but we could also add a note on line 23.

@eregon eregon mentioned this pull request Oct 10, 2024
README.md Outdated
ruby-version: asan
```

You should also compile your own code with ASan to get the full benefits of this feature.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be automatic for extensions yes. If you’re linking in some other library maybe you want to compile that with ASAN too but that’s probably niche enough to to mention here?

* Remove the ASan Debugging section for now as it seems mostly redundant.
  Native extensions are automatically compiled with ASan, so there is nothing special to do to use it.
@eregon eregon merged commit 1d9686e into ruby:master Oct 11, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants