Issue #19315 has been updated by rhenium (Kazuki Yamaguchi). Eregon (Benoit Daloze) wrote in #note-22:
What's the problem? Using `RSTRING_END()` means one is aware of null bytes in the middle and does not assume null-terminated. Is it because of this case? ```c char* end = RSTRING_END(str); char* ptr = RSTRING_PTR(str); ```
I think it would also be a problem in `some_function_call(RSTRING_PTR(str), RSTRING_END(str))`, where the evaluation order is not guaranteed. --- himura467 (Akito Shitara) wrote in #note-18:
To ensure null-termination for shared middle substrings, `RSTRING_PTR()` now calls `str_make_independent_expand`.
I wonder if `RSTRING_PTR()` could instead allocate a read-only copy dedicated for `RSTRING_PTR()`, rather than permanently converting the String away from `STR_SHARED`. I think the current patch in <https://github.com/ruby/ruby/pull/17045> introduces an unfortunate corner case that could be difficult to reproduce or debug: ```c VALUE shared_root = rb_str_freeze(rb_str_new(NULL, 100)); VALUE str = rb_str_subseq(shared_root, 0, 50); // STR_SHARED rb_str_freeze(str); shared_root = Qnil; // Forget it char *ptr; long len; RSTRING_GETMEM(str, ptr, len); // ptr = RSTRING_RAW_PTR(str); // This is legal if `str` is frozen or `rb_str_locktmp()`'ed, but would become unsafe with the PR. // // I also suspect there are carelessly written core/extension methods that accidentally allow // Ruby code to run after taking a pointer from a non-`rb_str_locktmp()`'ed mutable string. // Such code is unsafe even without the PR, but may happen to work in practice unless someone // actively tries to break it by modifying the string content. { // Converts `str` into an independent string, making `shared_root` unreachable from GC roots (void)RSTRING_PTR(str); rb_gc(); } // `ptr` may no longer be valid at this point. printf("%.*s", (int)len, ptr); ``` This also means `RSTRING_RAW_PTR()` is slightly harder to use safely, even when a NUL terminator is not required. For example, in ruby/openssl, there is code that needs a pointer that survives across execution of arbitrary Ruby code: https://github.com/ruby/openssl/blob/c7cdca78f5ee19576113d6d600fd22650f0e37c... I think it's also worth noting that some users of `RSTRING_PTR()` assume it never fail and use it where longjmp isn't allowed. However, since allocation failure should be rare in normal circumstances, this may be an acceptable incompatibility. ---------------------------------------- Feature #19315: Lazy substrings in CRuby https://bugs.ruby-lang.org/issues/19315#change-117460 * Author: Eregon (Benoit Daloze) * Status: Open ---------------------------------------- CRuby should implement lazy substrings, i.e., "abcdef"[1..3] must not copy bytes. Currently CRuby only reuse the char* if the substring is until the end of the buffer. But it should also work wherever the substring starts and ends. Yes, it means RSTRING_PTR() might need to allocate to \0-terminate, so be it, it's worth it. There is already code for this (`SHARABLE_MIDDLE_SUBSTRING`), but it's disabled by default and `RSTRING_PTR()` needs to be changed to deal with this. It seems a good idea to introduce a variant of `RSTRING_PTR` which doesn't guarantee \0-termination, so such callers can then use the existing bytes always without copy. There are countless workarounds for this missing optimization, all not worth it with lazy substring and all less readable: * https://bugs.ruby-lang.org/issues/19314 * https://bugs.ruby-lang.org/issues/18598#note-3 * https://github.com/ruby/net-protocol/pull/14 * Manual lazy substrings which track string + index + length * More but I don't remember all now, feel free to comment or link more urls/tickets. -- https://bugs.ruby-lang.org/