Project

General

Profile

Actions

Feature #20274

closed

Add RubyVM::ASAN.enabled?

Added by kjtsanaktsidis (KJ Tsanaktsidis) 10 months ago. Updated 10 months ago.

Status:
Closed
Assignee:
-
Target version:
-
[ruby-core:116815]

Description

Some parts of the Ruby test suite won't work correctly under ASAN. In particular, assert_no_memory_leak will need different parameters for ASAN (or be skipped, in the same way as for MJIT/RJIT).

I propose that we add a module RubyVM::ASAN (which will be unconditionally defined), and RubyVM::ASAN.enabled? (which will return true if Ruby was compiled with ASAN, or false otherwise).

This means we can check if ASAN is enabled by running defined?(RubyVM::ASAN) && RubyVM::ASAN.enabled?, in the same way that the mjit/rjit check is performed.


Related issues 1 (1 open0 closed)

Related to Ruby master - Misc #20387: Meta-ticket for ASAN supportAssignedkjtsanaktsidis (KJ Tsanaktsidis)Actions

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 10 months ago

The other option is to define RubyVM.asan_enabled? as a method, rather than defining RubyVM::ASAN as a module.

Updated by nobu (Nobuyoshi Nakada) 10 months ago

At first, ASAN is not related to Ruby VM, neither the constant nor method under RubyVM do not make sense.

How are you going to detect it?
Maybe from CFLAGS?

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 10 months ago

GCC & clang set macros indicating that asan is enabled: https://github.com/ruby/ruby/blob/df63e5bef67ff74216834f61748aa6ea8b0de22e/internal/sanitizers.h#L18

So I was going to implement RubyVM::ASAN.enabled? by using conditional compilation to just return Qtrue or Qfalse.

Maybe RbConfig might be a better place for it? I think I’d have to write some autoconf macros/test program to emit an AC_SUBST about whether asan is enabled. But that should be just as robust. WDYT?

Updated by mame (Yusuke Endoh) 10 months ago

You just need the API for the Ruby test suite, right? Then, it would be enough to implement it in ext/-test-/. Introducing a method under RubyVM means making it available to Ruby users. Is that necessary?

Note, I don't think that it is a good idea to change the test behavior only when ASAN is enabled. I understand #20273 since callcc is very special, though.

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 10 months ago

Then, it would be enough to implement it in ext/-test-/.

This is a much better idea - I'll do this.

Note, I don't think that it is a good idea to change the test behavior only when ASAN is enabled

I definitely agree with this in general. The only place I've found so far where I actually want to change test behaviour on ASAN is assert_no_memory_leak which I think is also a bit special (it just hardcodes some guess numbers about how much memory the subprocess "should" use. I'm very carefully evaluating all the other failures the test suite throws up and fixing them :) (I think I have found two actual real bugs already with it!)

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 10 months ago

I opened https://github.com/ruby/ruby/pull/10087 to implement this as an extension in ext/-test-/ as suggested.

Actions #7

Updated by Anonymous 10 months ago

  • Status changed from Open to Closed

Applied in changeset git|fe0b704df5553bdd59e90650ffbbfac785a2e48a.


Skip assert_no_memory_leak when ASAN is enabled

ASAN greatly increases the memory footprint of Ruby, so these static
thresholds are not appropriate. There's no real need to run these tests
under ASAN.

[Bug #20274]

Actions #8

Updated by kjtsanaktsidis (KJ Tsanaktsidis) 9 months ago

  • Related to Misc #20387: Meta-ticket for ASAN support added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0