Skip to content

Commit

Permalink
Automatically detect Ruby native extensions
Browse files Browse the repository at this point in the history
$LOADED_FEATURES is now automatically communicated from the spawned
process back into the main ruby_memcheck process which allows
ruby_memcheck to automatically detect which binaries are native
extensions and allow ruby_memcheck to work for multiple native
extensions.
  • Loading branch information
peterzhu2118 committed Jul 5, 2023
1 parent 95dcefb commit 60dd673
Show file tree
Hide file tree
Showing 20 changed files with 261 additions and 142 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ When you run `RubyMemcheck.config`, you are creating a default `RubyMemcheck::Co
- `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`.
Expand Down
9 changes: 8 additions & 1 deletion Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
39 changes: 21 additions & 18 deletions lib/ruby_memcheck/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,33 +31,34 @@ 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

alias_method :valgrind_generate_suppressions?, :valgrind_generate_suppressions
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
Expand All @@ -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)
Expand Down Expand Up @@ -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
16 changes: 12 additions & 4 deletions lib/ruby_memcheck/frame.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 3 additions & 5 deletions lib/ruby_memcheck/rspec/rake_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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

Expand Down
10 changes: 4 additions & 6 deletions lib/ruby_memcheck/stack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 7 additions & 1 deletion lib/ruby_memcheck/test_helper.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 7 additions & 3 deletions lib/ruby_memcheck/test_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

module RubyMemcheck
class TestTask < Rake::TestTask
include TestTaskReporter

attr_reader :configuration
attr_reader :reporter

def initialize(*args)
@configuration =
Expand All @@ -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
Expand Down
59 changes: 42 additions & 17 deletions lib/ruby_memcheck/test_task_reporter.rb
Original file line number Diff line number Diff line change
@@ -1,53 +1,78 @@
# 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)
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
reader.each do |node|
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
end
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
Expand Down
4 changes: 2 additions & 2 deletions lib/ruby_memcheck/valgrind_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ 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?
error.at_xpath("xwhat/text").content
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")
Expand Down
7 changes: 0 additions & 7 deletions test/ruby_memcheck/ext/extconf.rb

This file was deleted.

7 changes: 7 additions & 0 deletions test/ruby_memcheck/ext/extconf_one.rb
Original file line number Diff line number Diff line change
@@ -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")
7 changes: 7 additions & 0 deletions test/ruby_memcheck/ext/extconf_two.rb
Original file line number Diff line number Diff line change
@@ -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")
Loading

0 comments on commit 60dd673

Please sign in to comment.