From 5225cd744a31f27cb927ca91c9c3dc899b8b3065 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Mon, 10 Aug 2026 04:16:01 +0900 Subject: [PATCH 1/2] Add Valgrind memcheck (ruby_memcheck) and run it in CI Running the suite under Valgrind on a Ruby C extension is normally impractical: the interpreter itself produces a large volume of reports that have nothing to do with the extension. ruby_memcheck wraps Valgrind and only surfaces errors whose stack trace passes through the extension's .so, which makes the output usable. It is the same tool Shopify runs on nokogiri and liquid-c. rake spec:valgrind This reworks #147, which was reverted in #150 because it broke CI: the Gemfile guarded the dependency on Linux but not on the Ruby version, and ruby_memcheck 3.x requires Ruby >= 3.0, so `bundle install` failed on the 2.7 entry of the matrix. The guard now covers both. Verified on Ruby 2.7.8 locally: bundle install, rake compile and rspec all pass, and the Rakefile simply does not define the task there. The memcheck run gets its own workflow rather than a job inside Ruby.yml, so a report from it cannot turn the main test matrix red. It pins one Ruby, installs Valgrind, and carries a 30 minute job timeout; a cold run -- extension build plus the suite under memcheck -- takes about 1m45s locally. The suite reports nothing on the current tree, so no suppression file is needed. The task does have teeth: run against the use-after-free of a stream's borrowed CDict/DDict it reports 113 Invalid read records, and against the ZSTD_DCtx leak it reports the context as definitely lost, both attributed inside zstdruby.so. Co-Authored-By: Claude Opus 5 --- .github/workflows/valgrind.yml | 39 ++++++++++++++++++++++++++++++++++ Gemfile | 4 ++++ Rakefile | 26 +++++++++++++++++++++++ 3 files changed, 69 insertions(+) create mode 100644 .github/workflows/valgrind.yml diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml new file mode 100644 index 0000000..6188c62 --- /dev/null +++ b/.github/workflows/valgrind.yml @@ -0,0 +1,39 @@ +name: Valgrind + +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + valgrind: + name: memcheck (ubuntu, ruby ${{ matrix.ruby }}) + runs-on: ubuntu-latest + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + ruby: + - '4.0' + + steps: + - uses: actions/checkout@v6 + - name: Install Valgrind + run: | + sudo apt-get update + sudo apt-get install -y valgrind + - name: Set up Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + - name: Run the spec suite under Valgrind memcheck + run: bundle exec rake spec:valgrind diff --git a/Gemfile b/Gemfile index afffcc2..07ae089 100644 --- a/Gemfile +++ b/Gemfile @@ -2,3 +2,7 @@ source 'https://rubygems.org' # Specify your gem's dependencies in zstd_ruby.gemspec gemspec + +if RUBY_PLATFORM.include?('linux') && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('3.0.0') + gem 'ruby_memcheck', '~> 3.0' +end diff --git a/Rakefile b/Rakefile index f76e1c2..6a89482 100644 --- a/Rakefile +++ b/Rakefile @@ -15,6 +15,32 @@ end task :default => [:clobber, :compile, :spec] +begin + require 'ruby_memcheck' + require 'ruby_memcheck/rspec/rake_task' + + RubyMemcheck.config( + binary_name: 'zstdruby', + # Valgrind and YJIT interfere with each other, adding noise and slowdown, + # so keep YJIT disabled while running under Valgrind. + ruby: "#{FileUtils::RUBY} --disable-yjit" + ) + + namespace :spec do + task :check_valgrind do + unless system('command -v valgrind > /dev/null 2>&1') + abort("\nValgrind is required for `rake spec:valgrind` but was not found.\n" \ + "Install it first (Linux only), e.g. `sudo apt-get install valgrind`.\n") + end + end + + RubyMemcheck::RSpec::RakeTask.new(valgrind: [:check_valgrind, :compile]) + end +rescue LoadError + # ruby_memcheck is an optional development dependency, absent on the platforms + # and Ruby versions the Gemfile excludes. Skip the task instead of breaking. +end + desc 'Sync zstd libs dirs to ext/zstdruby/libzstd' task :zstd_update do FileUtils.rm_r("ext/zstdruby/libzstd") From a131fcb01b1440d2c093b6c8c9d90b24b1c58172 Mon Sep 17 00:00:00 2001 From: Shizuo Fujita Date: Mon, 10 Aug 2026 04:26:26 +0900 Subject: [PATCH 2/2] Fix the large-bytes spec allocating 4 GiB Random.bytes(1<<17 + 15) parses as Random.bytes(1 << 32), because + binds tighter than <<, so the spec allocates 4 GiB instead of the 131,087 bytes it means -- just past the 128 KiB block boundary the neighbouring specs exercise. Adding the parentheses cuts the whole suite from about 8 seconds to 0.3, since that one example was nearly all of it. Under Valgrind it matters more: the suite goes from 1m18s to 1.4s, and on a CI runner the 4 GiB version cannot finish at all -- it stalls for minutes and the job is killed. Co-Authored-By: Claude Opus 5 --- spec/zstd-ruby_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/zstd-ruby_spec.rb b/spec/zstd-ruby_spec.rb index 6ef02d9..f5a373a 100644 --- a/spec/zstd-ruby_spec.rb +++ b/spec/zstd-ruby_spec.rb @@ -34,7 +34,7 @@ end it 'should compress large bytes' do - large_string = Random.bytes(1<<17 + 15) + large_string = Random.bytes((1<<17) + 15) compressed = Zstd.compress(large_string) expect(Zstd.decompress(compressed)).to eq(large_string) end