From: David Sehr Date: Tue, 23 Aug 2016 15:59:24 +0000 (+0000) Subject: Revert "Save environment snapshot and use at fork/exec" X-Git-Tag: android-x86-7.1-r1~34^2 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=47393386aa1f44c61a10045c7dbb5d559c9f7cab;p=android-x86%2Fart.git Revert "Save environment snapshot and use at fork/exec" This reverts commit 1488ff8aa3b041734ef0fbd113df512a2376e44e. Change-Id: I3c237c94ffa865378f8efd9aa2b0fb2ad33867c2 --- diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 21e664fc9..66c8f87fd 100644 --- a/runtime/runtime.cc +++ b/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. diff --git a/runtime/runtime.h b/runtime/runtime.h index d0e8b4122..1394462fd 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -96,11 +96,6 @@ class Transaction; typedef std::vector> RuntimeOptions; -struct EnvSnapshot { - public: - std::vector > 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 env_snapshot_; - DISALLOW_COPY_AND_ASSIGN(Runtime); }; std::ostream& operator<<(std::ostream& os, const Runtime::CalleeSaveType& rhs); diff --git a/runtime/utils.cc b/runtime/utils.cc index fb6c1b3d3..6a50b8eee 100644 --- a/runtime/utils.cc +++ b/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& 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& 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(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 { diff --git a/runtime/utils.h b/runtime/utils.h index 61db76a19..c1e88a4fe 100644 --- a/runtime/utils.h +++ b/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& arg_vector, std::string* error_msg); int ExecAndReturnCode(std::vector& arg_vector, std::string* error_msg); diff --git a/runtime/utils_test.cc b/runtime/utils_test.cc index 64f295e33..f00edffab 100644 --- a/runtime/utils_test.cc +++ b/runtime/utils_test.cc @@ -16,8 +16,6 @@ #include "utils.h" -#include - #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 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 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 descriptor( { 'L', 'a', '/', 'b', '$', 0xed, 0xa0, 0x80, 0xed, 0xb0, 0x80, ';', 0x00 });