From b919c83688c96a2bed628b55bc6a8ac36839c387 Mon Sep 17 00:00:00 2001 From: Jonathan Marler Date: Mon, 5 Aug 2019 15:56:43 -0600 Subject: [PATCH] Maintain same environment for dmd -run Fix issue 18729 --- src/dmd/dinifile.d | 15 ++---- src/dmd/env.d | 77 ++++++++++++++++++++++++++++++ src/dmd/link.d | 27 +++++------ src/posix.mak | 2 +- src/win32.mak | 2 +- test/dshell/extra-files/printenv.d | 11 +++++ test/dshell/sameenv.d | 23 +++++++++ 7 files changed, 128 insertions(+), 29 deletions(-) create mode 100644 src/dmd/env.d create mode 100644 test/dshell/extra-files/printenv.d create mode 100644 test/dshell/sameenv.d diff --git a/src/dmd/dinifile.d b/src/dmd/dinifile.d index b9214e98d2c7..23ce1676622a 100644 --- a/src/dmd/dinifile.d +++ b/src/dmd/dinifile.d @@ -19,6 +19,7 @@ import core.sys.posix.stdlib; import core.sys.windows.winbase; import core.sys.windows.windef; +import dmd.env; import dmd.errors; import dmd.globals; import dmd.root.rmem; @@ -28,7 +29,6 @@ import dmd.root.port; import dmd.root.stringtable; import dmd.utils; -version (Windows) extern (C) int putenv(const char*) nothrow; private enum LOG = false; /***************************** @@ -145,20 +145,11 @@ void updateRealEnvironment(ref StringTable environment) static int envput(const(StringValue)* sv) nothrow { const name = sv.toDchars(); - const namelen = strlen(name); const value = cast(const(char)*)sv.ptrvalue; if (!value) // deleted? return 0; - const valuelen = strlen(value); - auto s = cast(char*)malloc(namelen + 1 + valuelen + 1); - if (!s) - Mem.error(); - memcpy(s, name, namelen); - s[namelen] = '='; - memcpy(s + namelen + 1, value, valuelen); - s[namelen + 1 + valuelen] = 0; - //printf("envput('%s')\n", s); - putenv(s); + if (putenvRestorable(name.toDString, value.toDString)) + assert(0); return 0; // do all of them } diff --git a/src/dmd/env.d b/src/dmd/env.d new file mode 100644 index 000000000000..e0db6662f0c1 --- /dev/null +++ b/src/dmd/env.d @@ -0,0 +1,77 @@ +module dmd.env; + +import core.stdc.string; +import core.sys.posix.stdlib; +import dmd.globals; +import dmd.root.array; +import dmd.root.rmem; +import dmd.utils; + +version (Windows) + private extern (C) int putenv(const char*) nothrow; + +/** +Construct a variable from `name` and `value` and put it in the environment while saving +the previous value of the environment variable into a global list so it can be restored later. +Params: + name = the name of the variable + value = the value of the variable +Returns: + true on error, false on success +*/ +bool putenvRestorable(const(char)[] name, const(char)[] value) nothrow +{ + saveEnvVar(name); + const nameValue = allocNameValue(name, value); + const result = putenv(cast(char*)nameValue.ptr); + version (Windows) + mem.xfree(cast(void*)nameValue.ptr); + else + { + if (result) + mem.xfree(cast(void*)nameValue.ptr); + } + return result ? true : false; +} + +/** +Allocate a new variable via xmalloc that can be added to the global environment. The +resulting string will be null-terminated immediately after the end of the array. +Params: + name = name of the variable + value = value of the variable +Returns: + a newly allocated variable that can be added to the global environment +*/ +string allocNameValue(const(char)[] name, const(char)[] value) nothrow +{ + const length = name.length + 1 + value.length; + auto str = (cast(char*)mem.xmalloc(length + 1))[0 .. length]; + str[0 .. name.length] = name[]; + str[name.length] = '='; + str[name.length + 1 .. length] = value[]; + str.ptr[length] = '\0'; + return cast(string)str; +} + +/// Holds the original values of environment variables when they are overwritten. +private __gshared string[string] envNameValues; + +/// Restore the original environment. +void restoreEnvVars() +{ + foreach (var; envNameValues.values) + { + if (putenv(cast(char*)var.ptr)) + assert(0); + } +} + +/// Save the environment variable `name` if not saved already. +void saveEnvVar(const(char)[] name) nothrow +{ + if (!(name in envNameValues)) + { + envNameValues[name.idup] = allocNameValue(name, name.toCStringThen!(n => getenv(n.ptr)).toDString); + } +} diff --git a/src/dmd/link.d b/src/dmd/link.d index 17fcff233fbf..2f2c60a3c899 100644 --- a/src/dmd/link.d +++ b/src/dmd/link.d @@ -21,6 +21,7 @@ import core.sys.posix.unistd; import core.sys.windows.winbase; import core.sys.windows.windef; import core.sys.windows.winreg; +import dmd.env; import dmd.errors; import dmd.globals; import dmd.root.file; @@ -30,7 +31,6 @@ import dmd.root.rmem; import dmd.utils; version (Posix) extern (C) int pipe(int*); -version (Windows) extern (C) int putenv(const char*); version (Windows) extern (C) int spawnlp(int, const char*, const char*, const char*, const char*); version (Windows) extern (C) int spawnl(int, const char*, const char*, const char*, const char*); version (Windows) extern (C) int spawnv(int, const char*, const char**); @@ -769,17 +769,11 @@ version (Windows) { if ((len = strlen(args)) > 255) { - char* q = cast(char*)alloca(8 + len + 1); - sprintf(q, "_CMDLINE=%s", args); - status = putenv(q); + status = putenvRestorable("_CMDLINE", args[0 .. len]); if (status == 0) - { args = "@_CMDLINE"; - } else - { error(Loc.initial, "command line length of %d is too long", len); - } } } // Normalize executable path separators @@ -897,6 +891,7 @@ public int runProgram() argv.push(a); } argv.push(null); + restoreEnvVars(); version (Windows) { const(char)[] ex = FileName.name(global.params.exefile); @@ -1053,16 +1048,18 @@ version (Windows) { // debug info needs DLLs from $(VSInstallDir)\Common7\IDE for most linker versions // so prepend it too the PATH environment variable - const char* path = getenv("PATH"); + const path = getenv("PATH"); const pathlen = strlen(path); const addpathlen = strlen(addpath); - char* npath = cast(char*)mem.xmalloc(5 + pathlen + 1 + addpathlen + 1); - memcpy(npath, "PATH=".ptr, 5); - memcpy(npath + 5, addpath, addpathlen); - npath[5 + addpathlen] = ';'; - memcpy(npath + 5 + addpathlen + 1, path, pathlen + 1); - putenv(npath); + const length = addpathlen + 1 + pathlen; + char* npath = cast(char*)mem.xmalloc(length); + memcpy(npath, addpath, addpathlen); + npath[addpathlen] = ';'; + memcpy(npath + addpathlen + 1, path, pathlen); + if (putenvRestorable("PATH", npath[0 .. length])) + assert(0); + mem.xfree(npath); } return cmdbuf.extractChars(); } diff --git a/src/posix.mak b/src/posix.mak index 7a36d81d911a..7222e133a44f 100644 --- a/src/posix.mak +++ b/src/posix.mak @@ -317,7 +317,7 @@ FRONT_SRCS=$(addsuffix .d, $(addprefix $D/,access aggregate aliasthis apply argt arraytypes astcodegen ast_node attrib builtin canthrow cli clone compiler complex cond constfold \ cppmangle cppmanglewin ctfeexpr ctorflow dcast dclass declaration delegatize denum dimport \ dinifile dinterpret dmacro dmangle dmodule doc dscope dstruct dsymbol dsymbolsem \ - dtemplate dversion escape expression expressionsem func \ + dtemplate dversion env escape expression expressionsem func \ hdrgen id impcnvtab imphint init initsem inline inlinecost intrange \ json lambdacomp lib libelf libmach link mars mtype nogc nspace objc opover optimize parse permissivevisitor sapply templateparamsem \ sideeffect statement staticassert target typesem traits transitivevisitor parsetimevisitor visitor \ diff --git a/src/win32.mak b/src/win32.mak index ccd2d542445e..fdb51a9dccc8 100644 --- a/src/win32.mak +++ b/src/win32.mak @@ -139,7 +139,7 @@ FRONT_SRCS=$D/access.d $D/aggregate.d $D/aliasthis.d $D/apply.d $D/argtypes.d $D $D/cond.d $D/constfold.d $D/cppmangle.d $D/cppmanglewin.d $D/ctfeexpr.d $D/ctorflow.d $D/dcast.d $D/dclass.d \ $D/declaration.d $D/delegatize.d $D/denum.d $D/dimport.d $D/dinifile.d $D/dinterpret.d \ $D/dmacro.d $D/dmangle.d $D/dmodule.d $D/doc.d $D/dscope.d $D/dstruct.d $D/dsymbol.d $D/dsymbolsem.d \ - $D/lambdacomp.d $D/dtemplate.d $D/dversion.d $D/escape.d \ + $D/lambdacomp.d $D/dtemplate.d $D/dversion.d $D/env.d $D/escape.d \ $D/expression.d $D/expressionsem.d $D/func.d $D/hdrgen.d $D/id.d $D/imphint.d \ $D/impcnvtab.d $D/init.d $D/initsem.d $D/inline.d $D/inlinecost.d $D/intrange.d $D/json.d $D/lib.d $D/link.d \ $D/mars.d $D/mtype.d $D/nogc.d $D/nspace.d $D/objc.d $D/opover.d $D/optimize.d $D/parse.d \ diff --git a/test/dshell/extra-files/printenv.d b/test/dshell/extra-files/printenv.d new file mode 100644 index 000000000000..b8536d200c05 --- /dev/null +++ b/test/dshell/extra-files/printenv.d @@ -0,0 +1,11 @@ +import std.array, std.stdio, std.process, std.algorithm; +void main() +{ + foreach (varPair; environment.toAA().byKeyValue.array.sort!"a.key < b.key") + { + if (varPair.key != "_") + { + writeln(varPair.key, "=", varPair.value); + } + } +} diff --git a/test/dshell/sameenv.d b/test/dshell/sameenv.d new file mode 100644 index 000000000000..a00d78d3136f --- /dev/null +++ b/test/dshell/sameenv.d @@ -0,0 +1,23 @@ +import dshell; +void main() +{ + const envFromExe = shellExpand("$OUTPUT_BASE/envFromExe.txt"); + const envFromRun = shellExpand("$OUTPUT_BASE/envFromRun.txt"); + + run("$DMD -m$MODEL -of$OUTPUT_BASE/printenv$EXE $EXTRA_FILES/printenv.d"); + run("$OUTPUT_BASE/printenv$EXE", File(envFromExe, "wb")); + run("$DMD -m$MODEL -run $EXTRA_FILES/printenv.d", File(envFromRun, "wb")); + + const fromExe = readText(envFromExe); + const fromRun = readText(envFromRun); + if (fromExe != fromRun) + { + writefln("FromExe:"); + writeln("-----------"); + writeln(fromExe); + writefln("FromRun:"); + writeln("-----------"); + writeln(fromRun); + assert(0, "output from exe/run differ"); + } +}