Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

String.clone is unsafe #39

Open
schveiguy opened this issue May 1, 2020 · 3 comments
Open

String.clone is unsafe #39

schveiguy opened this issue May 1, 2020 · 3 comments

Comments

@schveiguy
Copy link
Owner

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
Copy link
Owner Author

schveiguy commented May 1, 2020

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
Copy link
Collaborator

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
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants