From b74e2f19b14eb8b4225cca1c23d8d9af9b3fbf9c Mon Sep 17 00:00:00 2001 From: Lou Knauer Date: Tue, 25 Feb 2025 19:50:30 +0100 Subject: [PATCH 1/6] [tools] Disable tools/test under user-mode emulation --- tools/test/CMakeLists.txt | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index a724da14fe..f4839fc3ae 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -13,15 +13,20 @@ add_dependencies(ret0 not) llvm_test_run(EXECUTABLE "$" "$" "$") llvm_add_test_for_target(ret0) -# Check that expected crashes are handled correctly. -llvm_test_executable_no_test(abrt abort.c) -add_dependencies(abrt not) -llvm_test_run(EXECUTABLE "$" "--crash" "$") -llvm_add_test_for_target(abrt) +# These test will always fail under user-mode emulation because 'not' +# spawns a subprocess outside the emulator and the check_env test +# runs the host python interpreter under the emulator for the target. +if(NOT(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER)) + # Check that expected crashes are handled correctly. + llvm_test_executable_no_test(abrt abort.c) + add_dependencies(abrt not) + llvm_test_run(EXECUTABLE "$" "--crash" "$") + llvm_add_test_for_target(abrt) -# Check that not passes environment variables to the called executable. -find_package(Python COMPONENTS Interpreter) -llvm_test_executable_no_test(check_env check_env.c) -add_dependencies(check_env not) -llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "%b/test/test_not.py" "$" "$") -llvm_add_test_For_target(check_env) + # Check that not passes environment variables to the called executable. + find_package(Python COMPONENTS Interpreter) + llvm_test_executable_no_test(check_env check_env.c) + add_dependencies(check_env not) + llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "%b/test/test_not.py" "$" "$") + llvm_add_test_For_target(check_env) +endif() From 6474814113f3162654f37809777cc6b13be3515f Mon Sep 17 00:00:00 2001 From: Lou Knauer Date: Tue, 25 Feb 2025 19:51:27 +0100 Subject: [PATCH 2/6] [tools/not] Basic support for user-mode emulation --- Fortran/gfortran/CMakeLists.txt | 10 +++++++++- tools/CMakeLists.txt | 15 +++++++++++++++ tools/not.cpp | 17 +++++++++++++++++ tools/test/CMakeLists.txt | 6 ++++++ 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/Fortran/gfortran/CMakeLists.txt b/Fortran/gfortran/CMakeLists.txt index b2896db621..ae170ea4bf 100644 --- a/Fortran/gfortran/CMakeLists.txt +++ b/Fortran/gfortran/CMakeLists.txt @@ -716,10 +716,18 @@ function(gfortran_add_execute_test expect_error main others fflags ldflags) gfortran_make_working_dir("${target}" working_dir) get_filename_component(working_dir_name "${working_dir}" NAME) + # When running in a emulator, use the version of 'not' that will + # spawn the subprocess under that emulator as well. + if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) + set(NOT_HELPER "not-spawning-emulator") + else() + set(NOT_HELPER "not") + endif() + llvm_test_executable_no_test(${target} ${main} ${others}) if (expect_error) llvm_test_run( - EXECUTABLE "%b/not --crash %S/${target}" + EXECUTABLE "%b/${NOT_HELPER} --crash %S/${target}" WORKDIR "%S/${working_dir_name}") else () llvm_test_run(WORKDIR "%S/${working_dir_name}") diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index e8812d8265..ed54651247 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -41,3 +41,18 @@ else() endif() add_executable(not ${CMAKE_CURRENT_SOURCE_DIR}/not.cpp) + +if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) + # Because 'not' spawns a subprocess for its argument, it needs to know about the + # emulator when running in user-mode emulation. This is not needed when using 'not' to + # verify outputs, only when using it to execute a test binary, which is why this + # definition is only added to the 'not-target' target. + separate_arguments(TEST_SUITE_RUN_UNDER_AS_LIST NATIVE_COMMAND ${TEST_SUITE_RUN_UNDER}) + set(AS_STRINGLITS "") + foreach(arg ${TEST_SUITE_RUN_UNDER_AS_LIST}) + set(AS_STRINGLITS "${AS_STRINGLITS}\t\"${arg}\",\n") + endforeach() + add_executable(not-spawning-emulator ${CMAKE_CURRENT_SOURCE_DIR}/not.cpp) + target_compile_definitions(not-spawning-emulator PUBLIC "-DTEST_SUITE_RUN_UNDER=${AS_STRINGLITS}") + unset(AS_STRINGLITS) +endif() diff --git a/tools/not.cpp b/tools/not.cpp index 31721f9473..4932d6e6fc 100644 --- a/tools/not.cpp +++ b/tools/not.cpp @@ -29,6 +29,10 @@ #include #endif +#ifdef TEST_SUITE_RUN_UNDER +#include +#endif + int main(int argc, char* const* argv) { bool expectCrash = false; @@ -54,6 +58,19 @@ int main(int argc, char* const* argv) { if (argc == 0) return 1; +#ifdef TEST_SUITE_RUN_UNDER + // In case of user-mode emulation, before spawning a new subprocess, the + // emulator needs to be preprended to the argv vector for the child. + // TEST_SUITE_RUN_UNDER will be defined to a comma-separated list of + // string litterals. + std::vector argvbuf = {TEST_SUITE_RUN_UNDER}; + for (char *const *argp = argv; *argp != NULL; ++argp) + argvbuf.push_back(*argp); + argvbuf.push_back(NULL); + argv = argvbuf.data(); + argc = argvbuf.size() - 1; +#endif + int result; #if defined(__unix__) || defined(__APPLE__) diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index f4839fc3ae..b32096652a 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -29,4 +29,10 @@ if(NOT(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER)) add_dependencies(check_env not) llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "%b/test/test_not.py" "$" "$") llvm_add_test_For_target(check_env) +else() + # Check that expected crashes are handled correctly under user-mode emulation. + llvm_test_executable_no_test(abrt abort.c) + add_dependencies(abrt not-spawning-emulator) + llvm_test_run(EXECUTABLE "$" "--crash" "$") + llvm_add_test_for_target(abrt) endif() From b6645ef9a487f805f6526946cf0e8c152d0856f5 Mon Sep 17 00:00:00 2001 From: Lou Knauer Date: Fri, 28 Feb 2025 10:58:59 +0100 Subject: [PATCH 3/6] Review comments --- Fortran/gfortran/CMakeLists.txt | 8 ++++---- tools/not.cpp | 2 +- tools/test/CMakeLists.txt | 19 +++++++++---------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/Fortran/gfortran/CMakeLists.txt b/Fortran/gfortran/CMakeLists.txt index ae170ea4bf..916c30b6c6 100644 --- a/Fortran/gfortran/CMakeLists.txt +++ b/Fortran/gfortran/CMakeLists.txt @@ -716,18 +716,18 @@ function(gfortran_add_execute_test expect_error main others fflags ldflags) gfortran_make_working_dir("${target}" working_dir) get_filename_component(working_dir_name "${working_dir}" NAME) - # When running in a emulator, use the version of 'not' that will + # When running in an emulator, use the version of 'not' that will # spawn the subprocess under that emulator as well. if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) - set(NOT_HELPER "not-spawning-emulator") + set(NOT_TOOL "not-spawning-emulator") else() - set(NOT_HELPER "not") + set(NOT_TOOL "not") endif() llvm_test_executable_no_test(${target} ${main} ${others}) if (expect_error) llvm_test_run( - EXECUTABLE "%b/${NOT_HELPER} --crash %S/${target}" + EXECUTABLE "%b/${NOT_TOOL} --crash %S/${target}" WORKDIR "%S/${working_dir_name}") else () llvm_test_run(WORKDIR "%S/${working_dir_name}") diff --git a/tools/not.cpp b/tools/not.cpp index 4932d6e6fc..35f8e2991f 100644 --- a/tools/not.cpp +++ b/tools/not.cpp @@ -62,7 +62,7 @@ int main(int argc, char* const* argv) { // In case of user-mode emulation, before spawning a new subprocess, the // emulator needs to be preprended to the argv vector for the child. // TEST_SUITE_RUN_UNDER will be defined to a comma-separated list of - // string litterals. + // string literals. std::vector argvbuf = {TEST_SUITE_RUN_UNDER}; for (char *const *argp = argv; *argp != NULL; ++argp) argvbuf.push_back(*argp); diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index b32096652a..0ac028fc0c 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -13,10 +13,13 @@ add_dependencies(ret0 not) llvm_test_run(EXECUTABLE "$" "$" "$") llvm_add_test_for_target(ret0) -# These test will always fail under user-mode emulation because 'not' -# spawns a subprocess outside the emulator and the check_env test -# runs the host python interpreter under the emulator for the target. -if(NOT(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER)) +if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) + # Check that expected crashes are handled correctly under user-mode emulation. + llvm_test_executable_no_test(abrt abort.c) + add_dependencies(abrt not-spawning-emulator) + llvm_test_run(EXECUTABLE "$" "--crash" "$") + llvm_add_test_for_target(abrt) +else() # Check that expected crashes are handled correctly. llvm_test_executable_no_test(abrt abort.c) add_dependencies(abrt not) @@ -24,15 +27,11 @@ if(NOT(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER)) llvm_add_test_for_target(abrt) # Check that not passes environment variables to the called executable. + # This test is disabled in case of user-mode emulation because it would otherwise run the + # host python interpreter under the target emulator. find_package(Python COMPONENTS Interpreter) llvm_test_executable_no_test(check_env check_env.c) add_dependencies(check_env not) llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "%b/test/test_not.py" "$" "$") llvm_add_test_For_target(check_env) -else() - # Check that expected crashes are handled correctly under user-mode emulation. - llvm_test_executable_no_test(abrt abort.c) - add_dependencies(abrt not-spawning-emulator) - llvm_test_run(EXECUTABLE "$" "--crash" "$") - llvm_add_test_for_target(abrt) endif() From 321227a93538f887f545e8f67edbd01ed040f27a Mon Sep 17 00:00:00 2001 From: Lou Knauer Date: Fri, 28 Feb 2025 11:32:51 +0100 Subject: [PATCH 4/6] Replace test_not.cpp by test_not.py so that it can be tested under emulation --- CMakeLists.txt | 6 +++ Fortran/gfortran/CMakeLists.txt | 8 --- tools/test/CMakeLists.txt | 37 +++++--------- tools/test/test_not.cpp | 88 +++++++++++++++++++++++++++++++++ tools/test/test_not.py | 7 --- 5 files changed, 105 insertions(+), 41 deletions(-) create mode 100644 tools/test/test_not.cpp delete mode 100644 tools/test/test_not.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 21b997134b..269825ef43 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -319,6 +319,12 @@ if (TEST_SUITE_USER_MODE_EMULATION) set(FPCMP fpcmp) endif() +# Shortcut for the path to the not executable +set(NOT_TOOL not) +if (TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) + set(NOT_TOOL not-spawning-emulator) +endif() + add_subdirectory(litsupport) option(TEST_SUITE_COLLECT_COMPILE_TIME diff --git a/Fortran/gfortran/CMakeLists.txt b/Fortran/gfortran/CMakeLists.txt index 916c30b6c6..3b770631c7 100644 --- a/Fortran/gfortran/CMakeLists.txt +++ b/Fortran/gfortran/CMakeLists.txt @@ -716,14 +716,6 @@ function(gfortran_add_execute_test expect_error main others fflags ldflags) gfortran_make_working_dir("${target}" working_dir) get_filename_component(working_dir_name "${working_dir}" NAME) - # When running in an emulator, use the version of 'not' that will - # spawn the subprocess under that emulator as well. - if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) - set(NOT_TOOL "not-spawning-emulator") - else() - set(NOT_TOOL "not") - endif() - llvm_test_executable_no_test(${target} ${main} ${others}) if (expect_error) llvm_test_run( diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index 0ac028fc0c..56d51923d4 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -1,8 +1,3 @@ -# Copy these files to the build directory so that the tests can be run even -# without the source directory. -configure_file(test_not.py test_not.py - COPYONLY) - llvm_test_executable_no_test(ret1 ret1.c) add_dependencies(ret1 not) llvm_test_run(EXECUTABLE "$" "$") @@ -13,25 +8,15 @@ add_dependencies(ret0 not) llvm_test_run(EXECUTABLE "$" "$" "$") llvm_add_test_for_target(ret0) -if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) - # Check that expected crashes are handled correctly under user-mode emulation. - llvm_test_executable_no_test(abrt abort.c) - add_dependencies(abrt not-spawning-emulator) - llvm_test_run(EXECUTABLE "$" "--crash" "$") - llvm_add_test_for_target(abrt) -else() - # Check that expected crashes are handled correctly. - llvm_test_executable_no_test(abrt abort.c) - add_dependencies(abrt not) - llvm_test_run(EXECUTABLE "$" "--crash" "$") - llvm_add_test_for_target(abrt) +# Check that expected crashes are handled correctly under user-mode emulation. +llvm_test_executable_no_test(abrt abort.c) +add_dependencies(abrt ${NOT_TOOL}) +llvm_test_run(EXECUTABLE "$" "--crash" "$") +llvm_add_test_for_target(abrt) - # Check that not passes environment variables to the called executable. - # This test is disabled in case of user-mode emulation because it would otherwise run the - # host python interpreter under the target emulator. - find_package(Python COMPONENTS Interpreter) - llvm_test_executable_no_test(check_env check_env.c) - add_dependencies(check_env not) - llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "%b/test/test_not.py" "$" "$") - llvm_add_test_For_target(check_env) -endif() +# Check that not passes environment variables to the called executable. +llvm_test_executable_no_test(test_not test_not.cpp) +llvm_test_executable_no_test(check_env check_env.c) +add_dependencies(check_env not test_not) +llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "$" "$" "$") +llvm_add_test_For_target(check_env) diff --git a/tools/test/test_not.cpp b/tools/test/test_not.cpp new file mode 100644 index 0000000000..e6fd817bc5 --- /dev/null +++ b/tools/test/test_not.cpp @@ -0,0 +1,88 @@ +//===- test_not.cpp - Test for enviroment variables in the not tool -------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include +#include +#include + +#ifdef _WIN32 +#define WIN32_LEAN_AND_MEAN +#define NOMINMAX +#include +#endif + +#if defined(__unix__) || defined(__APPLE__) +#include +#include +#endif + +int main(int argc, char *const *argv) { + ++argv; + --argc; + if (argc == 0) + return EXIT_FAILURE; + +#ifdef _WIN32 + SetEnvironmentVariableA("SET_IN_PARENT", "something"); +#else + setenv("SET_IN_PARENT", "something", 0); +#endif + + int result; + +#if defined(__unix__) || defined(__APPLE__) + pid_t pid; + extern char **environ; + if (posix_spawn(&pid, argv[0], NULL, NULL, argv, environ)) + return EXIT_FAILURE; + if (waitpid(pid, &result, WUNTRACED | WCONTINUED) == -1) + return EXIT_FAILURE; +#else + std::stringstream ss; + ss << argv[0]; + for (int i = 1; i < argc; ++i) + ss << " " << argv[i]; + std::string cmd = ss.str(); + result = std::system(cmd.c_str()); +#endif + + int retcode = 0; + int signal = 0; + +#ifdef _WIN32 + // Handle abort() in msvcrt -- It has exit code as 3. abort(), aka + // unreachable, should be recognized as a crash. However, some binaries use + // exit code 3 on non-crash failure paths, so only do this if we expect a + // crash. + if (errno) { + // If the command interpreter was not found, errno will be set and 0 will + // be returned. It is unlikely that this will happen in our use case, but + // check anyway. + retcode = 1; + signal = 1; + } else { + // On Windows, result is the exit code, except for the special case above. + retcode = result; + signal = 0; + } +#elif defined(WIFEXITED) && defined(WEXITSTATUS) && defined(WIFSIGNALED) && \ + defined(WTERMSIG) + // On POSIX systems and Solaris, result is a composite value of the exit code + // and, potentially, the signal that caused termination of the command. + if (WIFEXITED(result)) + retcode = WEXITSTATUS(result); + if (WIFSIGNALED(result)) + signal = WTERMSIG(result); +#else +#error "Unsupported system" +#endif + + if (!signal && retcode == EXIT_SUCCESS) + return EXIT_SUCCESS; + return EXIT_FAILURE; +} diff --git a/tools/test/test_not.py b/tools/test/test_not.py deleted file mode 100644 index 8eafa53683..0000000000 --- a/tools/test/test_not.py +++ /dev/null @@ -1,7 +0,0 @@ -import os -import subprocess -import sys - -os.environ["SET_IN_PARENT"] = "something" -out = subprocess.run([sys.argv[1], sys.argv[2]]) -sys.exit(out.returncode) From 61568705d6b3954b2e4dd3a391c1c40e662fbdef Mon Sep 17 00:00:00 2001 From: Lou Knauer Date: Mon, 3 Mar 2025 20:07:31 +0100 Subject: [PATCH 5/6] Add new not argument instead of different target --- CMakeLists.txt | 7 +++-- tools/CMakeLists.txt | 15 ----------- tools/not.cpp | 56 ++++++++++++++++++++++++++++----------- tools/test/CMakeLists.txt | 12 ++++++--- 4 files changed, 54 insertions(+), 36 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 269825ef43..fbbf21b896 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -320,9 +320,12 @@ if (TEST_SUITE_USER_MODE_EMULATION) endif() # Shortcut for the path to the not executable -set(NOT_TOOL not) +set(NOT_TOOL "$") if (TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) - set(NOT_TOOL not-spawning-emulator) + # Because 'not' spawns a subprocess for its argument, it needs to know about the + # emulator when running in user-mode emulation. This is not needed when using 'not' to + # verify outputs, only when using it to execute a test binary. + set(NOT_TOOL "$" "--run-under" ${TEST_SUITE_RUN_UNDER} "--") endif() add_subdirectory(litsupport) diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt index ed54651247..e8812d8265 100644 --- a/tools/CMakeLists.txt +++ b/tools/CMakeLists.txt @@ -41,18 +41,3 @@ else() endif() add_executable(not ${CMAKE_CURRENT_SOURCE_DIR}/not.cpp) - -if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) - # Because 'not' spawns a subprocess for its argument, it needs to know about the - # emulator when running in user-mode emulation. This is not needed when using 'not' to - # verify outputs, only when using it to execute a test binary, which is why this - # definition is only added to the 'not-target' target. - separate_arguments(TEST_SUITE_RUN_UNDER_AS_LIST NATIVE_COMMAND ${TEST_SUITE_RUN_UNDER}) - set(AS_STRINGLITS "") - foreach(arg ${TEST_SUITE_RUN_UNDER_AS_LIST}) - set(AS_STRINGLITS "${AS_STRINGLITS}\t\"${arg}\",\n") - endforeach() - add_executable(not-spawning-emulator ${CMAKE_CURRENT_SOURCE_DIR}/not.cpp) - target_compile_definitions(not-spawning-emulator PUBLIC "-DTEST_SUITE_RUN_UNDER=${AS_STRINGLITS}") - unset(AS_STRINGLITS) -endif() diff --git a/tools/not.cpp b/tools/not.cpp index 35f8e2991f..1536868803 100644 --- a/tools/not.cpp +++ b/tools/not.cpp @@ -10,6 +10,12 @@ // Will return true if cmd doesn't crash and returns false. // not --crash cmd // Will return true if cmd crashes (e.g. for testing crash reporting). +// not --run-under ...... -- cmd +// Will prepend the emulator and its arguments to the spawn of the +// subcommand. If --crash is used as well, it has to come after the +// '--run-under <...> ...' arguments. The double-dash is used to separate +// emulator arguments from cmd arguments. This order makes it easier to +// use the not tool in the test-suite. // This file is a stripped down version of not.cpp from llvm/utils. This does // not depend on any LLVM library. @@ -29,9 +35,7 @@ #include #endif -#ifdef TEST_SUITE_RUN_UNDER #include -#endif int main(int argc, char* const* argv) { bool expectCrash = false; @@ -39,11 +43,44 @@ int main(int argc, char* const* argv) { ++argv; --argc; - if (argc > 0 && std::string(argv[0]) == "--crash") { + // If necessary, prepend the command and arguments for a user-mode + // emulator such as QEMU to the command line that will be used + // to then spawn a subcommand. + std::vector argvbuf; + if (argc > 0 && std::string(argv[0]) == "--run-under") { + ++argv; + --argc; + while (argc > 0) { + --argc; + if (std::string(argv[0]) == "--") { + ++argv; + break; + } + + argvbuf.push_back(argv[0]); + ++argv; + } + + // If present, the crash flag is between the emulator + // arguments and the cmd. + if (argc > 0 && std::string(argv[0]) == "--crash") { + ++argv; + --argc; + expectCrash = true; + } + + for (char *const *argp = argv; *argp != NULL; ++argp) + argvbuf.push_back(*argp); + argvbuf.push_back(NULL); + argv = argvbuf.data(); + argc = argvbuf.size() - 1; + } else if (argc > 0 && std::string(argv[0]) == "--crash") { ++argv; --argc; expectCrash = true; + } + if (expectCrash) { // Crash is expected, so disable crash report and symbolization to reduce // output and avoid potentially slow symbolization. #ifdef _WIN32 @@ -58,19 +95,6 @@ int main(int argc, char* const* argv) { if (argc == 0) return 1; -#ifdef TEST_SUITE_RUN_UNDER - // In case of user-mode emulation, before spawning a new subprocess, the - // emulator needs to be preprended to the argv vector for the child. - // TEST_SUITE_RUN_UNDER will be defined to a comma-separated list of - // string literals. - std::vector argvbuf = {TEST_SUITE_RUN_UNDER}; - for (char *const *argp = argv; *argp != NULL; ++argp) - argvbuf.push_back(*argp); - argvbuf.push_back(NULL); - argv = argvbuf.data(); - argc = argvbuf.size() - 1; -#endif - int result; #if defined(__unix__) || defined(__APPLE__) diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index 56d51923d4..6b5f3fe4e9 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -10,13 +10,19 @@ llvm_add_test_for_target(ret0) # Check that expected crashes are handled correctly under user-mode emulation. llvm_test_executable_no_test(abrt abort.c) -add_dependencies(abrt ${NOT_TOOL}) -llvm_test_run(EXECUTABLE "$" "--crash" "$") +add_dependencies(abrt not) +llvm_test_run(EXECUTABLE ${NOT_TOOL} "--crash" "$") llvm_add_test_for_target(abrt) # Check that not passes environment variables to the called executable. llvm_test_executable_no_test(test_not test_not.cpp) llvm_test_executable_no_test(check_env check_env.c) add_dependencies(check_env not test_not) -llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "$" "$" "$") +if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) + # ${TEST_SUITE_RUN_UNDER} is needed here because test_not is not itself + # user-mode emulation aware, unlike the not tool. + llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "$" ${TEST_SUITE_RUN_UNDER} ${NOT_TOOL} "$") +else() + llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "$" ${NOT_TOOL} "$") +endif() llvm_add_test_For_target(check_env) From 9bc637eb3f91650f43409f7b82f58261975c66da Mon Sep 17 00:00:00 2001 From: Lou Knauer Date: Fri, 7 Mar 2025 18:10:02 +0100 Subject: [PATCH 6/6] Address review comments --- CMakeLists.txt | 6 +++--- tools/not.cpp | 10 +++++----- tools/test/CMakeLists.txt | 6 ++++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index fbbf21b896..83875686bd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -322,9 +322,9 @@ endif() # Shortcut for the path to the not executable set(NOT_TOOL "$") if (TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) - # Because 'not' spawns a subprocess for its argument, it needs to know about the - # emulator when running in user-mode emulation. This is not needed when using 'not' to - # verify outputs, only when using it to execute a test binary. + # Because 'not' spawns a subprocess for its argument, it needs to know about + # the emulator when running in user-mode emulation. This is not needed when + # using 'not' to verify outputs, only when using it to execute a test binary. set(NOT_TOOL "$" "--run-under" ${TEST_SUITE_RUN_UNDER} "--") endif() diff --git a/tools/not.cpp b/tools/not.cpp index 1536868803..e6d788ad23 100644 --- a/tools/not.cpp +++ b/tools/not.cpp @@ -15,7 +15,9 @@ // subcommand. If --crash is used as well, it has to come after the // '--run-under <...> ...' arguments. The double-dash is used to separate // emulator arguments from cmd arguments. This order makes it easier to -// use the not tool in the test-suite. +// use the not tool in the test-suite. The emulator is expected to exit +// with the same exit status/signal than the emulated binary in case of +// a crash, and the --crash flag will behave the same way. // This file is a stripped down version of not.cpp from llvm/utils. This does // not depend on any LLVM library. @@ -23,6 +25,7 @@ #include #include #include +#include #ifdef _WIN32 #define WIN32_LEAN_AND_MEAN @@ -35,8 +38,6 @@ #include #endif -#include - int main(int argc, char* const* argv) { bool expectCrash = false; @@ -61,8 +62,7 @@ int main(int argc, char* const* argv) { ++argv; } - // If present, the crash flag is between the emulator - // arguments and the cmd. + // If present, the crash flag is between the emulator arguments and cmd. if (argc > 0 && std::string(argv[0]) == "--crash") { ++argv; --argc; diff --git a/tools/test/CMakeLists.txt b/tools/test/CMakeLists.txt index 6b5f3fe4e9..b755609117 100644 --- a/tools/test/CMakeLists.txt +++ b/tools/test/CMakeLists.txt @@ -21,8 +21,10 @@ add_dependencies(check_env not test_not) if(TEST_SUITE_USER_MODE_EMULATION AND TEST_SUITE_RUN_UNDER) # ${TEST_SUITE_RUN_UNDER} is needed here because test_not is not itself # user-mode emulation aware, unlike the not tool. - llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "$" ${TEST_SUITE_RUN_UNDER} ${NOT_TOOL} "$") + llvm_test_run(EXECUTABLE "$" ${TEST_SUITE_RUN_UNDER} + ${NOT_TOOL} "$") else() - llvm_test_run(EXECUTABLE ${Python_EXECUTABLE} "$" ${NOT_TOOL} "$") + llvm_test_run(EXECUTABLE "$" ${NOT_TOOL} + "$") endif() llvm_add_test_For_target(check_env)