From: "Eregon (Benoit Daloze) via ruby-core" <ruby-core@ml.ruby-lang.org>
To: ruby-core@ml.ruby-lang.org
Cc: "Eregon (Benoit Daloze)" <noreply@ruby-lang.org>
Subject: [ruby-core:120583] [Ruby master Bug#20998] rb_str_locktmp() changes flags of frozen strings and string literals
Date: Thu, 09 Jan 2025 16:14:35 +0000 (UTC) [thread overview]
Message-ID: <redmine.journal-111408.20250109161433.772@ruby-lang.org> (raw)
In-Reply-To: <redmine.issue-20998.20250103105118.772@ruby-lang.org>
Issue #20998 has been updated by Eregon (Benoit Daloze).
@nobu @byroot So do you think rb_str_locktmp() should noop on frozen strings (since they can't change anyway), or it should raise because the usage seems somewhat dubious?
I'm fine with either, I'd just like a decision so we can do the same in TruffleRuby.
(the current status is broken so that's worse than either fix)
----------------------------------------
Bug #20998: rb_str_locktmp() changes flags of frozen strings and string literals
https://bugs.ruby-lang.org/issues/20998#change-111408
* Author: Eregon (Benoit Daloze)
* Status: Open
* ruby -v: ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN
----------------------------------------
```ruby
# frozen_string_literal: true
# BOILERPLATE START
require 'tmpdir'
require 'rbconfig'
def inline_c_extension(c_code)
Dir.mktmpdir('inline_c_extension') do |dir|
File.write("#{dir}/cext.c", c_code)
File.write("#{dir}/extconf.rb", <<~RUBY)
require 'mkmf'
create_makefile('cext')
RUBY
out = IO.popen([RbConfig.ruby, 'extconf.rb'], chdir: dir, &:read)
raise "ruby extconf.rb failed: #{$?.inspect}\n#{out}" unless $?.success?
out = IO.popen(['make'], chdir: dir, &:read)
raise "make failed: #{$?.inspect}\n#{out}" unless $?.success?
require "#{dir}/cext.#{RbConfig::CONFIG['DLEXT']}"
end
end
inline_c_extension <<~C
#include "ruby.h"
static VALUE foo(VALUE self, VALUE str) {
rb_str_locktmp(str);
return str;
}
void Init_cext(void) {
VALUE c = rb_define_class("Foo", rb_cObject);
rb_define_singleton_method(c, "foo", foo, 1);
}
C
# BOILERPLATE END
a = "str"
Foo.foo(a)
# imagine a million lines of code in between
b = "str"
b << "."
# can't modify string; temporarily locked (RuntimeError)
# What? Who "locked" that immutable frozen string literal?
# It should be: can't modify frozen String: "str" (FrozenError)
```
Same problem with:
```ruby
Foo.foo("abc")
# imagine a million lines of code in between
Foo.foo("abc") # temporal locking already locked string (RuntimeError)
```
Related: https://github.com/oracle/truffleruby/issues/3752
It seems a clear bug to mutate a frozen string (with visible side effects), even more so for shared frozen string literals.
I think rb_str_locktmp() should raise (a FrozenError, as it's effectively attempting to mutate it, same as calling `rb_str_modify()`) if called on a frozen string, because it makes little sense, I think "locking a string" only makes sense for mutable strings.
The alternative would be to noop on frozen strings for rb_str_locktmp() and rb_str_unlocktmp(), I think that's susprising, and potentially hiding more bugs, e.g. if one by mistake tries to mutate the RSTRING_PTR() or so, and also rb_str_locktmp() wouldn't imply the flag is set after it returns.
Any attempt to mutate a frozen string should fail, so might as well fail early.
--
https://bugs.ruby-lang.org/
______________________________________________
ruby-core mailing list -- ruby-core@ml.ruby-lang.org
To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/
next prev parent reply other threads:[~2025-01-09 16:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-03 10:51 [ruby-core:120465] " Eregon (Benoit Daloze) via ruby-core
2025-01-06 2:21 ` [ruby-core:120493] " nobu (Nobuyoshi Nakada) via ruby-core
2025-01-06 9:34 ` [ruby-core:120495] " Eregon (Benoit Daloze) via ruby-core
2025-01-06 9:56 ` [ruby-core:120497] " byroot (Jean Boussier) via ruby-core
2025-01-06 12:37 ` [ruby-core:120499] " Eregon (Benoit Daloze) via ruby-core
2025-01-06 12:45 ` [ruby-core:120500] " Eregon (Benoit Daloze) via ruby-core
2025-01-07 10:36 ` [ruby-core:120520] " byroot (Jean Boussier) via ruby-core
2025-01-09 16:14 ` Eregon (Benoit Daloze) via ruby-core [this message]
2025-01-09 17:00 ` [ruby-core:120585] " byroot (Jean Boussier) via ruby-core
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=redmine.journal-111408.20250109161433.772@ruby-lang.org \
--to=ruby-core@ml.ruby-lang.org \
--cc=noreply@ruby-lang.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).