From 380a1e9eafa05a0bd5704f999d16f598f7dc0776 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Wed, 17 Jan 2018 10:21:49 -0800 Subject: [PATCH] Make multiprocess_exec_test_child depend on base to get OS_ defines Some doofus really wanted to shoot himself in the foot ( https://codereview.chromium.org/808493003/diff/80001/util/test/multiprocess_exec_test_child.cc ), and it worked. Depend on base to get build_config.h instead, so that OS_POSIX is also set on Fuchsia. Also, don't call getrlimit() on Fuchsia. Bug: crashpad:196 Change-Id: I102de22751fd90889bfb59b6c4a577e99357094b Reviewed-on: https://chromium-review.googlesource.com/868742 Commit-Queue: Scott Graham Reviewed-by: Mark Mentovai --- test/BUILD.gn | 4 +++ test/multiprocess_exec_test_child.cc | 40 +++++++++++++++------------- test/test_test.gyp | 3 +++ 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/test/BUILD.gn b/test/BUILD.gn index 2b7991e2..a937854e 100644 --- a/test/BUILD.gn +++ b/test/BUILD.gn @@ -166,6 +166,10 @@ executable("crashpad_test_test_multiprocess_exec_test_child") { sources = [ "multiprocess_exec_test_child.cc", ] + + deps = [ + "../third_party/mini_chromium:base", + ] } static_library("gmock_main") { diff --git a/test/multiprocess_exec_test_child.cc b/test/multiprocess_exec_test_child.cc index 011a37d0..f2761ea5 100644 --- a/test/multiprocess_exec_test_child.cc +++ b/test/multiprocess_exec_test_child.cc @@ -15,16 +15,12 @@ #include #include #include -#include #include #include -#if defined(__APPLE__) || defined(__linux__) -#define OS_POSIX 1 -#elif defined(_WIN32) -#define OS_WIN 1 -#endif +#include "base/logging.h" +#include "build/build_config.h" #if defined(OS_POSIX) #include @@ -35,18 +31,26 @@ int main(int argc, char* argv[]) { #if defined(OS_POSIX) + +#if defined(OS_FUCHSIA) + // getrlimit() is not implemented on Fuchsia. By construction, the child only + // receieves specific fds that it's given, but check low values as mild + // verification. + int last_fd = 1024; +#else + rlimit rlimit_nofile; + if (getrlimit(RLIMIT_NOFILE, &rlimit_nofile) != 0) { + LOG(FATAL) << "getrlimit"; + } + int last_fd = static_cast(rlimit_nofile.rlim_cur); +#endif // OS_FUCHSIA + // Make sure that there’s nothing open at any FD higher than 3. All FDs other // than stdin, stdout, and stderr should have been closed prior to or at // exec(). - rlimit rlimit_nofile; - if (getrlimit(RLIMIT_NOFILE, &rlimit_nofile) != 0) { - abort(); - } - for (int fd = STDERR_FILENO + 1; - fd < static_cast(rlimit_nofile.rlim_cur); - ++fd) { + for (int fd = STDERR_FILENO + 1; fd < last_fd; ++fd) { if (close(fd) == 0 || errno != EBADF) { - abort(); + LOG(FATAL) << "close"; } } @@ -54,14 +58,14 @@ int main(int argc, char* argv[]) { char c; ssize_t rv = read(STDIN_FILENO, &c, 1); if (rv != 1 || c != 'z') { - abort(); + LOG(FATAL) << "read"; } // Write a byte to stdout. c = 'Z'; rv = write(STDOUT_FILENO, &c, 1); if (rv != 1) { - abort(); + LOG(FATAL) << "write"; } #elif defined(OS_WIN) // TODO(scottmg): Verify that only the handles we expect to be open, are. @@ -72,7 +76,7 @@ int main(int argc, char* argv[]) { HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE); if (!ReadFile(stdin_handle, &c, 1, &bytes_read, nullptr) || bytes_read != 1 || c != 'z') { - abort(); + LOG(FATAL) << "ReadFile"; } // Write a byte to stdout. @@ -81,7 +85,7 @@ int main(int argc, char* argv[]) { if (!WriteFile( GetStdHandle(STD_OUTPUT_HANDLE), &c, 1, &bytes_written, nullptr) || bytes_written != 1) { - abort(); + LOG(FATAL) << "WriteFile"; } #endif // OS_POSIX diff --git a/test/test_test.gyp b/test/test_test.gyp index a31650da..3c6d7066 100644 --- a/test/test_test.gyp +++ b/test/test_test.gyp @@ -48,6 +48,9 @@ { 'target_name': 'crashpad_test_test_multiprocess_exec_test_child', 'type': 'executable', + 'dependencies': [ + '../third_party/mini_chromium/mini_chromium.gyp:base', + ], 'sources': [ 'multiprocess_exec_test_child.cc', ],