llvm-reduce: Only disable crash reports in the child

[arsenm]

Summary

Disabling the crash reports and symbolization had unexpected
effects when using lldb on llvm-reduce itself.

Core files are still disabled in the parent. It's probably
not simple to only set this in the child, and I can't think of a
scenario where I would need core dumps for llvm-reduce.

[arsenm]

llvm/tools/llvm-reduce/TestRunner.cpp

54

This is also dropping the PATH which might be really annoying for test scripts

[aeubanks]

yes, dropping PATH is bad

is there a way to get the current env and add to it?

[arsenm]

In D140861#4070402, @aeubanks wrote:

yes, dropping PATH is bad

is there a way to get the current env and add to it?

Yes, but I'm not sure if any of them are fully portable. The two I know are use the 3rd envp argument to main, and then the environ global variable.

llvm/tools/llvm-reduce/TestRunner.h

 // This class contains all the info necessary for running the provided
 // interesting-ness test, as well as the most reduced module and its
 // respective filename.
 class TestRunner {
 public:
   TestRunner(StringRef TestName, ArrayRef<std::string> TestArgs,
              std::unique_ptr<ReducerWorkItem> Program,
              std::unique_ptr<TargetMachine> TM, const char *ToolName,
-             StringRef OutputFilename, bool InputIsBitcode, bool OutputBitcode);
+             StringRef OutputFilename, bool InputIsBitcode, bool OutputBitcode,
+             bool PreserveDebugEnvironment);
 
   /// Runs the interesting-ness test for the specified file
   /// @returns 0 if test was successful, 1 if otherwise
   int run(StringRef Filename) const;
 
   /// Returns the most reduced version of the original testcase
   ReducerWorkItem &getProgram() const { return *Program; }
 
private:
   StringRef TestName;
   const char *ToolName;
   SmallVector<StringRef> TestArgs;
   std::unique_ptr<ReducerWorkItem> Program;
   std::unique_ptr<TargetMachine> TM;
   StringRef OutputFilename;
   const bool InputIsBitcode;
   bool EmitBitcode;
+  const bool PreserveDebugEnvironment;
 };
 
 } // namespace llvm
 
 #endif

llvm/tools/llvm-reduce/TestRunner.cpp

 #include "llvm/Transforms/IPO/ThinLTOBitcodeWriter.h"
 
 using namespace llvm;
 
 TestRunner::TestRunner(StringRef TestName, ArrayRef<std::string> RawTestArgs,
                        std::unique_ptr<ReducerWorkItem> Program,
                        std::unique_ptr<TargetMachine> TM, const char *ToolName,
                        StringRef OutputName, bool InputIsBitcode,
-                       bool OutputBitcode)
+                       bool OutputBitcode, bool PreserveDebugEnvironment)
     : TestName(TestName), ToolName(ToolName), Program(std::move(Program)),
       TM(std::move(TM)), OutputFilename(OutputName),
-      InputIsBitcode(InputIsBitcode), EmitBitcode(OutputBitcode) {
+      InputIsBitcode(InputIsBitcode), EmitBitcode(OutputBitcode),
+      PreserveDebugEnvironment(PreserveDebugEnvironment) {
   assert(this->Program && "Initialized with null program?");
 
   TestArgs.push_back(TestName); // argv[0]
   TestArgs.append(RawTestArgs.begin(), RawTestArgs.end());
 }
 
 static constexpr std::array<std::optional<StringRef>, 3> DefaultRedirects = {
     StringRef()};
 static constexpr std::array<std::optional<StringRef>, 3> NullRedirects;
 
+static constexpr std::array<StringRef, 2> DebugEnvironmentVars = {
+  "LLVM_DISABLE_CRASH_REPORT=1",
+  "LLVM_DISABLE_SYMBOLIZATION=1"
+};
+
 /// Runs the interestingness test, passes file to be tested as first argument
 /// and other specified test arguments after that.
 int TestRunner::run(StringRef Filename) const {
   SmallVector<StringRef> ExecArgs(TestArgs);
   ExecArgs.push_back(Filename);
 
   std::string ErrMsg;
 
-  int Result =
-      sys::ExecuteAndWait(TestName, ExecArgs, /*Env=*/std::nullopt,
-                          Verbose ? DefaultRedirects : NullRedirects,
-                          /*SecondsToWait=*/0, /*MemoryLimit=*/0, &ErrMsg);
+  const std::optional<ArrayRef<StringRef>> Env =
+      PreserveDebugEnvironment ? std::nullopt
+                               : std::make_optional(DebugEnvironmentVars);
+
+  int Result = sys::ExecuteAndWait(
+      TestName, ExecArgs,
+      /*Env=*/Env,
+      /*Redirects=*/Verbose ? DefaultRedirects : NullRedirects,
 
   if (Result < 0) {
     Error E = make_error<StringError>("Error running interesting-ness test: " +
                                           ErrMsg,
                                       inconvertibleErrorCode());
     errs() << toString(std::move(E));
     exit(1);
   }

llvm/tools/llvm-reduce/llvm-reduce.cpp

 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <system_error>
 #include <vector>
 
-#ifdef _WIN32
-#include <windows.h>
-#endif
-
 using namespace llvm;
 
 cl::OptionCategory LLVMReduceOptions("llvm-reduce options");
 
 static cl::opt<bool> Help("h", cl::desc("Alias for -help"), cl::Hidden,
                           cl::cat(LLVMReduceOptions));
 static cl::opt<bool> Version("v", cl::desc("Alias for -version"), cl::Hidden,
                              cl::cat(LLVMReduceOptions));

 bool isReduced(ReducerWorkItem &M, const TestRunner &Test);
 
 /// Turn off crash debugging features
 ///
 /// Crash is expected, so disable crash reports and symbolization to reduce
 /// output clutter and avoid potentially slow symbolization.
 static void disableEnvironmentDebugFeatures() {
   sys::Process::PreventCoreFiles();
-
-  // TODO: Copied from not. Should have a wrapper around setenv.
-#ifdef _WIN32
-  SetEnvironmentVariableA("LLVM_DISABLE_CRASH_REPORT", "1");
-  SetEnvironmentVariableA("LLVM_DISABLE_SYMBOLIZATION", "1");
-#else
-  setenv("LLVM_DISABLE_CRASH_REPORT", "1", /*overwrite=*/1);
-  setenv("LLVM_DISABLE_SYMBOLIZATION", "1", /*overwrite=*/1);
-#endif
 }
 
 static std::pair<StringRef, bool> determineOutputType(bool IsMIR,
                                                       bool InputIsBitcode) {
   bool OutputBitcode = ForceOutputBitcode || InputIsBitcode;
 
   if (ReplaceInput) { // In-place
     OutputFilename = InputFilename.c_str();
int main(int Argc, char **Argv) {
   StringRef OutputFilename;
   bool OutputBitcode;
   std::tie(OutputFilename, OutputBitcode) =
       determineOutputType(ReduceModeMIR, InputIsBitcode);
 
   // Initialize test environment
   TestRunner Tester(TestFilename, TestArguments, std::move(OriginalProgram),
                     std::move(TM), Argv[0], OutputFilename, InputIsBitcode,
-                    OutputBitcode);
+                    OutputBitcode, PreserveDebugEnvironment);
 
   // This parses and writes out the testcase into a temporary file copy for the
   // test, rather than evaluating the source IR directly. This is for the
   // convenience of lit tests; the stripped out comments may have broken the
   // interestingness checks.
   if (!isReduced(Tester.getProgram(), Tester)) {
     errs() << "\nInput isn't interesting! Verify interesting-ness test\n";
     return 1;