openplanet-nl / issues

Issue tracker for Openplanet.
10 stars 0 forks source link

String concat with itself is undefined behavior #585

Open nan-inf opened 2 weeks ago

nan-inf commented 2 weeks ago

The game crashes with a "Crash detected in Openplanet.dll" dialog box when in-place appending a large string. This can be triggered with the following code, which repeatedly appends itself to itself:

string temp = "a";
for (uint i = 1; i <= 30; ++i) {
    temp += temp;
    print(temp.Length);
    yield();
}

On my system, this crash happens when temp has a length of 524288 (2^19).

On the other hand, concatenating and then assigning to the variable works fine all the way up to 2^30 (1 GiB) (I haven't tested for larger strings):

string temp = "a";
for (uint i = 1; i <= 30; ++i) {
    temp = temp + temp;  // <-- changed
    print(temp.Length);
    yield();
}
codecat commented 1 week ago

This is mostly intentional and I don't really plan to change this without a good reason for it.

In your example, temp += temp modifies temp on the left side and as a result also (maybe unexpectedly) on the right side during the string concat. This means that there is a chance we end up with undefined behavior. We can fix this by making a copy of the string, but obviously this is a change that brings a definite performance hit.

If you do temp + temp, an intermediary string is created and assigned to temp only after the 2 strings are concatted, so no buffer is changed.

What is your use case for appending a string to itself?

nan-inf commented 6 days ago

My use case was completely academic; I needed a large string for testing the runtime (and its scaling behavior) of unrelated code on large strings, this was the first method I came up with for generating one, and I was surprised by its behavior. Of course, temp = temp + temp would work equally well in that case.