YJIT: Protect strings from GC on String#<<

[k0kubun]

Fix Shopify/yjit#310

[Bug #19483]

rb_str_buf_append may allocate when a buffer needs to be reallocated. At that point, string objects could be freed and random bytes could be written to the string content. We need gen_save_sp to let GC scan and mark those objects.

[machty]

Maybe a noob question but shouldn't a bugfix like this with a somewhat esoteric fix have a breaking test?

[maximecb]

Maybe a noob question but shouldn't a bugfix like this with a somewhat esoteric fix have a breaking test?

That's a fair point. We'll accept a PR if you can find one :)

It would involve triggering GC during string concat, maybe with a very large string, or lots of repeated concatenation.

[k0kubun]

Actually I tried to write one:

original = (('a'..'z').to_a * 100).join

buf0 = ''
buf1 = ''
GC.stress = true
original.size.times do |i|
  buf0 << original[i]
  buf1 << original[i]
end

p(original == buf0)
p(original == buf1)

and failed. You probably need to find an operation that are likely to (not just could) overwrite freed space with random bytes, but I can't think of one right now.

It's possible to copy original code of rubylib even while it's a gem, but it was too rare to reproduce even with the original code in a few iterations. So we need to come up with something else.

[k0kubun]

Hmm, but actually, let me try your code rubyzip/rubyzip#550 first. I've overlooked that PR.

[k0kubun]

Ah I thought you meant "non-indeterministic" but it was actually "non-deterministic". I tried it but it doesn't seem to reproduce the issue with a few attempts. We need something else.

[machty]

Yes, I failed to provide a repro -- unfortunately i ate through my budget of investigating this but perhaps the reason it happens on my machine / CI is that it's running in a rake task of a large rails app with the Rails environment fully loaded with much higher memory pressure. Not sure the best way to simulate that.