Skip to content

Commit

Permalink
Fix Regexp#inspect for GC compaction
Browse files Browse the repository at this point in the history
rb_reg_desc was not safe for GC compaction because it took in the C
string and length but not the backing String object so it get moved
during compaction. This commit changes rb_reg_desc to use the string
from the Regexp object.

The test fails when RGENGC_CHECK_MODE is turned on:

    TestRegexp#test_inspect_under_gc_compact_stress [test/ruby/test_regexp.rb:474]:
    <"(?-mix:\\/)|"> expected but was
    <"/\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00/">.
  • Loading branch information
peterzhu2118 committed Dec 24, 2023
1 parent 8ad8803 commit f0efedd
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
20 changes: 12 additions & 8 deletions re.c
Expand Up @@ -450,7 +450,7 @@ rb_reg_expr_str(VALUE str, const char *s, long len,
}

static VALUE
rb_reg_desc(const char *s, long len, VALUE re)
rb_reg_desc(VALUE re)
{
rb_encoding *enc = rb_enc_get(re);
VALUE str = rb_str_buf_new2("/");
Expand All @@ -463,7 +463,11 @@ rb_reg_desc(const char *s, long len, VALUE re)
else {
rb_enc_associate(str, rb_usascii_encoding());
}
rb_reg_expr_str(str, s, len, enc, resenc, '/');

VALUE src_str = RREGEXP_SRC(re);
rb_reg_expr_str(str, RSTRING_PTR(src_str), RSTRING_LEN(src_str), enc, resenc, '/');
RB_GC_GUARD(src_str);

rb_str_buf_cat2(str, "/");
if (re) {
char opts[OPTBUF_SIZE];
Expand Down Expand Up @@ -522,7 +526,7 @@ rb_reg_inspect(VALUE re)
if (!RREGEXP_PTR(re) || !RREGEXP_SRC(re) || !RREGEXP_SRC_PTR(re)) {
return rb_any_to_s(re);
}
return rb_reg_desc(RREGEXP_SRC_PTR(re), RREGEXP_SRC_LEN(re), re);
return rb_reg_desc(re);
}

static VALUE rb_reg_str_with_term(VALUE re, int term);
Expand Down Expand Up @@ -670,12 +674,12 @@ rb_reg_str_with_term(VALUE re, int term)
return str;
}

NORETURN(static void rb_reg_raise(const char *s, long len, const char *err, VALUE re));
NORETURN(static void rb_reg_raise(const char *err, VALUE re));

static void
rb_reg_raise(const char *s, long len, const char *err, VALUE re)
rb_reg_raise(const char *err, VALUE re)
{
VALUE desc = rb_reg_desc(s, len, re);
VALUE desc = rb_reg_desc(re);

rb_raise(rb_eRegexpError, "%s: %"PRIsVALUE, err, desc);
}
Expand Down Expand Up @@ -1634,7 +1638,7 @@ rb_reg_prepare_re(VALUE re, VALUE str)

if (r) {
onig_error_code_to_str((UChar*)err, r, &einfo);
rb_reg_raise(pattern, RREGEXP_SRC_LEN(re), err, re);
rb_reg_raise(err, re);
}

reg->timelimit = timelimit;
Expand Down Expand Up @@ -1667,7 +1671,7 @@ rb_reg_onig_match(VALUE re, VALUE str,
if (result != ONIG_MISMATCH) {
onig_errmsg_buffer err = "";
onig_error_code_to_str((UChar*)err, (int)result);
rb_reg_raise(RREGEXP_SRC_PTR(re), RREGEXP_SRC_LEN(re), err, re);
rb_reg_raise(err, re);
}
}

Expand Down
6 changes: 6 additions & 0 deletions test/ruby/test_regexp.rb
Expand Up @@ -469,6 +469,12 @@ def test_inspect
assert_equal('/\/\xF1\xF2\xF3/i', /\/#{s}/i.inspect)
end

def test_inspect_under_gc_compact_stress
EnvUtil.under_gc_compact_stress do
assert_equal('/(?-mix:\\/)|/', Regexp.union(/\//, "").inspect)
end
end

def test_char_to_option
assert_equal("BAR", "FOOBARBAZ"[/b../i])
assert_equal("bar", "foobarbaz"[/ b . . /x])
Expand Down

0 comments on commit f0efedd

Please sign in to comment.