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