diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e023ea3..743ce3c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,7 +7,6 @@ jobs: fail-fast: false matrix: entry: - - { ruby: '2.7', allowed-failure: false } - { ruby: '3.0', allowed-failure: false } - { ruby: '3.1', allowed-failure: false } - { ruby: '3.2', allowed-failure: false } diff --git a/README.md b/README.md index f832fd4..364a67b 100644 --- a/README.md +++ b/README.md @@ -84,12 +84,6 @@ The easiest way to use this gem is to use it on your test suite (minitest or RSp require "ruby_memcheck/rspec/rake_task" ``` -1. Configure the gem by calling `RubyMemcheck.config`. You must pass it your binary name. This is the same value you passed into `create_makefile` in your `extconf.rb` file. Make sure this value is correct or it will filter out almost everything as a false-positive! - - ```ruby - RubyMemcheck.config(binary_name: "your_binary_name") - ``` - 1. Setup the test task for your test framework. - **minitest** @@ -145,18 +139,17 @@ The easiest way to use this gem is to use it on your test suite (minitest or RSp ## Configuration -When you run `RubyMemcheck.config`, you are creating a default `RubyMemcheck::Configuration`. By default, the Rake tasks for minitest and RSpec will use this configuration. You can also manually pass in a `Configuration` object as the first argument to the constructor of `RubyMemcheck::TestTask` or `RubyMemcheck::RSpec::RakeTask` to use a different `Configuration` object rather than the default one. +If you want to override any of the default configurations you can call `RubyMemcheck.config` after `require "ruby_memcheck"`. This will create a default `RubyMemcheck::Configuration`. By default, the Rake tasks for minitest and RSpec will use this configuration. You can also manually pass in a `Configuration` object as the first argument to the constructor of `RubyMemcheck::TestTask` or `RubyMemcheck::RSpec::RakeTask` to use a different `Configuration` object rather than the default one. `RubyMemcheck::Configuration` accepts a variety of keyword arguments. Here are all the arguments: -- `binary_name`: Required. The binary name of your native extension gem. This is the same value you passed into `create_makefile` in your `extconf.rb` file. - `ruby`: Optional. The command to run to invoke Ruby. Defaults to the Ruby that is currently being used. - `valgrind`: Optional. The command to run to invoke Valgrind. Defaults to the string `"valgrind"`. - `valgrind_options`: Optional. Array of options to pass into Valgrind. This is only present as an escape hatch, so avoid using it. This may be deprecated or removed in future versions. - `valgrind_suppressions_dir`: Optional. The string path of the directory that stores suppression files for Valgrind. See the [`Suppression files`](#suppression-files) section for more details. Defaults to `suppressions`. - `valgrind_generate_suppressions`: Optional. Whether suppressions should also be outputted along with the errors. the [`Suppression files`](#suppression-files) section for more details. Defaults to `false`. - `skipped_ruby_functions`: Optional. Ruby functions that are ignored because they are considered a call back into Ruby. This is only present as an escape hatch, so avoid using it. If you find another Ruby function that is a false positive because it calls back into Ruby, please send a patch into this repo. Otherwise, use a Valgrind suppression file. -- `valgrind_xml_dir`: Optional. The directory to store temporary XML files for Valgrind. It defaults to a temporary directory. This is present for development debugging, so you shouldn't have to use it. +- `temp_dir`: Optional. The directory to store temporary files. It defaults to a temporary directory. This is present for development debugging, so you shouldn't have to use it. - `output_io`: Optional. The `IO` object to output Valgrind errors to. Defaults to standard error. - `filter_all_errors`: Optional. Whether to filter all kinds of Valgrind errors (not just memory leaks). This feature should only be used if you're encountering a large number of illegal memory accesses coming from Ruby. If you need to use this feature, you may have found a bug inside of Ruby. Consider reporting it to the [Ruby bug tracker](https://bugs.ruby-lang.org/projects/ruby-master/issues/new). Defaults to `false`. diff --git a/Rakefile b/Rakefile index 00e8ce4..45e7bee 100644 --- a/Rakefile +++ b/Rakefile @@ -11,9 +11,16 @@ Rake::TestTask.new(test: "test:compile") do |t| end namespace :test do - Rake::ExtensionTask.new("ruby_memcheck_c_test") do |ext| + Rake::ExtensionTask.new("ruby_memcheck_c_test_one") do |ext| ext.ext_dir = "test/ruby_memcheck/ext" ext.lib_dir = "test/ruby_memcheck/ext" + ext.config_script = "extconf_one.rb" + end + + Rake::ExtensionTask.new("ruby_memcheck_c_test_two") do |ext| + ext.ext_dir = "test/ruby_memcheck/ext" + ext.lib_dir = "test/ruby_memcheck/ext" + ext.config_script = "extconf_two.rb" end end diff --git a/lib/ruby_memcheck.rb b/lib/ruby_memcheck.rb index 58a2310..35286f0 100644 --- a/lib/ruby_memcheck.rb +++ b/lib/ruby_memcheck.rb @@ -19,11 +19,7 @@ def config(**opts) end def default_configuration - unless @default_configuration - raise "RubyMemcheck is not configured with a default configuration. "\ - "Please run RubyMemcheck.config before using it." - end - @default_configuration + @default_configuration ||= config end end end diff --git a/lib/ruby_memcheck/configuration.rb b/lib/ruby_memcheck/configuration.rb index 0529e4a..0e31ac1 100644 --- a/lib/ruby_memcheck/configuration.rb +++ b/lib/ruby_memcheck/configuration.rb @@ -31,14 +31,14 @@ class Configuration /\Arb_yield/, ].freeze - attr_reader :binary_name attr_reader :ruby attr_reader :valgrind attr_reader :valgrind_options attr_reader :valgrind_suppression_files attr_reader :valgrind_generate_suppressions attr_reader :skipped_ruby_functions - attr_reader :valgrind_xml_dir + attr_reader :temp_dir + attr_reader :loaded_features_file attr_reader :output_io attr_reader :filter_all_errors @@ -46,18 +46,19 @@ class Configuration alias_method :filter_all_errors?, :filter_all_errors def initialize( - binary_name:, + binary_name: nil, 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, + temp_dir: Dir.mktmpdir, output_io: $stderr, filter_all_errors: false ) - @binary_name = binary_name + warn("ruby_memcheck: binary_name is no longer required for configuration") if binary_name + @ruby = ruby @valgrind = valgrind @valgrind_options = valgrind_options @@ -69,18 +70,18 @@ def initialize( @output_io = output_io @filter_all_errors = filter_all_errors - if valgrind_xml_dir - valgrind_xml_dir = File.expand_path(valgrind_xml_dir) - FileUtils.mkdir_p(valgrind_xml_dir) - @valgrind_xml_dir = valgrind_xml_dir - @valgrind_options += [ - "--xml=yes", - # %p will be replaced with the PID - # This prevents forking and shelling out from generating a corrupted XML - # See --log-file from https://valgrind.org/docs/manual/manual-core.html - "--xml-file=#{File.join(valgrind_xml_dir, "%p.out")}", - ] - end + temp_dir = File.expand_path(temp_dir) + FileUtils.mkdir_p(temp_dir) + @temp_dir = temp_dir + @valgrind_options += [ + "--xml=yes", + # %p will be replaced with the PID + # This prevents forking and shelling out from generating a corrupted XML + # See --log-file from https://valgrind.org/docs/manual/manual-core.html + "--xml-file=#{File.join(temp_dir, "%p.xml")}", + ] + + @loaded_features_file = Tempfile.create("", @temp_dir) end def command(*args) @@ -112,7 +113,9 @@ def get_valgrind_suppression_files(dir) (0..3).reverse_each { |i| versions << full_ruby_version.split(".")[0, i].join(".") } versions << RUBY_ENGINE - versions.map { |version| Dir[File.join(dir, "#{version}.supp")] }.flatten + versions.map do |version| + Dir[File.join(dir, "#{version}.supp")] + end.flatten end end end diff --git a/lib/ruby_memcheck/frame.rb b/lib/ruby_memcheck/frame.rb index a499b20..c738520 100644 --- a/lib/ruby_memcheck/frame.rb +++ b/lib/ruby_memcheck/frame.rb @@ -2,10 +2,12 @@ module RubyMemcheck class Frame - attr_reader :configuration, :fn, :obj, :file, :line + attr_reader :configuration, :loaded_binaries, :fn, :obj, :file, :line - def initialize(configuration, frame_xml) + def initialize(configuration, loaded_binaries, frame_xml) @configuration = configuration + @loaded_binaries = loaded_binaries + @fn = frame_xml.at_xpath("fn")&.content @obj = frame_xml.at_xpath("obj")&.content # file and line may not be available @@ -24,8 +26,14 @@ def in_ruby? def in_binary? return false unless obj - binary_name_without_ext = File.join(File.dirname(obj), File.basename(obj, ".*")) - binary_name_without_ext.end_with?(File.join("", configuration.binary_name)) + loaded_binaries.include?(obj) + end + + def binary_init_func? + return false unless in_binary? + + binary_name = File.basename(obj, ".*") + fn == "Init_#{binary_name}" end def to_s diff --git a/lib/ruby_memcheck/rspec/rake_task.rb b/lib/ruby_memcheck/rspec/rake_task.rb index 7840ef0..eb22a78 100644 --- a/lib/ruby_memcheck/rspec/rake_task.rb +++ b/lib/ruby_memcheck/rspec/rake_task.rb @@ -5,9 +5,8 @@ module RubyMemcheck module RSpec class RakeTask < ::RSpec::Core::RakeTask - include TestTaskReporter - attr_reader :configuration + attr_reader :reporter def initialize(*args) @configuration = @@ -23,15 +22,14 @@ def initialize(*args) def run_task(verbose) error = nil - begin + @reporter = TestTaskReporter.new(configuration) + @reporter.run_ruby_with_valgrind do # RSpec::Core::RakeTask#run_task calls Kernel.exit on failure super rescue SystemExit => e error = e end - report_valgrind_errors - raise error if error end diff --git a/lib/ruby_memcheck/stack.rb b/lib/ruby_memcheck/stack.rb index 5b77b66..bd1228d 100644 --- a/lib/ruby_memcheck/stack.rb +++ b/lib/ruby_memcheck/stack.rb @@ -4,30 +4,28 @@ module RubyMemcheck class Stack attr_reader :configuration, :frames - def initialize(configuration, stack_xml) + def initialize(configuration, loaded_binaries, stack_xml) @configuration = configuration - @frames = stack_xml.xpath("frame").map { |frame| Frame.new(configuration, frame) } + @frames = stack_xml.xpath("frame").map { |frame| Frame.new(configuration, loaded_binaries, frame) } end def skip? in_binary = false frames.each do |frame| - fn = frame.fn - if frame.in_ruby? # If a stack from from the binary was encountered first, then this # memory leak did not occur from Ruby unless in_binary # Skip this stack because it was called from Ruby - return true if configuration.skipped_ruby_functions.any? { |r| r.match?(fn) } + return true if configuration.skipped_ruby_functions.any? { |r| r.match?(frame.fn) } end elsif frame.in_binary? in_binary = true # Skip the Init function because it is only ever called once, so # leaks in it cannot cause memory bloat - return true if fn == "Init_#{configuration.binary_name}" + return true if frame.binary_init_func? end end diff --git a/lib/ruby_memcheck/test_helper.rb b/lib/ruby_memcheck/test_helper.rb index daf636a..d9cf9e5 100644 --- a/lib/ruby_memcheck/test_helper.rb +++ b/lib/ruby_memcheck/test_helper.rb @@ -1,3 +1,9 @@ # frozen_string_literal: true -at_exit { GC.start } +at_exit do + File.open(ENV["RUBY_MEMCHECK_LOADED_FEATURES_FILE"], "w") do |f| + f.write($LOADED_FEATURES.join("\n")) + end + + GC.start +end diff --git a/lib/ruby_memcheck/test_task.rb b/lib/ruby_memcheck/test_task.rb index c1be1f2..a12a54f 100644 --- a/lib/ruby_memcheck/test_task.rb +++ b/lib/ruby_memcheck/test_task.rb @@ -2,9 +2,8 @@ module RubyMemcheck class TestTask < Rake::TestTask - include TestTaskReporter - attr_reader :configuration + attr_reader :reporter def initialize(*args) @configuration = @@ -19,8 +18,13 @@ def initialize(*args) def ruby(*args, **options, &block) command = configuration.command(args) + + @reporter = TestTaskReporter.new(configuration) + + @reporter.setup + sh(command, **options) do |ok, res| - report_valgrind_errors + @reporter.report_valgrind_errors yield ok, res if block_given? end diff --git a/lib/ruby_memcheck/test_task_reporter.rb b/lib/ruby_memcheck/test_task_reporter.rb index 0ae426c..a9c21b9 100644 --- a/lib/ruby_memcheck/test_task_reporter.rb +++ b/lib/ruby_memcheck/test_task_reporter.rb @@ -1,36 +1,62 @@ # frozen_string_literal: true module RubyMemcheck - module TestTaskReporter + class TestTaskReporter VALGRIND_REPORT_MSG = "Valgrind reported errors (e.g. memory leak or use-after-free)" + attr_reader :configuration attr_reader :errors - private + def initialize(configuration) + @configuration = configuration + @loaded_binaries = nil + end + + def run_ruby_with_valgrind(&block) + setup + yield + report_valgrind_errors + end + + def setup + ENV["RUBY_MEMCHECK_LOADED_FEATURES_FILE"] = File.expand_path(configuration.loaded_features_file) + ENV["RUBY_MEMCHECK_RUNNING"] = "1" + end def report_valgrind_errors - if configuration.valgrind_xml_dir - xml_files = valgrind_xml_files - parse_valgrind_output(xml_files) - remove_valgrind_xml_files(xml_files) - - unless errors.empty? - output_valgrind_errors - raise VALGRIND_REPORT_MSG - end + parse_valgrind_output + remove_valgrind_xml_files + + unless errors.empty? + output_valgrind_errors + raise VALGRIND_REPORT_MSG end end + private + + def loaded_binaries + return @loaded_binaries if @loaded_binaries + + loaded_features = File.readlines(configuration.loaded_features_file, chomp: true) + @loaded_binaries = loaded_features.keep_if do |feat| + # Keep only binaries (ignore Ruby files). + File.extname(feat) == ".so" + end.freeze + + @loaded_binaries + end + def valgrind_xml_files - Dir[File.join(configuration.valgrind_xml_dir, "*")] + @valgrind_xml_files ||= Dir[File.join(configuration.temp_dir, "*.xml")].freeze end - def parse_valgrind_output(xml_files) + def parse_valgrind_output require "nokogiri" @errors = [] - xml_files.each do |file| + valgrind_xml_files.each do |file| reader = Nokogiri::XML::Reader(File.open(file)) do |config| # rubocop:disable Style/SymbolProc config.huge end @@ -38,7 +64,7 @@ def parse_valgrind_output(xml_files) next unless node.name == "error" && node.node_type == Nokogiri::XML::Reader::TYPE_ELEMENT error_xml = Nokogiri::XML::Document.parse(node.outer_xml).root - error = ValgrindError.new(configuration, error_xml) + error = ValgrindError.new(configuration, loaded_binaries, error_xml) next if error.skip? @errors << error @@ -46,8 +72,8 @@ def parse_valgrind_output(xml_files) end end - def remove_valgrind_xml_files(xml_files) - xml_files.each do |file| + def remove_valgrind_xml_files + valgrind_xml_files.each do |file| File.delete(file) end end diff --git a/lib/ruby_memcheck/valgrind_error.rb b/lib/ruby_memcheck/valgrind_error.rb index 7a434fd..93e91a2 100644 --- a/lib/ruby_memcheck/valgrind_error.rb +++ b/lib/ruby_memcheck/valgrind_error.rb @@ -7,7 +7,7 @@ class ValgrindError attr_reader :kind, :msg, :stack, :suppression - def initialize(configuration, error) + def initialize(configuration, loaded_binaries, error) @kind = error.at_xpath("kind").content @msg = if kind_leak? @@ -15,7 +15,7 @@ def initialize(configuration, error) else error.at_xpath("what").content end - @stack = Stack.new(configuration, error.at_xpath("stack")) + @stack = Stack.new(configuration, loaded_binaries, error.at_xpath("stack")) @configuration = configuration suppression_node = error.at_xpath("suppression") diff --git a/suppressions/ruby.supp b/suppressions/ruby.supp index 38b1cc3..c84224b 100644 --- a/suppressions/ruby.supp +++ b/suppressions/ruby.supp @@ -50,3 +50,11 @@ fun:*try_statx* ... } +{ + strscan_do_scan in strscan.c will sometimes replace the ptr of the regex, which can be reported as a memory leak if the regex is stored in an iseq. https://github.com/ruby/ruby/pull/8136 + Memcheck:Leak + ... + fun:rb_reg_prepare_re + fun:strscan_do_scan + ... +} diff --git a/test/ruby_memcheck/ext/extconf.rb b/test/ruby_memcheck/ext/extconf.rb deleted file mode 100644 index 48888c5..0000000 --- a/test/ruby_memcheck/ext/extconf.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -require "mkmf" - -$warnflags&.gsub!(/-Wdeclaration-after-statement/, "") - -create_makefile("ruby_memcheck_c_test") diff --git a/test/ruby_memcheck/ext/extconf_one.rb b/test/ruby_memcheck/ext/extconf_one.rb new file mode 100644 index 0000000..5f48633 --- /dev/null +++ b/test/ruby_memcheck/ext/extconf_one.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require "mkmf" + +$warnflags&.gsub!(/-Wdeclaration-after-statement/, "") # rubocop:disable Style/GlobalVars + +create_makefile("ruby_memcheck_c_test_one") diff --git a/test/ruby_memcheck/ext/extconf_two.rb b/test/ruby_memcheck/ext/extconf_two.rb new file mode 100644 index 0000000..4d1ba8e --- /dev/null +++ b/test/ruby_memcheck/ext/extconf_two.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +require "mkmf" + +$warnflags&.gsub!(/-Wdeclaration-after-statement/, "") # rubocop:disable Style/GlobalVars + +create_makefile("ruby_memcheck_c_test_two") diff --git a/test/ruby_memcheck/ext/ruby_memcheck_c_test.c b/test/ruby_memcheck/ext/ruby_memcheck_c_test.c deleted file mode 100644 index fa195d4..0000000 --- a/test/ruby_memcheck/ext/ruby_memcheck_c_test.c +++ /dev/null @@ -1,63 +0,0 @@ -#include - -VALUE cRubyMemcheckCTest; - -static VALUE no_memory_leak(VALUE _) -{ - return Qnil; -} - -/* This function must not be inlined to ensure that it has a stack frame. */ -static void __attribute__((noinline)) allocate_memory_leak(void) -{ - volatile char *ptr = malloc(100); - ptr[0] = 'a'; -} - -static VALUE memory_leak(VALUE _) -{ - allocate_memory_leak(); - return Qnil; -} - -static VALUE use_after_free(VALUE _) -{ - volatile char *ptr = malloc(100); - free((void *)ptr); - ptr[0] = 'a'; - return Qnil; -} - -static VALUE uninitialized_value(VALUE _) -{ - volatile int foo; -#pragma GCC diagnostic ignored "-Wuninitialized" - return foo == 0 ? rb_str_new_cstr("zero") : rb_str_new_cstr("not zero"); -#pragma GCC diagnostic pop -} - -static VALUE call_into_ruby_mem_leak(VALUE obj) -{ - char str[20]; - for (int i = 0; i < 10000; i++) { - sprintf(str, "foobar%d", i); - rb_intern(str); - } - return Qnil; -} - -void Init_ruby_memcheck_c_test(void) -{ - /* Memory leaks in the Init functions should be ignored. */ - allocate_memory_leak(); - - VALUE mRubyMemcheck = rb_define_module("RubyMemcheck"); - cRubyMemcheckCTest = rb_define_class_under(mRubyMemcheck, "CTest", rb_cObject); - rb_global_variable(&cRubyMemcheckCTest); - - rb_define_method(cRubyMemcheckCTest, "no_memory_leak", no_memory_leak, 0); - rb_define_method(cRubyMemcheckCTest, "memory_leak", memory_leak, 0); - rb_define_method(cRubyMemcheckCTest, "use_after_free", use_after_free, 0); - rb_define_method(cRubyMemcheckCTest, "uninitialized_value", uninitialized_value, 0); - rb_define_method(cRubyMemcheckCTest, "call_into_ruby_mem_leak", call_into_ruby_mem_leak, 0); -} diff --git a/test/ruby_memcheck/ext/ruby_memcheck_c_test_one.c b/test/ruby_memcheck/ext/ruby_memcheck_c_test_one.c new file mode 100644 index 0000000..33c65f9 --- /dev/null +++ b/test/ruby_memcheck/ext/ruby_memcheck_c_test_one.c @@ -0,0 +1,63 @@ +#include + +static VALUE cRubyMemcheckCTestOne; + +static VALUE c_test_one_no_memory_leak(VALUE _) +{ + return Qnil; +} + +/* This function must not be inlined to ensure that it has a stack frame. */ +static void __attribute__((noinline)) c_test_one_allocate_memory_leak(void) +{ + volatile char *ptr = malloc(100); + ptr[0] = 'a'; +} + +static VALUE c_test_one_memory_leak(VALUE _) +{ + c_test_one_allocate_memory_leak(); + return Qnil; +} + +static VALUE c_test_one_use_after_free(VALUE _) +{ + volatile char *ptr = malloc(100); + free((void *)ptr); + ptr[0] = 'a'; + return Qnil; +} + +static VALUE c_test_one_uninitialized_value(VALUE _) +{ + volatile int foo; +#pragma GCC diagnostic ignored "-Wuninitialized" + return foo == 0 ? rb_str_new_cstr("zero") : rb_str_new_cstr("not zero"); +#pragma GCC diagnostic pop +} + +static VALUE c_test_one_call_into_ruby_mem_leak(VALUE obj) +{ + char str[20]; + for (int i = 0; i < 10000; i++) { + sprintf(str, "foobar%d", i); + rb_intern(str); + } + return Qnil; +} + +void Init_ruby_memcheck_c_test_one(void) +{ + /* Memory leaks in the Init functions should be ignored. */ + c_test_one_allocate_memory_leak(); + + VALUE mRubyMemcheck = rb_define_module("RubyMemcheck"); + cRubyMemcheckCTestOne = rb_define_class_under(mRubyMemcheck, "CTestOne", rb_cObject); + rb_global_variable(&cRubyMemcheckCTestOne); + + rb_define_method(cRubyMemcheckCTestOne, "no_memory_leak", c_test_one_no_memory_leak, 0); + rb_define_method(cRubyMemcheckCTestOne, "memory_leak", c_test_one_memory_leak, 0); + rb_define_method(cRubyMemcheckCTestOne, "use_after_free", c_test_one_use_after_free, 0); + rb_define_method(cRubyMemcheckCTestOne, "uninitialized_value", c_test_one_uninitialized_value, 0); + rb_define_method(cRubyMemcheckCTestOne, "call_into_ruby_mem_leak", c_test_one_call_into_ruby_mem_leak, 0); +} diff --git a/test/ruby_memcheck/ext/ruby_memcheck_c_test_two.c b/test/ruby_memcheck/ext/ruby_memcheck_c_test_two.c new file mode 100644 index 0000000..91fe33a --- /dev/null +++ b/test/ruby_memcheck/ext/ruby_memcheck_c_test_two.c @@ -0,0 +1,63 @@ +#include + +static VALUE cRubyMemcheckCTestTwo; + +static VALUE c_test_two_no_memory_leak(VALUE _) +{ + return Qnil; +} + +/* This function must not be inlined to ensure that it has a stack frame. */ +static void __attribute__((noinline)) c_test_two_allocate_memory_leak(void) +{ + volatile char *ptr = malloc(100); + ptr[0] = 'a'; +} + +static VALUE c_test_two_memory_leak(VALUE _) +{ + c_test_two_allocate_memory_leak(); + return Qnil; +} + +static VALUE c_test_two_use_after_free(VALUE _) +{ + volatile char *ptr = malloc(100); + free((void *)ptr); + ptr[0] = 'a'; + return Qnil; +} + +static VALUE c_test_two_uninitialized_value(VALUE _) +{ + volatile int foo; +#pragma GCC diagnostic ignored "-Wuninitialized" + return foo == 0 ? rb_str_new_cstr("zero") : rb_str_new_cstr("not zero"); +#pragma GCC diagnostic pop +} + +static VALUE c_test_two_call_into_ruby_mem_leak(VALUE obj) +{ + char str[20]; + for (int i = 0; i < 10000; i++) { + sprintf(str, "foobar%d", i); + rb_intern(str); + } + return Qnil; +} + +void Init_ruby_memcheck_c_test_two(void) +{ + /* Memory leaks in the Init functions should be ignored. */ + c_test_two_allocate_memory_leak(); + + VALUE mRubyMemcheck = rb_define_module("RubyMemcheck"); + cRubyMemcheckCTestTwo = rb_define_class_under(mRubyMemcheck, "CTestTwo", rb_cObject); + rb_global_variable(&cRubyMemcheckCTestTwo); + + rb_define_method(cRubyMemcheckCTestTwo, "no_memory_leak", c_test_two_no_memory_leak, 0); + rb_define_method(cRubyMemcheckCTestTwo, "memory_leak", c_test_two_memory_leak, 0); + rb_define_method(cRubyMemcheckCTestTwo, "use_after_free", c_test_two_use_after_free, 0); + rb_define_method(cRubyMemcheckCTestTwo, "uninitialized_value", c_test_two_uninitialized_value, 0); + rb_define_method(cRubyMemcheckCTestTwo, "call_into_ruby_mem_leak", c_test_two_call_into_ruby_mem_leak, 0); +} diff --git a/test/ruby_memcheck/rspec/rake_task_test.rb b/test/ruby_memcheck/rspec/rake_task_test.rb index 5454be2..9da9022 100644 --- a/test/ruby_memcheck/rspec/rake_task_test.rb +++ b/test/ruby_memcheck/rspec/rake_task_test.rb @@ -32,9 +32,10 @@ def run_with_memcheck(code, raise_on_failure: true, spawn_opts: {}) $stdout.reopen(File.open("#{stdout_log.path}", "w")) $LOAD_PATH.unshift("#{File.join(__dir__, "../ext")}") - require "ruby_memcheck_c_test" + require "ruby_memcheck_c_test_one" + require "ruby_memcheck_c_test_two" - RSpec.describe RubyMemcheck::CTest do + RSpec.describe RubyMemcheck do it "test" do #{code} end diff --git a/test/ruby_memcheck/ruby_memcheck_suppression_test.rb b/test/ruby_memcheck/ruby_memcheck_suppression_test.rb index 5199e6a..d458873 100644 --- a/test/ruby_memcheck/ruby_memcheck_suppression_test.rb +++ b/test/ruby_memcheck/ruby_memcheck_suppression_test.rb @@ -6,7 +6,7 @@ module RubyMemcheck class RubyMemcheckSuppressionTest < Minitest::Test def setup - @configuration = Configuration.new(binary_name: "ruby_memcheck_c_test") + @configuration = Configuration.new end def test_given_a_suppression_node diff --git a/test/ruby_memcheck/shared_test_task_reporter_tests.rb b/test/ruby_memcheck/shared_test_task_reporter_tests.rb index 4e68ea0..94eb914 100644 --- a/test/ruby_memcheck/shared_test_task_reporter_tests.rb +++ b/test/ruby_memcheck/shared_test_task_reporter_tests.rb @@ -6,64 +6,64 @@ module RubyMemcheck module SharedTestTaskReporterTests def test_succeeds_when_there_is_no_memory_leak ok = run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.no_memory_leak + RubyMemcheck::CTestOne.new.no_memory_leak RUBY assert(ok) - assert_empty(@test_task.errors) + assert_empty(@test_task.reporter.errors) assert_empty(@output_io.string) end def test_reports_memory_leak error = assert_raises do run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.memory_leak + RubyMemcheck::CTestOne.new.memory_leak RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) - assert_equal(1, @test_task.errors.length) + assert_equal(1, @test_task.reporter.errors.length) output = @output_io.string refute_empty(output) assert_match(/^100 bytes in 1 blocks are definitely lost in loss record/, output) - assert_match(/^ \*memory_leak \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_memory_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) end def test_reports_use_after_free error = assert_raises do run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.use_after_free + RubyMemcheck::CTestOne.new.use_after_free RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) - assert_equal(1, @test_task.errors.length) + assert_equal(1, @test_task.reporter.errors.length) output = @output_io.string refute_empty(output) assert_match(/^Invalid write of size 1$/, output) - assert_match(/^ \*use_after_free \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_use_after_free \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) end # Potential improvement: support uninitialized values def test_does_not_report_uninitialized_value run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.uninitialized_value + RubyMemcheck::CTestOne.new.uninitialized_value RUBY - assert_equal(0, @test_task.errors.length) - assert_empty(@test_task.errors) + assert_equal(0, @test_task.reporter.errors.length) + assert_empty(@test_task.reporter.errors) assert_empty(@output_io.string) end def test_call_into_ruby_mem_leak_does_not_report ok = run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.call_into_ruby_mem_leak + RubyMemcheck::CTestOne.new.call_into_ruby_mem_leak RUBY assert(ok) - assert_empty(@test_task.errors) + assert_empty(@test_task.reporter.errors) assert_empty(@output_io.string) end @@ -72,23 +72,23 @@ def test_call_into_ruby_mem_leak_reports_when_not_skipped error = assert_raises do run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.call_into_ruby_mem_leak + RubyMemcheck::CTestOne.new.call_into_ruby_mem_leak RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) - assert_operator(@test_task.errors.length, :>=, 1) + assert_operator(@test_task.reporter.errors.length, :>=, 1) end def test_suppressions build_configuration(valgrind_suppressions_dir: File.join(__dir__, "suppressions")) ok = run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.memory_leak + RubyMemcheck::CTestOne.new.memory_leak RUBY assert(ok) - assert_empty(@test_task.errors) + assert_empty(@test_task.reporter.errors) assert_empty(@output_io.string) end @@ -97,65 +97,85 @@ def test_generation_of_suppressions error = assert_raises do run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.memory_leak + RubyMemcheck::CTestOne.new.memory_leak RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) - assert_equal(1, @test_task.errors.length) + assert_equal(1, @test_task.reporter.errors.length) output = @output_io.string refute_empty(output) assert_match(/^100 bytes in 1 blocks are definitely lost in loss record/, output) - assert_match(/^ \*memory_leak \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_memory_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) assert_match(/^ insert_a_suppression_name_here/, output) assert_match(/^ Memcheck:Leak/, output) - assert_match(/^ fun:allocate_memory_leak/, output) + assert_match(/^ fun:c_test_one_allocate_memory_leak/, output) end def test_follows_forked_children error = assert_raises do run_with_memcheck(<<~RUBY) pid = Process.fork do - RubyMemcheck::CTest.new.memory_leak + RubyMemcheck::CTestOne.new.memory_leak end Process.wait(pid) RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) - assert_equal(1, @test_task.errors.length) + assert_equal(1, @test_task.reporter.errors.length) output = @output_io.string refute_empty(output) assert_match(/^100 bytes in 1 blocks are definitely lost in loss record/, output) - assert_match(/^ \*memory_leak \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_memory_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) end def test_reports_multiple_errors error = assert_raises do run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.memory_leak - RubyMemcheck::CTest.new.use_after_free + RubyMemcheck::CTestOne.new.memory_leak + RubyMemcheck::CTestOne.new.use_after_free RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) - - assert_equal(2, @test_task.errors.length) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) output = @output_io.string + + assert_equal(2, @test_task.reporter.errors.length, output) + refute_empty(output) assert_match(/^100 bytes in 1 blocks are definitely lost in loss record/, output) - assert_match(/^ \*memory_leak \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_memory_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) assert_match(/^Invalid write of size 1$/, output) - assert_match(/^ \*use_after_free \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_use_after_free \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) + end + + def test_reports_errors_in_all_binaries + error = assert_raises do + run_with_memcheck(<<~RUBY) + RubyMemcheck::CTestOne.new.memory_leak + RubyMemcheck::CTestTwo.new.memory_leak + RUBY + end + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) + + output = @output_io.string + + assert_equal(2, @test_task.reporter.errors.length, output) + + refute_empty(output) + assert_match(/^100 bytes in 1 blocks are definitely lost in loss record/, output) + assert_match(/^ \*c_test_one_memory_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) + assert_match(/^ \*c_test_two_memory_leak \(ruby_memcheck_c_test_two\.c:\d+\)$/, output) end def test_can_run_multiple_times 2.times do ok = run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.no_memory_leak + RubyMemcheck::CTestOne.new.no_memory_leak RUBY assert(ok) end @@ -167,7 +187,7 @@ def test_ruby_failure_without_errors RUBY refute(ok) - assert_empty(@test_task.errors) + assert_empty(@test_task.reporter.errors) assert_empty(@output_io.string) rescue $stderr.puts(@test_task.reporter.errors) @@ -177,18 +197,18 @@ def test_ruby_failure_without_errors def test_ruby_failure_with_errors error = assert_raises do run_with_memcheck(<<~RUBY, raise_on_failure: false, spawn_opts: { out: "/dev/null", err: "/dev/null" }) - RubyMemcheck::CTest.new.memory_leak + RubyMemcheck::CTestOne.new.memory_leak raise RUBY end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) + assert_equal(RubyMemcheck::TestTaskReporter::VALGRIND_REPORT_MSG, error.message) - assert_equal(1, @test_task.errors.length) + assert_equal(1, @test_task.reporter.errors.length) output = @output_io.string refute_empty(output) assert_match(/^100 bytes in 1 blocks are definitely lost in loss record/, output) - assert_match(/^ \*memory_leak \(ruby_memcheck_c_test\.c:\d+\)$/, output) + assert_match(/^ \*c_test_one_memory_leak \(ruby_memcheck_c_test_one\.c:\d+\)$/, output) end def test_test_helper_is_loaded @@ -198,27 +218,21 @@ def test_test_helper_is_loaded RUBY assert(ok) - assert_empty(@test_task.errors) + assert_empty(@test_task.reporter.errors) assert_includes(tempfile.read, File.expand_path(File.join(__dir__, "../../lib/ruby_memcheck/test_helper.rb"))) end end - def test_binary_name_with_path_to_binary - # Correct path - build_configuration(binary_name: "ext/ruby_memcheck_c_test") - error = assert_raises do - run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.memory_leak + def test_envionment_variable_RUBY_MEMCHECK_RUNNING + Tempfile.create do |tempfile| + ok = run_with_memcheck(<<~RUBY, raise_on_failure: false) + File.write(#{tempfile.path.inspect}, ENV["RUBY_MEMCHECK_RUNNING"]) RUBY - end - assert_equal(RubyMemcheck::TestTask::VALGRIND_REPORT_MSG, error.message) - # Incorrect path - build_configuration(binary_name: "foobar/ruby_memcheck_c_test") - ok = run_with_memcheck(<<~RUBY) - RubyMemcheck::CTest.new.memory_leak - RUBY - assert(ok) + assert(ok) + assert_empty(@test_task.reporter.errors) + assert_includes(tempfile.read, "1") + end end private @@ -228,12 +242,10 @@ def run_with_memcheck(code, raise_on_failure: true, spawn_opts: {}) end def build_configuration( - binary_name: "ruby_memcheck_c_test", output_io: @output_io, **options ) @configuration = Configuration.new( - binary_name: binary_name, output_io: @output_io, **options, ) diff --git a/test/ruby_memcheck/suppressions/ruby.supp b/test/ruby_memcheck/suppressions/ruby.supp index b2cd1eb..95b4056 100644 --- a/test/ruby_memcheck/suppressions/ruby.supp +++ b/test/ruby_memcheck/suppressions/ruby.supp @@ -2,6 +2,6 @@ suppress memory_leak Memcheck:Leak ... - fun:memory_leak + fun:c_test_one_memory_leak ... } diff --git a/test/ruby_memcheck/test_task_test.rb b/test/ruby_memcheck/test_task_test.rb index 64c3304..eda18ed 100644 --- a/test/ruby_memcheck/test_task_test.rb +++ b/test/ruby_memcheck/test_task_test.rb @@ -18,7 +18,8 @@ def setup def run_with_memcheck(code, raise_on_failure: true, spawn_opts: {}) script = Tempfile.new script.write(<<~RUBY) - require "ruby_memcheck_c_test" + require "ruby_memcheck_c_test_one" + require "ruby_memcheck_c_test_two" #{code} RUBY script.flush diff --git a/test/ruby_memcheck/valgrind_error_test.rb b/test/ruby_memcheck/valgrind_error_test.rb index eaacd62..2f4cff5 100644 --- a/test/ruby_memcheck/valgrind_error_test.rb +++ b/test/ruby_memcheck/valgrind_error_test.rb @@ -6,7 +6,7 @@ module RubyMemcheck class ValgrindErrorTest < Minitest::Test def setup - @configuration = Configuration.new(binary_name: "ruby_memcheck_c_test") + @configuration = Configuration.new end def test_raises_when_suppressions_generated_but_not_configured @@ -30,7 +30,7 @@ def test_raises_when_suppressions_generated_but_not_configured XML error = assert_raises do - RubyMemcheck::ValgrindError.new(@configuration, output) + RubyMemcheck::ValgrindError.new(@configuration, [], output) end assert_equal(ValgrindError::SUPPRESSION_NOT_CONFIGURED_ERROR_MSG, error.message) end diff --git a/test/ruby_memcheck_test.rb b/test/ruby_memcheck_test.rb new file mode 100644 index 0000000..82e1ce7 --- /dev/null +++ b/test/ruby_memcheck_test.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require "test_helper" + +class RubyMemcheckTest < Minitest::Test + def setup + RubyMemcheck.instance_variable_set(:@default_configuration, nil) + end + + def test_config_sets_default_configuration + config = RubyMemcheck.config + + assert_equal(config, RubyMemcheck.default_configuration) + end + + def test_default_configuration_creates_new_configuration + config = RubyMemcheck.default_configuration + refute_nil(config) + assert_equal(config, RubyMemcheck.default_configuration) + end +end