std.process: memory allocation with malloc in execv_
Steven Schveighoffer
schveiguy at gmail.com
Thu Jan 26 14:45:29 UTC 2023
On 1/26/23 8:32 AM, kdevel wrote:
> Why is the memory in `execv_` in `std.process` allocated with `malloc`
> (`core.stdc.stdlib.malloc`) and why is this memory not registered with
> the GC? It seems to be responsible for the memory corruption in [1].
>
> ```
> diff --git a/process.d b/process.d
> index 3eaa283..cea45a9 100644
> --- a/process.d
> +++ b/process.d
> @@ -4295,7 +4295,9 @@ private int execvp_(in string pathname, in
> string[] argv)
> import std.exception : enforce;
> auto argv_ =
> cast(const(char)**)core.stdc.stdlib.malloc((char*).sizeof * (1 +
> argv.length));
> enforce!OutOfMemoryError(argv_ !is null, "Out of memory in
> std.process.");
> - scope(exit) core.stdc.stdlib.free(argv_);
> + import core.memory;
> + GC.addRange (argv_, (char *).sizeof * (1 + argv.length));
> + scope(exit) { GC.removeRange (argv_); core.stdc.stdlib.free(argv_); }
>
> toAStringz(argv, argv_);
> ```
>
> [1] https://issues.dlang.org/show_bug.cgi?id=23654
> Issue 23654 - execv_: toAStringz: memory corruption
Yep, that's 100% the problem. Introduced here:
https://github.com/dlang/phobos/commit/6302257b0cdc5d171511cc6f1566956ff11b09c5
Amazing it's been broken like this for 7 years!
I don't like the solution of adding the range, or using the GC. A better
option is to replace toAStringz with a function that creates everything
for you into a type (toStringz isn't complex, just replace with one that
uses malloc), that then automatically frees everything.
-Steve
More information about the Digitalmars-d
mailing list