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

[NO-TICKET] Add more memory leak testing for profiling using asan #3864

Merged
merged 2 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .github/workflows/test-memory-leaks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,21 @@ jobs:
cache-version: v1 # bump this to invalidate cache
- run: sudo apt install -y valgrind && valgrind --version
- run: bundle exec rake compile spec:profiling:memcheck
test-asan:
runs-on: ubuntu-24.04
steps:
- uses: actions/checkout@v4
# We're using a fork of ruby/setup-ruby because the "asan" tool is built into the clang compiler toolchain, and
# needs Ruby to be built with a special configuration.
#
# The special configuration is not yet available in the upstream `ruby/setup-ruby` github action, so I needed to
# fork it and push a small tweak to make it available.
#
# (The Ruby builds were added in https://github.com/ruby/ruby-dev-builder/pull/10 ).
- uses: datadog/setup-ruby@0c7206d6db81faf999795ceebfac00d164298bd5
with:
ruby-version: asan
bundler-cache: true # runs 'bundle install' and caches installed gems automatically
bundler: latest
cache-version: v1 # bump this to invalidate cache
- run: env RUBY_FREE_AT_EXIT=1 LSAN_OPTIONS=verbosity=0:log_threads=1:suppressions=`pwd`/suppressions/lsan.supp ASAN_OPTIONS=detect_leaks=1 bundle exec rake spec:profiling:main
21 changes: 21 additions & 0 deletions ext/datadog_profiling_native_extension/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,27 @@ def skip_building_extension!(reason)
require "debase/ruby_core_source"
dir_config("ruby") # allow user to pass in non-standard core include directory

# This is a workaround for a weird issue...
#
# The mkmf tool defines a `with_cppflags` helper that debase-ruby_core_source uses. This helper temporarily
# replaces `$CPPFLAGS` (aka the C pre-processor [not c++!] flags) with a different set when doing something.
#
# The debase-ruby_core_source gem uses `with_cppflags` during makefile generation to inject extra headers into the
# path. But because `with_cppflags` replaces `$CPPFLAGS`, well, the default `$CPPFLAGS` are not included in the
# makefile.
#
# This is a problem because the default `$CPPFLAGS` carries configuration that was set when Ruby was being built.
# Thus, if we ignore it, we don't compile the profiler with the exact same configuration as Ruby.
# In practice, this can generate crashes and weird bugs if the Ruby configuration is tweaked in a manner that
# changes some of the internal structures that the profiler relies on. Concretely, setting for instance
# `VM_CHECK_MODE=1` when building Ruby will trigger this issue (because somethings in structures the profiler reads
# are ifdef'd out using this setting).
#
# To workaround this issue, we override `with_cppflags` for debase-ruby_core_source to still include `$CPPFLAGS`.
Debase::RubyCoreSource.define_singleton_method(:with_cppflags) do |newflags, &block|
super("#{newflags} #{$CPPFLAGS}", &block)
end

Debase::RubyCoreSource
.create_makefile_with_core(
proc do
Expand Down
9 changes: 9 additions & 0 deletions suppressions/lsan.supp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# This is a Leak Sanitizer ("lsan") suppression configuration file.
#
# We use it together with special builds for Ruby
# (https://github.com/ruby/ruby/blob/master/doc/contributing/building_ruby.md#building-with-address-sanitizer)
# to find issues and memory leaks in the dd-trace-rb native extensions; in some cases
# we need to ignore potential issues as they're not something we can fix (e.g. outside our code.)
#
# See https://github.com/google/sanitizers/wiki/AddressSanitizerLeakSanitizer for details.
leak:native_thread_create
Loading