From 0d4e6519c5dd81034935d4da9c519c17e41b1202 Mon Sep 17 00:00:00 2001 From: "Joel E. Denny" Date: Wed, 30 Oct 2019 14:22:16 -0400 Subject: [PATCH] [lit] Fix internal env calling other internal commands Without this patch, when using lit's internal shell, if `env` on a lit RUN line calls `cd`, `mkdir`, or any of the other in-process shell builtins that lit implements, lit accidentally searches for the latter as an external executable. This patch puts such builtins in a map so that boilerplate for them need be implemented only once. This patch moves that handling after processing of `env` so that `env` calling such a builtin can be detected. Finally, because such calls appear to be useless, this patch takes the safe approach of diagnosing them rather than supporting them. Reviewed By: probinson, mgorny, rnk Differential Revision: https://reviews.llvm.org/D66506 --- llvm/utils/lit/lit/TestRunner.py | 115 ++++++++++----------- .../lit/tests/Inputs/shtest-env/env-calls-cd.txt | 1 + .../tests/Inputs/shtest-env/env-calls-colon.txt | 1 + .../lit/tests/Inputs/shtest-env/env-calls-echo.txt | 1 + .../tests/Inputs/shtest-env/env-calls-export.txt | 1 + .../tests/Inputs/shtest-env/env-calls-mkdir.txt | 1 + .../lit/tests/Inputs/shtest-env/env-calls-rm.txt | 1 + llvm/utils/lit/tests/shtest-env.py | 37 ++++++- 8 files changed, 95 insertions(+), 63 deletions(-) create mode 100644 llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt create mode 100644 llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt create mode 100644 llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt create mode 100644 llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt create mode 100644 llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt create mode 100644 llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py index a334d0d..278f6dea 100644 --- a/llvm/utils/lit/lit/TestRunner.py +++ b/llvm/utils/lit/lit/TestRunner.py @@ -260,6 +260,27 @@ def updateEnv(env, args): env.env[key] = val return args[arg_idx_next:] +def executeBuiltinCd(cmd, shenv): + """executeBuiltinCd - Change the current directory.""" + if len(cmd.args) != 2: + raise InternalShellError("'cd' supports only one argument") + newdir = cmd.args[1] + # Update the cwd in the parent environment. + if os.path.isabs(newdir): + shenv.cwd = newdir + else: + shenv.cwd = os.path.realpath(os.path.join(shenv.cwd, newdir)) + # The cd builtin always succeeds. If the directory does not exist, the + # following Popen calls will fail instead. + return ShellCommandResult(cmd, "", "", 0, False) + +def executeBuiltinExport(cmd, shenv): + """executeBuiltinExport - Set an environment variable.""" + if len(cmd.args) != 2: + raise InternalShellError("'export' supports only one argument") + updateEnv(shenv, cmd.args) + return ShellCommandResult(cmd, "", "", 0, False) + def executeBuiltinEcho(cmd, shenv): """Interpret a redirected echo command""" opened_files = [] @@ -319,9 +340,8 @@ def executeBuiltinEcho(cmd, shenv): for (name, mode, f, path) in opened_files: f.close() - if not is_redirected: - return stdout.getvalue() - return "" + output = "" if is_redirected else stdout.getvalue() + return ShellCommandResult(cmd, output, "", 0, False) def executeBuiltinMkdir(cmd, cmd_shenv): """executeBuiltinMkdir - Create new directories.""" @@ -456,6 +476,10 @@ def executeBuiltinRm(cmd, cmd_shenv): exitCode = 1 return ShellCommandResult(cmd, "", stderr.getvalue(), exitCode, False) +def executeBuiltinColon(cmd, cmd_shenv): + """executeBuiltinColon - Discard arguments and exit with status 0.""" + return ShellCommandResult(cmd, "", "", 0, False) + def processRedirects(cmd, stdin_source, cmd_shenv, opened_files): """Return the standard fds for cmd after applying redirects @@ -581,64 +605,6 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper): raise ValueError('Unknown shell command: %r' % cmd.op) assert isinstance(cmd, ShUtil.Pipeline) - # Handle shell builtins first. - if cmd.commands[0].args[0] == 'cd': - if len(cmd.commands) != 1: - raise ValueError("'cd' cannot be part of a pipeline") - if len(cmd.commands[0].args) != 2: - raise ValueError("'cd' supports only one argument") - newdir = cmd.commands[0].args[1] - # Update the cwd in the parent environment. - if os.path.isabs(newdir): - shenv.cwd = newdir - else: - shenv.cwd = os.path.realpath(os.path.join(shenv.cwd, newdir)) - # The cd builtin always succeeds. If the directory does not exist, the - # following Popen calls will fail instead. - return 0 - - # Handle "echo" as a builtin if it is not part of a pipeline. This greatly - # speeds up tests that construct input files by repeatedly echo-appending to - # a file. - # FIXME: Standardize on the builtin echo implementation. We can use a - # temporary file to sidestep blocking pipe write issues. - if cmd.commands[0].args[0] == 'echo' and len(cmd.commands) == 1: - output = executeBuiltinEcho(cmd.commands[0], shenv) - results.append(ShellCommandResult(cmd.commands[0], output, "", 0, - False)) - return 0 - - if cmd.commands[0].args[0] == 'export': - if len(cmd.commands) != 1: - raise ValueError("'export' cannot be part of a pipeline") - if len(cmd.commands[0].args) != 2: - raise ValueError("'export' supports only one argument") - updateEnv(shenv, cmd.commands[0].args) - return 0 - - if cmd.commands[0].args[0] == 'mkdir': - if len(cmd.commands) != 1: - raise InternalShellError(cmd.commands[0], "Unsupported: 'mkdir' " - "cannot be part of a pipeline") - cmdResult = executeBuiltinMkdir(cmd.commands[0], shenv) - results.append(cmdResult) - return cmdResult.exitCode - - if cmd.commands[0].args[0] == 'rm': - if len(cmd.commands) != 1: - raise InternalShellError(cmd.commands[0], "Unsupported: 'rm' " - "cannot be part of a pipeline") - cmdResult = executeBuiltinRm(cmd.commands[0], shenv) - results.append(cmdResult) - return cmdResult.exitCode - - if cmd.commands[0].args[0] == ':': - if len(cmd.commands) != 1: - raise InternalShellError(cmd.commands[0], "Unsupported: ':' " - "cannot be part of a pipeline") - results.append(ShellCommandResult(cmd.commands[0], '', '', 0, False)) - return 0; - procs = [] default_stdin = subprocess.PIPE stderrTempFiles = [] @@ -646,6 +612,12 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper): named_temp_files = [] builtin_commands = set(['cat', 'diff']) builtin_commands_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), "builtin_commands") + inproc_builtins = {'cd': executeBuiltinCd, + 'export': executeBuiltinExport, + 'echo': executeBuiltinEcho, + 'mkdir': executeBuiltinMkdir, + 'rm': executeBuiltinRm, + ':': executeBuiltinColon} # To avoid deadlock, we use a single stderr stream for piped # output. This is null until we have seen some output using # stderr. @@ -663,6 +635,27 @@ def _executeShCmd(cmd, shenv, results, timeoutHelper): raise InternalShellError(j, "Error: 'env' requires a subcommand") + # Handle in-process builtins. + # + # Handle "echo" as a builtin if it is not part of a pipeline. This + # greatly speeds up tests that construct input files by repeatedly + # echo-appending to a file. + # FIXME: Standardize on the builtin echo implementation. We can use a + # temporary file to sidestep blocking pipe write issues. + inproc_builtin = inproc_builtins.get(args[0], None) + if inproc_builtin and (args[0] != 'echo' or len(cmd.commands) == 1): + # env calling an in-process builtin is useless, so we take the safe + # approach of complaining. + if not cmd_shenv is shenv: + raise InternalShellError(j, "Error: 'env' cannot call '{}'" + .format(args[0])) + if len(cmd.commands) != 1: + raise InternalShellError(j, "Unsupported: '{}' cannot be part" + " of a pipeline".format(args[0])) + result = inproc_builtin(j, cmd_shenv) + results.append(result) + return result.exitCode + stdin, stdout, stderr = processRedirects(j, default_stdin, cmd_shenv, opened_files) diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt new file mode 100644 index 0000000..b045506 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-cd.txt @@ -0,0 +1 @@ +# RUN: env -u FOO BAR=3 cd foobar diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt new file mode 100644 index 0000000..f2566c4 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-colon.txt @@ -0,0 +1 @@ +# RUN: env -u FOO BAR=3 : diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt new file mode 100644 index 0000000..b4591ee --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-echo.txt @@ -0,0 +1 @@ +# RUN: env -u FOO BAR=3 echo hello world diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt new file mode 100644 index 0000000..9712e42 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-export.txt @@ -0,0 +1 @@ +# RUN: env -u FOO BAR=3 export BAZ=3 diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt new file mode 100644 index 0000000..6116a25 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-mkdir.txt @@ -0,0 +1 @@ +# RUN: env -u FOO BAR=3 mkdir foobar diff --git a/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt new file mode 100644 index 0000000..6c5e8e8 --- /dev/null +++ b/llvm/utils/lit/tests/Inputs/shtest-env/env-calls-rm.txt @@ -0,0 +1 @@ +# RUN: env -u FOO BAR=3 rm foobar diff --git a/llvm/utils/lit/tests/shtest-env.py b/llvm/utils/lit/tests/shtest-env.py index e89bfd6..3b547df 100644 --- a/llvm/utils/lit/tests/shtest-env.py +++ b/llvm/utils/lit/tests/shtest-env.py @@ -7,7 +7,7 @@ # Make sure env commands are included in printed commands. -# CHECK: -- Testing: 7 tests{{.*}} +# CHECK: -- Testing: 13 tests{{.*}} # CHECK: FAIL: shtest-env :: env-args-last-is-assign.txt ({{[^)]*}}) # CHECK: Error: 'env' requires a subcommand @@ -25,19 +25,52 @@ # CHECK: Error: 'env' requires a subcommand # CHECK: error: command failed with exit status: {{.*}} +# CHECK: FAIL: shtest-env :: env-calls-cd.txt ({{[^)]*}}) +# CHECK: $ "env" "-u" "FOO" "BAR=3" "cd" "foobar" +# CHECK: Error: 'env' cannot call 'cd' +# CHECK: error: command failed with exit status: {{.*}} + +# CHECK: FAIL: shtest-env :: env-calls-colon.txt ({{[^)]*}}) +# CHECK: $ "env" "-u" "FOO" "BAR=3" ":" +# CHECK: Error: 'env' cannot call ':' +# CHECK: error: command failed with exit status: {{.*}} + +# CHECK: FAIL: shtest-env :: env-calls-echo.txt ({{[^)]*}}) +# CHECK: $ "env" "-u" "FOO" "BAR=3" "echo" "hello" "world" +# CHECK: Error: 'env' cannot call 'echo' +# CHECK: error: command failed with exit status: {{.*}} + +# CHECK: FAIL: shtest-env :: env-calls-export.txt ({{[^)]*}}) +# CHECK: $ "env" "-u" "FOO" "BAR=3" "export" "BAZ=3" +# CHECK: Error: 'env' cannot call 'export' +# CHECK: error: command failed with exit status: {{.*}} + +# CHECK: FAIL: shtest-env :: env-calls-mkdir.txt ({{[^)]*}}) +# CHECK: $ "env" "-u" "FOO" "BAR=3" "mkdir" "foobar" +# CHECK: Error: 'env' cannot call 'mkdir' +# CHECK: error: command failed with exit status: {{.*}} + +# CHECK: FAIL: shtest-env :: env-calls-rm.txt ({{[^)]*}}) +# CHECK: $ "env" "-u" "FOO" "BAR=3" "rm" "foobar" +# CHECK: Error: 'env' cannot call 'rm' +# CHECK: error: command failed with exit status: {{.*}} + # CHECK: PASS: shtest-env :: env-u.txt ({{[^)]*}}) # CHECK: $ "{{[^"]*}}" "print_environment.py" # CHECK: $ "env" "-u" "FOO" "{{[^"]*}}" "print_environment.py" # CHECK: $ "env" "-u" "FOO" "-u" "BAR" "{{[^"]*}}" "print_environment.py" +# CHECK-NOT: ${{.*}}print_environment.py # CHECK: PASS: shtest-env :: env.txt ({{[^)]*}}) # CHECK: $ "env" "A_FOO=999" "{{[^"]*}}" "print_environment.py" # CHECK: $ "env" "A_FOO=1" "B_BAR=2" "C_OOF=3" "{{[^"]*}}" "print_environment.py" +# CHECK-NOT: ${{.*}}print_environment.py # CHECK: PASS: shtest-env :: mixed.txt ({{[^)]*}}) # CHECK: $ "env" "A_FOO=999" "-u" "FOO" "{{[^"]*}}" "print_environment.py" # CHECK: $ "env" "A_FOO=1" "-u" "FOO" "B_BAR=2" "-u" "BAR" "C_OOF=3" "{{[^"]*}}" "print_environment.py" +# CHECK-NOT: ${{.*}}print_environment.py # CHECK: Expected Passes : 3 -# CHECK: Unexpected Failures: 4 +# CHECK: Unexpected Failures: 10 # CHECK-NOT: {{.}} -- 2.7.4