Show where mutated chilled strings were allocated

[Feature #20205]

The warning now suggests running with --debug-frozen-string-literal:

```
test.rb:3: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information)
```

When using --debug-frozen-string-literal, the location where the string
was created is shown:

```
test.rb:3: warning: literal string will be frozen in the future
test.rb:1: info: the string was created here
```

When resurrecting strings and debug mode is not enabled, the overhead is a simple FL_TEST_RAW.
When mutating chilled strings and deprecation warnings are not enabled,
the overhead is a simple warning category enabled check.

Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
This commit is contained in:
Étienne Barrié 2024-10-14 11:28:59 +02:00 committed by Jean Boussier
parent 75ef89ca16
commit 257f78fb67
Notes: git 2024-10-21 10:33:20 +00:00
9 changed files with 144 additions and 77 deletions

View File

@ -4726,7 +4726,7 @@ assert_equal '[0, {1 => 1}]', %q{
} }
# Chilled string setivar trigger warning # Chilled string setivar trigger warning
assert_equal 'literal string will be frozen in the future', %q{ assert_match(/literal string will be frozen in the future/, %q{
Warning[:deprecated] = true Warning[:deprecated] = true
$VERBOSE = true $VERBOSE = true
$warning = "no-warning" $warning = "no-warning"
@ -4754,7 +4754,7 @@ assert_equal 'literal string will be frozen in the future', %q{
setivar!("chilled") # Emit warning setivar!("chilled") # Emit warning
$warning $warning
} })
# arity=-2 cfuncs # arity=-2 cfuncs
assert_equal '["", "1/2", [0, [:ok, 1]]]', %q{ assert_equal '["", "1/2", [0, [:ok, 1]]]', %q{

View File

@ -4873,10 +4873,8 @@ static_literal_value(const NODE *node, rb_iseq_t *iseq)
case NODE_FILE: case NODE_FILE:
case NODE_STR: case NODE_STR:
if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) { if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) {
VALUE debug_info = rb_ary_new_from_args(2, rb_iseq_path(iseq), INT2FIX((int)nd_line(node))); VALUE lit = get_string_value(node);
VALUE lit = rb_str_dup(get_string_value(node)); return rb_str_with_debug_created_info(lit, rb_iseq_path(iseq), (int)nd_line(node));
rb_ivar_set(lit, id_debug_created_info, rb_ary_freeze(debug_info));
return rb_str_freeze(lit);
} }
else { else {
return get_string_value(node); return get_string_value(node);
@ -10927,28 +10925,25 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const no
debugp_param("nd_lit", get_string_value(node)); debugp_param("nd_lit", get_string_value(node));
if (!popped) { if (!popped) {
VALUE lit = get_string_value(node); VALUE lit = get_string_value(node);
switch (ISEQ_COMPILE_DATA(iseq)->option->frozen_string_literal) { const rb_compile_option_t *option = ISEQ_COMPILE_DATA(iseq)->option;
if ((option->debug_frozen_string_literal || RTEST(ruby_debug)) &&
option->frozen_string_literal != ISEQ_FROZEN_STRING_LITERAL_DISABLED) {
lit = rb_str_with_debug_created_info(lit, rb_iseq_path(iseq), line);
}
switch (option->frozen_string_literal) {
case ISEQ_FROZEN_STRING_LITERAL_UNSET: case ISEQ_FROZEN_STRING_LITERAL_UNSET:
ADD_INSN1(ret, node, putchilledstring, lit); ADD_INSN1(ret, node, putchilledstring, lit);
RB_OBJ_WRITTEN(iseq, Qundef, lit);
break; break;
case ISEQ_FROZEN_STRING_LITERAL_DISABLED: case ISEQ_FROZEN_STRING_LITERAL_DISABLED:
ADD_INSN1(ret, node, putstring, lit); ADD_INSN1(ret, node, putstring, lit);
RB_OBJ_WRITTEN(iseq, Qundef, lit);
break; break;
case ISEQ_FROZEN_STRING_LITERAL_ENABLED: case ISEQ_FROZEN_STRING_LITERAL_ENABLED:
if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) {
VALUE debug_info = rb_ary_new_from_args(2, rb_iseq_path(iseq), INT2FIX(line));
lit = rb_str_dup(lit);
rb_ivar_set(lit, id_debug_created_info, rb_ary_freeze(debug_info));
lit = rb_str_freeze(lit);
}
ADD_INSN1(ret, node, putobject, lit); ADD_INSN1(ret, node, putobject, lit);
RB_OBJ_WRITTEN(iseq, Qundef, lit);
break; break;
default: default:
rb_bug("invalid frozen_string_literal"); rb_bug("invalid frozen_string_literal");
} }
RB_OBJ_WRITTEN(iseq, Qundef, lit);
} }
break; break;
} }

57
error.c
View File

@ -3973,13 +3973,25 @@ inspect_frozen_obj(VALUE obj, VALUE mesg, int recur)
return mesg; return mesg;
} }
static VALUE
get_created_info(VALUE obj, int *pline)
{
VALUE info = rb_attr_get(obj, id_debug_created_info);
if (NIL_P(info)) return Qnil;
VALUE path = rb_ary_entry(info, 0);
VALUE line = rb_ary_entry(info, 1);
if (NIL_P(path)) return Qnil;
*pline = NUM2INT(line);
return StringValue(path);
}
void void
rb_error_frozen_object(VALUE frozen_obj) rb_error_frozen_object(VALUE frozen_obj)
{ {
rb_yjit_lazy_push_frame(GET_EC()->cfp->pc); rb_yjit_lazy_push_frame(GET_EC()->cfp->pc);
VALUE debug_info;
const ID created_info = id_debug_created_info;
VALUE mesg = rb_sprintf("can't modify frozen %"PRIsVALUE": ", VALUE mesg = rb_sprintf("can't modify frozen %"PRIsVALUE": ",
CLASS_OF(frozen_obj)); CLASS_OF(frozen_obj));
VALUE exc = rb_exc_new_str(rb_eFrozenError, mesg); VALUE exc = rb_exc_new_str(rb_eFrozenError, mesg);
@ -3987,15 +3999,46 @@ rb_error_frozen_object(VALUE frozen_obj)
rb_ivar_set(exc, id_recv, frozen_obj); rb_ivar_set(exc, id_recv, frozen_obj);
rb_exec_recursive(inspect_frozen_obj, frozen_obj, mesg); rb_exec_recursive(inspect_frozen_obj, frozen_obj, mesg);
if (!NIL_P(debug_info = rb_attr_get(frozen_obj, created_info))) { int created_line;
VALUE path = rb_ary_entry(debug_info, 0); VALUE created_path = get_created_info(frozen_obj, &created_line);
VALUE line = rb_ary_entry(debug_info, 1); if (!NIL_P(created_path)) {
rb_str_catf(mesg, ", created at %"PRIsVALUE":%d", created_path, created_line);
rb_str_catf(mesg, ", created at %"PRIsVALUE":%"PRIsVALUE, path, line);
} }
rb_exc_raise(exc); rb_exc_raise(exc);
} }
void
rb_warn_unchilled(VALUE obj)
{
rb_warning_category_t category = RB_WARN_CATEGORY_DEPRECATED;
if (!NIL_P(ruby_verbose) && rb_warning_category_enabled_p(category)) {
int line;
VALUE file = rb_source_location(&line);
VALUE mesg = NIL_P(file) ? rb_str_new(0, 0) : rb_str_dup(file);
if (!NIL_P(file)) {
if (line) rb_str_catf(mesg, ":%d", line);
rb_str_cat2(mesg, ": ");
}
rb_str_cat2(mesg, "warning: literal string will be frozen in the future");
VALUE str = obj;
if (STR_SHARED_P(str)) {
str = RSTRING(obj)->as.heap.aux.shared;
}
VALUE created = get_created_info(str, &line);
if (NIL_P(created)) {
rb_str_cat2(mesg, " (run with --debug-frozen-string-literal for more information)\n");
} else {
rb_str_cat2(mesg, "\n");
rb_str_append(mesg, created);
if (line) rb_str_catf(mesg, ":%d", line);
rb_str_cat2(mesg, ": info: the string was created here\n");
}
rb_warn_category(mesg, rb_warning_category_to_name(category));
}
}
#undef rb_check_frozen #undef rb_check_frozen
void void
rb_check_frozen(VALUE obj) rb_check_frozen(VALUE obj)

View File

@ -57,6 +57,10 @@ VALUE rb_str_upto_each(VALUE, VALUE, int, int (*each)(VALUE, VALUE), VALUE);
size_t rb_str_size_as_embedded(VALUE); size_t rb_str_size_as_embedded(VALUE);
bool rb_str_reembeddable_p(VALUE); bool rb_str_reembeddable_p(VALUE);
VALUE rb_str_upto_endless_each(VALUE, int (*each)(VALUE, VALUE), VALUE); VALUE rb_str_upto_endless_each(VALUE, int (*each)(VALUE, VALUE), VALUE);
VALUE rb_str_with_debug_created_info(VALUE, VALUE, int);
/* error.c */
void rb_warn_unchilled(VALUE str);
static inline bool STR_EMBED_P(VALUE str); static inline bool STR_EMBED_P(VALUE str);
static inline bool STR_SHARED_P(VALUE str); static inline bool STR_SHARED_P(VALUE str);
@ -124,7 +128,7 @@ static inline void
CHILLED_STRING_MUTATED(VALUE str) CHILLED_STRING_MUTATED(VALUE str)
{ {
FL_UNSET_RAW(str, STR_CHILLED); FL_UNSET_RAW(str, STR_CHILLED);
rb_category_warn(RB_WARN_CATEGORY_DEPRECATED, "literal string will be frozen in the future"); rb_warn_unchilled(str);
} }
static inline void static inline void

View File

@ -319,10 +319,7 @@ parse_static_literal_string(rb_iseq_t *iseq, const pm_scope_node_t *scope_node,
if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) { if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) {
int line_number = pm_node_line_number(scope_node->parser, node); int line_number = pm_node_line_number(scope_node->parser, node);
VALUE debug_info = rb_ary_new_from_args(2, rb_iseq_path(iseq), INT2FIX(line_number)); value = rb_str_with_debug_created_info(value, rb_iseq_path(iseq), line_number);
value = rb_str_dup(value);
rb_ivar_set(value, id_debug_created_info, rb_ary_freeze(debug_info));
rb_str_freeze(value);
} }
return value; return value;
@ -726,9 +723,7 @@ static VALUE
pm_static_literal_string(rb_iseq_t *iseq, VALUE string, int line_number) pm_static_literal_string(rb_iseq_t *iseq, VALUE string, int line_number)
{ {
if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) { if (ISEQ_COMPILE_DATA(iseq)->option->debug_frozen_string_literal || RTEST(ruby_debug)) {
VALUE debug_info = rb_ary_new_from_args(2, rb_iseq_path(iseq), INT2FIX(line_number)); return rb_str_with_debug_created_info(string, rb_iseq_path(iseq), line_number);
rb_ivar_set(string, id_debug_created_info, rb_ary_freeze(debug_info));
return rb_str_freeze(string);
} }
else { else {
return rb_fstring(string); return rb_fstring(string);

View File

@ -1,4 +1,3 @@
# frozen_string_literal: true
a = 'string' a = 'string'
b = a b = a
c = b c = b

View File

@ -57,9 +57,18 @@ end
describe "The --debug flag produces" do describe "The --debug flag produces" do
it "debugging info on attempted frozen string modification" do it "debugging info on attempted frozen string modification" do
error_str = ruby_exe(fixture(__FILE__, 'debug_info.rb'), options: '--debug', args: "2>&1") error_str = ruby_exe(fixture(__FILE__, 'debug_info.rb'), options: '--enable-frozen-string-literal --debug', args: "2>&1")
error_str.should include("can't modify frozen String") error_str.should include("can't modify frozen String")
error_str.should include("created at") error_str.should include("created at")
error_str.should include("command_line/fixtures/debug_info.rb:2") error_str.should include("command_line/fixtures/debug_info.rb:1")
end
guard -> { ruby_version_is "3.4" and !"test".frozen? } do
it "debugging info on mutating chilled string" do
error_str = ruby_exe(fixture(__FILE__, 'debug_info.rb'), options: '-w --debug', args: "2>&1")
error_str.should include("literal string will be frozen in the future")
error_str.should include("the string was created here")
error_str.should include("command_line/fixtures/debug_info.rb:1")
end
end end
end end

View File

@ -1811,22 +1811,37 @@ ec_str_alloc_heap(struct rb_execution_context_struct *ec, VALUE klass)
} }
static inline VALUE static inline VALUE
str_duplicate_setup(VALUE klass, VALUE str, VALUE dup) str_duplicate_setup_encoding(VALUE str, VALUE dup, VALUE flags)
{ {
const VALUE flag_mask =
ENC_CODERANGE_MASK | ENCODING_MASK |
FL_FREEZE
;
VALUE flags = FL_TEST_RAW(str, flag_mask);
int encidx = 0; int encidx = 0;
if (STR_EMBED_P(str)) { if ((flags & ENCODING_MASK) == (ENCODING_INLINE_MAX<<ENCODING_SHIFT)) {
encidx = rb_enc_get_index(str);
flags &= ~ENCODING_MASK;
}
FL_SET_RAW(dup, flags & ~FL_FREEZE);
if (encidx) rb_enc_associate_index(dup, encidx);
return dup;
}
static const VALUE flag_mask = ENC_CODERANGE_MASK | ENCODING_MASK | FL_FREEZE;
static inline VALUE
str_duplicate_setup_embed(VALUE klass, VALUE str, VALUE dup)
{
VALUE flags = FL_TEST_RAW(str, flag_mask);
long len = RSTRING_LEN(str); long len = RSTRING_LEN(str);
RUBY_ASSERT(STR_EMBED_P(dup)); RUBY_ASSERT(STR_EMBED_P(dup));
RUBY_ASSERT(str_embed_capa(dup) >= len + 1); RUBY_ASSERT(str_embed_capa(dup) >= len + 1);
MEMCPY(RSTRING(dup)->as.embed.ary, RSTRING(str)->as.embed.ary, char, len + 1); MEMCPY(RSTRING(dup)->as.embed.ary, RSTRING(str)->as.embed.ary, char, len + 1);
STR_SET_LEN(dup, RSTRING_LEN(str));
return str_duplicate_setup_encoding(str, dup, flags);
} }
else {
static inline VALUE
str_duplicate_setup_heap(VALUE klass, VALUE str, VALUE dup)
{
VALUE flags = FL_TEST_RAW(str, flag_mask);
VALUE root = str; VALUE root = str;
if (FL_TEST_RAW(str, STR_SHARED)) { if (FL_TEST_RAW(str, STR_SHARED)) {
root = RSTRING(str)->as.heap.aux.shared; root = RSTRING(str)->as.heap.aux.shared;
@ -1842,31 +1857,20 @@ str_duplicate_setup(VALUE klass, VALUE str, VALUE dup)
FL_SET(root, STR_SHARED_ROOT); FL_SET(root, STR_SHARED_ROOT);
RB_OBJ_WRITE(dup, &RSTRING(dup)->as.heap.aux.shared, root); RB_OBJ_WRITE(dup, &RSTRING(dup)->as.heap.aux.shared, root);
flags |= RSTRING_NOEMBED | STR_SHARED; flags |= RSTRING_NOEMBED | STR_SHARED;
}
STR_SET_LEN(dup, RSTRING_LEN(str)); STR_SET_LEN(dup, RSTRING_LEN(str));
return str_duplicate_setup_encoding(str, dup, flags);
if ((flags & ENCODING_MASK) == (ENCODING_INLINE_MAX<<ENCODING_SHIFT)) {
encidx = rb_enc_get_index(str);
flags &= ~ENCODING_MASK;
}
FL_SET_RAW(dup, flags & ~FL_FREEZE);
if (encidx) rb_enc_associate_index(dup, encidx);
return dup;
} }
static inline VALUE static inline VALUE
ec_str_duplicate(struct rb_execution_context_struct *ec, VALUE klass, VALUE str) str_duplicate_setup(VALUE klass, VALUE str, VALUE dup)
{ {
VALUE dup;
if (STR_EMBED_P(str)) { if (STR_EMBED_P(str)) {
dup = ec_str_alloc_embed(ec, klass, RSTRING_LEN(str) + TERM_LEN(str)); return str_duplicate_setup_embed(klass, str, dup);
} }
else { else {
dup = ec_str_alloc_heap(ec, klass); return str_duplicate_setup_heap(klass, str, dup);
} }
return str_duplicate_setup(klass, str, dup);
} }
static inline VALUE static inline VALUE
@ -1912,13 +1916,32 @@ VALUE
rb_ec_str_resurrect(struct rb_execution_context_struct *ec, VALUE str, bool chilled) rb_ec_str_resurrect(struct rb_execution_context_struct *ec, VALUE str, bool chilled)
{ {
RUBY_DTRACE_CREATE_HOOK(STRING, RSTRING_LEN(str)); RUBY_DTRACE_CREATE_HOOK(STRING, RSTRING_LEN(str));
VALUE new_str = ec_str_duplicate(ec, rb_cString, str); VALUE new_str, klass = rb_cString;
if (!(chilled && RTEST(rb_ivar_defined(str, id_debug_created_info))) && STR_EMBED_P(str)) {
new_str = ec_str_alloc_embed(ec, klass, RSTRING_LEN(str) + TERM_LEN(str));
str_duplicate_setup_embed(klass, str, new_str);
}
else {
new_str = ec_str_alloc_heap(ec, klass);
str_duplicate_setup_heap(klass, str, new_str);
}
if (chilled) { if (chilled) {
STR_CHILL_RAW(new_str); STR_CHILL_RAW(new_str);
} }
return new_str; return new_str;
} }
VALUE
rb_str_with_debug_created_info(VALUE str, VALUE path, int line)
{
VALUE debug_info = rb_ary_new_from_args(2, path, INT2FIX(line));
if (OBJ_FROZEN_RAW(str)) str = rb_str_dup(str);
rb_ivar_set(str, id_debug_created_info, rb_ary_freeze(debug_info));
STR_CHILL_RAW(str);
return rb_str_freeze(str);
}
/* /*
* *
* call-seq: * call-seq:

View File

@ -1262,9 +1262,8 @@ class TestRubyOptions < Test::Unit::TestCase
code = <<~RUBY code = <<~RUBY
"foo" << "bar" "foo" << "bar"
RUBY RUBY
warning = ["-:1: warning: literal string will be frozen in the future"] assert_in_out_err(["-W:deprecated"], code, [], ["-:1: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information)"])
assert_in_out_err(["-W:deprecated"], code, [], warning) assert_in_out_err(["-W:deprecated", "--debug-frozen-string-literal"], code, [], ["-:1: warning: literal string will be frozen in the future", "-:1: info: the string was created here"])
assert_in_out_err(["-W:deprecated", "--debug-frozen-string-literal"], code, [], warning)
assert_in_out_err(["-W:deprecated", "--disable-frozen-string-literal", "--debug-frozen-string-literal"], code, [], []) assert_in_out_err(["-W:deprecated", "--disable-frozen-string-literal", "--debug-frozen-string-literal"], code, [], [])
assert_in_out_err(["-W:deprecated", "--enable-frozen-string-literal", "--debug-frozen-string-literal"], code, [], ["-:1:in '<main>': can't modify frozen String: \"foo\", created at -:1 (FrozenError)"]) assert_in_out_err(["-W:deprecated", "--enable-frozen-string-literal", "--debug-frozen-string-literal"], code, [], ["-:1:in '<main>': can't modify frozen String: \"foo\", created at -:1 (FrozenError)"])
end end