Bug #13223
closed
`File.join` will segv if File::SEPARATOR and File::Separator are set.
Added by tenderlovemaking (Aaron Patterson) over 7 years ago.
Updated over 7 years ago.
Description
The program below will crash with a segv:
File.const_set :Separator, "hello"
File.const_set :SEPARATOR, "hello"
GC.start
File.join "hello", "world"
This is because the separator object is referenced from a global variable and setting the constants allows the object to be GC'd and the global will go bad.
I've attached a patch that contains a test to demonstrate the problem along with a fix.
Files
- Description updated (diff)
I doubt that the tests are expected results.
And a similar pattern:
$; = " "
$a = nil
alias $; $a
alias $-F $a
GC.start
"".split
@nobu (Nobuyoshi Nakada) I went by what the documentation says:
/*
* call-seq:
* File.join(string, ...) -> string
*
* Returns a new string formed by joining the strings using
* <code>File::SEPARATOR</code>.
*
* File.join("usr", "mail", "gumby") #=> "usr/mail/gumby"
*
*/
The current implementation doesn't seem to care what the value of File::SEPARATOR
is, so I think it's a bug. Also JRuby has this behavior:
$ jruby -v -e'File.const_set(:SEPARATOR, "$"); p File.join("a","b")'
jruby 9.1.6.0 (2.3.1) 2016-11-09 0150a76 Java HotSpot(TM) 64-Bit Server VM 25.77-b03 on 1.8.0_77-b03 +jit [darwin-x86_64]
-e:1: warning: already initialized constant SEPARATOR
"a$b"
- Status changed from Open to Closed
Applied in changeset r57960.
file.c: join with /
- Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: UNKNOWN to 2.2: REQUIRED, 2.3: REQUIRED, 2.4: REQUIRED
- Backport changed from 2.2: REQUIRED, 2.3: REQUIRED, 2.4: REQUIRED to 2.2: DONTNEED, 2.3: DONTNEED, 2.4: DONTNEED
Ugh, I have marked this as backport required for r57958, not r57960.
I'll create another ticket for r57958.
Nobu already made changes but, yes we looked at this issue in today's developer meeting.
File::SEPARATOR has been there, but not always used. For instance File.split does not honor that constant. JRuby is well-doing to carefully read our document but, we agreed that this is not somewhere we have to hustle.
Let us define File::SEPARATOR to be the default value of the file path separator; redefinition of it may or may not affect further invokations of methods, depending on implementations.
shyouhei (Shyouhei Urabe) wrote:
Nobu already made changes but, yes we looked at this issue in today's developer meeting.
File::SEPARATOR has been there, but not always used. For instance File.split does not honor that constant. JRuby is well-doing to carefully read our document but, we agreed that this is not somewhere we have to hustle.
Let us define File::SEPARATOR to be the default value of the file path separator; redefinition of it may or may not affect further invokations of methods, depending on implementations.
It is much better to not leave semantics to be implementation-defined, as it's essentially the same as undefined behavior and C people know how bad that is.
So I think we should settle to only one of the two possibilities here.
Otherwise any program using File.join and modifying File::SEPARATOR will produce different results based on the Ruby implementation, which is inacceptable for such a method.
I'd personally like to vote for ignoring redefinitions of constants. In general redefinitions prevent optimizations. That doesn't make me happy.
shyouhei (Shyouhei Urabe) wrote:
I'd personally like to vote for ignoring redefinitions of constants. In general redefinitions prevent optimizations. That doesn't make me happy.
I agree in that it is good to not change basic functionality such as File.join based on some global state.
Redefinition of constants is part of Ruby though.
With a JIT, the assumption that constants do not change can be embedded with the existence of the JIT'd code itself,
and deoptimize if the value change. Therefore redefinition of constants has no cost in compiled code, unless they are frequently changed.
Overall I think these decisions should not be based only on performance concerns,
especially in this case where the performance of File.join is likely insignificantly changed by looking up the constant.
Also available in: Atom
PDF
Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0