schveiguy / io

Core I/O functionality
Boost Software License 1.0
25 stars 9 forks source link

String.clone is unsafe #39

Open schveiguy opened 4 years ago

schveiguy commented 4 years ago

Current implementation of String.clone does not duplicate the string, it simply sets a new reference. If the original string is freed or reallocated, the new clone is now dangling.

schveiguy commented 4 years ago

ping @MartinNowak can you identify what the purpose of the internal String type is for? I understand you want to be @nogc, but what is there seems to be totally memory unsafe. I.e. I can slice the string in @safe code, and then it can be freed and I'm left with a dangling pointer:

@safe unittest {
   const(char)[] dangling;
   {
      String s1;
      s1 ~= "hello";
      dangling = s1[];
   }
   writeln(dangling); // uses now-freed memory
}

Edit: is this supposed to be depending on dip1000 to ensure it doesn't compile? I can't get the unittests to build with -dip1000, so maybe that is the intention.

But, reallocation via appending could easily free the original, so there is still that problem.

MartinNowak commented 4 years ago

Indeed there was some talk about implementing safe nogc strings at that time. So I tried how far scope would work. It had many deficiencies at that time. Nonetheless this is an internal type that gets us from having a path argument to open a file handle, so it doesn't really matter and gets the job done. You sure could just push the trusted from this internal type towards those constructors.

schveiguy commented 4 years ago

I was more thinking about just using more allocations to avoid memory issues. But I wasn't 100% sure what the different constructors were intended for.