Merge "Revert "Save environment snapshot and use at fork/exec"" into nyc-mr1-dev

runtime/runtime.cc

@@ -464,8 +464,6 @@ bool Runtime::Create(RuntimeArgumentMap&& runtime_options) {
     return false;
   }
   instance_ = new Runtime;
-  // Protect subprocesses from modifications to LD_LIBRARY_PATH, etc.
-  instance_->env_snapshot_.reset(art::TakeEnvSnapshot());
   if (!instance_->Init(std::move(runtime_options))) {
     // TODO: Currently deleting the instance will abort the runtime on destruction. Now This will
     // leak memory, instead. Fix the destructor. b/19100793.

runtime/runtime.h

@@ -96,11 +96,6 @@ class Transaction;
 
 typedef std::vector<std::pair<std::string, const void*>> RuntimeOptions;
 
-struct EnvSnapshot {
- public:
-  std::vector<std::unique_ptr<std::string> > name_value_pairs_;
-};
-
 // Not all combinations of flags are valid. You may not visit all roots as well as the new roots
 // (no logical reason to do this). You also may not start logging new roots and stop logging new
 // roots (also no logical reason to do this).
@@ -653,12 +648,6 @@ class Runtime {
     return zygote_no_threads_;
   }
 
-  // Returns a saved copy of the environment (getenv/setenv values).
-  // Used by Fork to protect against overwriting LD_LIBRARY_PATH, etc.
-  const EnvSnapshot* GetEnvSnapshot() const {
-    return env_snapshot_.get();
-  }
-
  private:
   static void InitPlatformSignalHandlers();
 
@@ -883,9 +872,6 @@ class Runtime {
   // Whether zygote code is in a section that should not start threads.
   bool zygote_no_threads_;
 
-  // Saved environment.
-  std::unique_ptr<const EnvSnapshot> env_snapshot_;
-
   DISALLOW_COPY_AND_ASSIGN(Runtime);
 };
 std::ostream& operator<<(std::ostream& os, const Runtime::CalleeSaveType& rhs);

runtime/utils.cc

@@ -56,22 +56,6 @@
 
 namespace art {
 
-namespace {
-#ifdef __APPLE__
-inline char** GetEnviron() {
-  // When Google Test is built as a framework on MacOS X, the environ variable
-  // is unavailable. Apple's documentation (man environ) recommends using
-  // _NSGetEnviron() instead.
-  return *_NSGetEnviron();
-}
-#else
-// Some POSIX platforms expect you to declare environ. extern "C" makes
-// it reside in the global namespace.
-extern "C" char** environ;
-inline char** GetEnviron() { return environ; }
-#endif
-}  // namespace
-
 #if defined(__linux__)
 static constexpr bool kUseAddr2line = !kIsTargetBuild;
 #endif
@@ -1409,15 +1393,6 @@ std::string GetSystemImageFilename(const char* location, const InstructionSet is
   return filename;
 }
 
-const EnvSnapshot* TakeEnvSnapshot() {
-  EnvSnapshot* snapshot = new EnvSnapshot();
-  char** env = GetEnviron();
-  for (size_t i = 0; env[i] != nullptr; ++i) {
-    snapshot->name_value_pairs_.emplace_back(new std::string(env[i]));
-  }
-  return snapshot;
-}
-
 int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_msg) {
   const std::string command_line(Join(arg_vector, ' '));
   CHECK_GE(arg_vector.size(), 1U) << command_line;
@@ -1441,20 +1416,8 @@ int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_m
     // change process groups, so we don't get reaped by ProcessManager
     setpgid(0, 0);
 
-    // The child inherits the environment unless the caller overrides it.
-    if (Runtime::Current() == nullptr || Runtime::Current()->GetEnvSnapshot() == nullptr) {
-      execv(program, &args[0]);
-    } else {
-      const EnvSnapshot* saved_snapshot = Runtime::Current()->GetEnvSnapshot();
-      // Allocation between fork and exec is not well-behaved.  Use a variable-length array instead.
-      char* envp[saved_snapshot->name_value_pairs_.size() + 1];
-      for (size_t i = 0; i < saved_snapshot->name_value_pairs_.size(); ++i) {
-        envp[i] = const_cast<char*>(saved_snapshot->name_value_pairs_[i]->c_str());
-      }
-      envp[saved_snapshot->name_value_pairs_.size()] = nullptr;
-      execve(program, &args[0], envp);
-    }
-    PLOG(ERROR) << "Failed to execve(" << command_line << ")";
+    execv(program, &args[0]);
+    PLOG(ERROR) << "Failed to execv(" << command_line << ")";
     // _exit to avoid atexit handlers in child.
     _exit(1);
   } else {

runtime/utils.h

@@ -42,7 +42,6 @@ namespace art {
 class ArtField;
 class ArtMethod;
 class DexFile;
-struct EnvSnapshot;
 
 namespace mirror {
 class Class;
@@ -291,10 +290,6 @@ std::string GetDalvikCacheFilenameOrDie(const char* file_location,
 // Returns the system location for an image
 std::string GetSystemImageFilename(const char* location, InstructionSet isa);
 
-// Capture an environment snapshot that will be used by Exec.
-// Any subsequent setenvs will be ignored in child processes.
-const EnvSnapshot* TakeEnvSnapshot();
-
 // Wrapper on fork/execv to run a command in a subprocess.
 bool Exec(std::vector<std::string>& arg_vector, std::string* error_msg);
 int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_msg);

runtime/utils_test.cc

@@ -16,8 +16,6 @@
 
 #include "utils.h"
 
-#include <stdlib.h>
-
 #include "class_linker-inl.h"
 #include "common_runtime_test.h"
 #include "mirror/array.h"
@@ -381,55 +379,6 @@ TEST_F(UtilsTest, ExecError) {
   }
 }
 
-TEST_F(UtilsTest, EnvSnapshotAdditionsAreNotVisible) {
-  static constexpr const char* kModifiedVariable = "EXEC_SHOULD_NOT_EXPORT_THIS";
-  static constexpr int kOverwrite = 1;
-  // Set an variable in the current environment.
-  EXPECT_EQ(setenv(kModifiedVariable, "NEVER", kOverwrite), 0);
-  // Test that it is not exported.
-  std::vector<std::string> command;
-  if (kIsTargetBuild) {
-    std::string android_root(GetAndroidRoot());
-    command.push_back(android_root + "/bin/printenv");
-  } else {
-    command.push_back("/usr/bin/printenv");
-  }
-  command.push_back(kModifiedVariable);
-  std::string error_msg;
-  if (!(RUNNING_ON_MEMORY_TOOL && kMemoryToolDetectsLeaks)) {
-    // Running on valgrind fails due to some memory that leaks in thread alternate signal stacks.
-    EXPECT_FALSE(Exec(command, &error_msg));
-    EXPECT_NE(0U, error_msg.size()) << error_msg;
-  }
-}
-
-TEST_F(UtilsTest, EnvSnapshotDeletionsAreNotVisible) {
-  static constexpr const char* kDeletedVariable = "PATH";
-  static constexpr int kOverwrite = 1;
-  // Save the variable's value.
-  const char* save_value = getenv(kDeletedVariable);
-  EXPECT_NE(save_value, nullptr);
-  // Delete the variable.
-  EXPECT_EQ(unsetenv(kDeletedVariable), 0);
-  // Test that it is not exported.
-  std::vector<std::string> command;
-  if (kIsTargetBuild) {
-    std::string android_root(GetAndroidRoot());
-    command.push_back(android_root + "/bin/printenv");
-  } else {
-    command.push_back("/usr/bin/printenv");
-  }
-  command.push_back(kDeletedVariable);
-  std::string error_msg;
-  if (!(RUNNING_ON_MEMORY_TOOL && kMemoryToolDetectsLeaks)) {
-    // Running on valgrind fails due to some memory that leaks in thread alternate signal stacks.
-    EXPECT_TRUE(Exec(command, &error_msg));
-    EXPECT_EQ(0U, error_msg.size()) << error_msg;
-  }
-  // Restore the variable's value.
-  EXPECT_EQ(setenv(kDeletedVariable, save_value, kOverwrite), 0);
-}
-
 TEST_F(UtilsTest, IsValidDescriptor) {
   std::vector<uint8_t> descriptor(
       { 'L', 'a', '/', 'b', '$', 0xed, 0xa0, 0x80, 0xed, 0xb0, 0x80, ';', 0x00 });