From 6f6b22016ad2eb5016c1c63bfa0b1941b51f310d Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Fri, 12 Feb 2021 23:59:57 +0000 Subject: [PATCH] command -x: tweak args list size detection (re: 9ddb45b1) src/cmd/ksh93/features/externs: ARG_EXTRA_BYTES detection: - Improve detection of extra bytes per argument: on every loop iteration, recalculate the size of the environment while taking the amount extra bytes we're currently trying into account. Also count arguments (argv[]) as they are stored in the same buffer. On 64-bit Linux with glibc, this now detects 9 extra bytes per argument instead of 8. An odd number (literally and figuratively) but apparently it needs it; I do think my method is correct now. On 64-bit Solaris and macOS, this still detects 8 extra bytes. (On 64-bit Linux with musl C library, it detects 0 bytes. Nice.) src/cmd/ksh93/sh/path.c: path_xargs(): - Remove the kludge subtracting twice the size of the environment. With the feature test fixed, this should no longer fail on Linux. - Take into account the size of the final null element in the argument and environment lists. src/cmd/ksh93/tests/path.sh: - Do not use awk for the test due to breakage in the system awks on Solaris/Illumos (hangs) and AIX & UnixWare (drops arguments). Instead, use (wait for it...) ksh. It's a bit slower, but works. --- src/cmd/ksh93/features/externs | 29 +++++++++++++++++------------ src/cmd/ksh93/sh/path.c | 3 ++- src/cmd/ksh93/tests/path.sh | 21 ++++++++++++--------- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/cmd/ksh93/features/externs b/src/cmd/ksh93/features/externs index 8f91456c0..ee0e16463 100644 --- a/src/cmd/ksh93/features/externs +++ b/src/cmd/ksh93/features/externs @@ -17,8 +17,8 @@ tst note{ determining extra bytes per argument for arguments list }end output{ * Figure out if this OS requires extra bytes per argument * in the arguments list of a process. * Outputs an appropriate #define ARG_EXTRA_BYTES. - * Without this, 'command -x' failed with E2BIG on macOS and Linux even - * if all the arguments should fit in ARG_MAX based on their length. + * Without this, 'command -x' failed with E2BIG on macOS, Linux and Solaris + * even if all the arguments should fit in ARG_MAX based on their length. */ /* AST includes */ @@ -43,20 +43,25 @@ tst note{ determining extra bytes per argument for arguments list }end output{ int main(int argc,char *argv[]) { - int extra_bytes = 0, envlen = 0, argmax, i; + int extra_bytes = 0, envlen, argmax, i; pid_t childpid; error_info.id="ARG_EXTRA_BYTES test (parent)"; - for(i=0; environ[i]; i++) - envlen += strlen(environ[i]) + 1; - argmax = strtoimax(astconf("ARG_MAX",NiL,NiL),NiL,0) - 2 * envlen; - if (argmax < 2048) - { - error(ERROR_ERROR|2, "argmax too small"); - return 1; - } while(1) { + envlen = 0; + for(i=0; argv[i]; i++) + envlen += strlen(argv[i]) + 1 + extra_bytes; + envlen += 1 + extra_bytes; /* final null element */ + for(i=0; environ[i]; i++) + envlen += strlen(environ[i]) + 1 + extra_bytes; + envlen += 1 + extra_bytes; /* final null element */ + argmax = strtoimax(astconf("ARG_MAX",NiL,NiL),NiL,0) - envlen; + if (argmax < 2048) + { + error(ERROR_ERROR|2, "argmax too small"); + return 1; + } if(!(childpid = fork())) { /* child */ @@ -122,5 +127,5 @@ tst note{ determining extra bytes per argument for arguments list }end output{ return 0; } }end fail{ - echo "#define ARG_EXTRA_BYTES 8 /* BUG: test failed; assuming 8 */" + echo "#define ARG_EXTRA_BYTES 16 /* BUG: test failed; assuming 16 */" }end diff --git a/src/cmd/ksh93/sh/path.c b/src/cmd/ksh93/sh/path.c index 5e950eda4..26130b87d 100644 --- a/src/cmd/ksh93/sh/path.c +++ b/src/cmd/ksh93/sh/path.c @@ -164,11 +164,12 @@ static pid_t path_xargs(Shell_t *shp,const char *path, char *argv[],char *const return((pid_t)-1); size = shp->gd->lim.arg_max - (ARG_EXTRA_BYTES > 2 ? 1024*ARG_EXTRA_BYTES : 2048); for(ev=envp; cp= *ev; ev++) - size -= 2 * (strlen(cp) + 1 + ARG_EXTRA_BYTES); + size -= strlen(cp) + 1 + ARG_EXTRA_BYTES; for(av=argv; (cp= *av) && av< &argv[shp->xargmin]; av++) size -= strlen(cp) + 1 + ARG_EXTRA_BYTES; for(av=avlast; cp= *av; av++,nlast++) size -= strlen(cp) + 1 + ARG_EXTRA_BYTES; + size -= 2 + 2 * ARG_EXTRA_BYTES; /* final null env and arg elements */ av = &argv[shp->xargmin]; if(!spawn) job_clear(); diff --git a/src/cmd/ksh93/tests/path.sh b/src/cmd/ksh93/tests/path.sh index dbe9eddc0..7b066a282 100755 --- a/src/cmd/ksh93/tests/path.sh +++ b/src/cmd/ksh93/tests/path.sh @@ -542,7 +542,7 @@ fi ofile=$tmp/command_x_chunks.sh trap 'sleep_pid=; while kill -9 $pid; do :; done 2>/dev/null; err_exit "'\''command -x'\'' hung"' TERM trap 'kill $sleep_pid; while kill -9 $pid; do :; done 2>/dev/null; trap - INT; kill -s INT $$"' INT -{ sleep 15; kill $$; } & +{ sleep 60; kill $$; } & sleep_pid=$! ( export LC_ALL=C @@ -558,14 +558,17 @@ sleep_pid=$! done print "chunks=0 expargs=$# args=0 expsize=$((${#args}+1)) size=0" unset args - command -px awk 'BEGIN { - ARGC -= 2; # final static args - for (i=1; i$ofile & pid=$! wait $pid