Add valgrind_generate_suppressions option to configuration

valgrind_generate_suppressions will generate suppressions in the output.
Shopify · Nov 1, 2021 · 8af14c8 · 8af14c8
1 parent 396aaf3
commit 8af14c8
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 14 deletions.
diff --git a/lib/ruby_memcheck/configuration.rb b/lib/ruby_memcheck/configuration.rb
@@ -25,25 +25,27 @@ class Configuration
       /\Arb_yield/,
     ].freeze
 
-    attr_reader :binary_name, :ruby, :valgrind_options, :valgrind,
-      :skipped_ruby_functions, :valgrind_xml_dir, :output_io
+    attr_reader :binary_name, :ruby, :valgrind, :valgrind_options, :valgrind_suppressions_dir,
+      :valgrind_generate_suppressions, :skipped_ruby_functions, :valgrind_xml_dir, :output_io
+    alias_method :valgrind_generate_suppressions?, :valgrind_generate_suppressions
 
     def initialize(
       binary_name:,
       ruby: FileUtils::RUBY,
       valgrind: DEFAULT_VALGRIND,
       valgrind_options: DEFAULT_VALGRIND_OPTIONS,
       valgrind_suppressions_dir: DEFAULT_VALGRIND_SUPPRESSIONS_DIR,
+      valgrind_generate_suppressions: false,
       skipped_ruby_functions: DEFAULT_SKIPPED_RUBY_FUNCTIONS,
       valgrind_xml_dir: Dir.mktmpdir,
       output_io: $stderr
     )
       @binary_name = binary_name
       @ruby = ruby
       @valgrind = valgrind
-      @valgrind_options =
-        valgrind_options +
-        get_valgrind_suppression_files(valgrind_suppressions_dir).map { |f| "--suppressions=#{f}" }
+      @valgrind_options = valgrind_options
+      @valgrind_suppressions_dir = valgrind_suppressions_dir
+      @valgrind_generate_suppressions = valgrind_generate_suppressions
       @skipped_ruby_functions = skipped_ruby_functions
       @output_io = output_io
 
@@ -62,7 +64,14 @@ def initialize(
     end
 
     def command(*args)
-      "#{valgrind} #{valgrind_options.join(" ")} #{ruby} #{args.join(" ")}"
+      [
+        valgrind,
+        valgrind_options,
+        get_valgrind_suppression_files(valgrind_suppressions_dir).map { |f| "--suppressions=#{f}" },
+        valgrind_generate_suppressions ? "--gen-suppressions=all" : "",
+        ruby,
+        args,
+      ].flatten.join(" ")
     end
 
     def skip_stack?(stack)

diff --git a/lib/ruby_memcheck/suppression.rb b/lib/ruby_memcheck/suppression.rb
@@ -9,8 +9,6 @@ def initialize(configuration, suppression_node)
     end
 
     def to_s
-      return "" if root.nil?
-
       str = StringIO.new
       str << "{\n"
       str << "  #{root.at_xpath("sname").content}\n"

diff --git a/lib/ruby_memcheck/valgrind_error.rb b/lib/ruby_memcheck/valgrind_error.rb
@@ -2,6 +2,9 @@
 
 module RubyMemcheck
   class ValgrindError
+    SUPPRESSION_NOT_CONFIGURED_ERROR_MSG =
+      "Please enable suppressions by configuring with valgrind_generate_suppressions set to true"
+
     attr_reader :kind, :msg, :stack, :suppression
 
     def initialize(configuration, error)
@@ -14,7 +17,13 @@ def initialize(configuration, error)
         end
       @stack = Stack.new(configuration, error.at_xpath("stack"))
       @configuration = configuration
-      @suppression = Suppression.new(configuration, error.at_xpath("suppression"))
+
+      suppression_node = error.at_xpath("suppression")
+      if configuration.valgrind_generate_suppressions?
+        @suppression = Suppression.new(configuration, suppression_node)
+      elsif suppression_node
+        raise SUPPRESSION_NOT_CONFIGURED_ERROR_MSG
+      end
     end
 
     def skip?
@@ -35,7 +44,7 @@ def to_s
           "  #{frame}\n"
         end
       end
-      str << suppression.to_s
+      str << suppression.to_s if suppression
       str.string
     end
 

diff --git a/test/ruby_memcheck/ruby_memcheck_suppression_test.rb b/test/ruby_memcheck/ruby_memcheck_suppression_test.rb
@@ -9,10 +9,6 @@ def setup
       @configuration = Configuration.new(binary_name: "ruby_memcheck_c_test")
     end
 
-    def test_given_nil
-      assert_equal("", RubyMemcheck::Suppression.new(@configuration, nil).to_s)
-    end
-
     def test_given_a_suppression_node
       suppression = ::Nokogiri::XML(<<~EOF).at_xpath("//suppression")
         <foo>

diff --git a/test/ruby_memcheck/valgrind_error_test.rb b/test/ruby_memcheck/valgrind_error_test.rb
@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "nokogiri"
+
+module RubyMemcheck
+  class ValgrindErrorTest < Minitest::Test
+    def setup
+      @configuration = Configuration.new(binary_name: "ruby_memcheck_c_test")
+    end
+
+    def test_raises_when_suppressions_generated_but_not_configured
+      output = ::Nokogiri::XML(<<~XML).at_xpath("//error")
+        <error>
+          <unique>0x1ab8</unique>
+          <tid>1</tid>
+          <kind>Leak_DefinitelyLost</kind>
+          <xwhat>
+            <text>48 bytes in 1 blocks are definitely lost in loss record 6,841 of 11,850</text>
+            <leakedbytes>48</leakedbytes>
+            <leakedblocks>1</leakedblocks>
+          </xwhat>
+
+          <stack>
+          </stack>
+
+          <suppression>
+          </suppression>
+        </foo>
+      XML
+
+      error = assert_raises do
+        RubyMemcheck::ValgrindError.new(@configuration, output)
+      end
+      assert_equal(ValgrindError::SUPPRESSION_NOT_CONFIGURED_ERROR_MSG, error.message)
+    end
+  end
+end