From 46883516231e3776ca68a97385366d19f1a2d521 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Mon, 3 Apr 2017 13:53:11 -0400 Subject: [PATCH] =?UTF-8?q?=E2=80=9CPromote=E2=80=9D=C2=A0test::Paths::Exe?= =?UTF-8?q?cutable()=20to=20Paths::Executable()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This supports the “double handler” or “double handler with low probability” models from https://crashpad.chromium.org/bug/143. For crashpad_handler to be become its own client, it needs access to its own executable path to pass to CrashpadClient::StartHandler(). This was formerly available in the test-only test::Paths::Executable(). Bring that function’s implementation to the non-test Paths::Executable() in util/misc, and rename test::Paths to test::TestPaths to avoid future confusion. test::TestPaths must still be used to access TestDataRoot(), which does not make any sense to non-test code. test::TestPaths::Executable() is retained for use by tests, which most likely prefer the fatal semantics of that function. Paths::Executable() is not fatal because for the purposes of implementing the double handler, a failure to locate the executable path (which may happen on some systems in deeply-nested directory hierarchies) shouldn’t cause the initial crashpad_handler to abort, even if it does prevent a second crashpad_handler from being started. Bug: crashpad:143 Test: crashpad_util_test Paths.*, crashpad_test_test TestPaths.* Change-Id: I9f75bf61839ce51e33c9f7c0d7031cebead6a156 Reviewed-on: https://chromium-review.googlesource.com/466346 Reviewed-by: Scott Graham Commit-Queue: Mark Mentovai --- client/crashpad_client_win_test.cc | 8 ++-- doc/developing.md | 4 +- handler/win/crash_other_program.cc | 4 +- handler/win/crashy_test_z7_loader.cc | 4 +- snapshot/crashpad_info_client_options_test.cc | 4 +- .../mach_o_image_annotations_reader_test.cc | 6 +-- snapshot/win/exception_snapshot_win_test.cc | 6 +-- snapshot/win/extra_memory_ranges_test.cc | 4 +- .../win/pe_image_annotations_reader_test.cc | 4 +- snapshot/win/process_snapshot_win_test.cc | 4 +- test/multiprocess_exec_test.cc | 4 +- test/test.gyp | 14 +----- test/{paths.cc => test_paths.cc} | 48 +++++++++++-------- test/{paths.h => test_paths.h} | 12 +++-- test/{paths_test.cc => test_paths_test.cc} | 20 ++------ ...root.txt => test_paths_test_data_root.txt} | 2 +- test/test_test.gyp | 2 +- test/win/win_child_process.cc | 6 +-- test/win/win_multiprocess.cc | 1 - util/misc/paths.h | 40 ++++++++++++++++ {test => util/misc}/paths_linux.cc | 23 +++++---- {test => util/misc}/paths_mac.cc | 19 +++++--- util/misc/paths_test.cc | 39 +++++++++++++++ {test => util/misc}/paths_win.cc | 18 ++++--- util/net/http_body_test.cc | 8 ++-- util/net/http_multipart_builder_test.cc | 8 ++-- util/net/http_transport_test.cc | 4 +- util/util.gyp | 5 ++ util/util_test.gyp | 1 + util/win/process_info_test.cc | 4 +- 30 files changed, 205 insertions(+), 121 deletions(-) rename test/{paths.cc => test_paths.cc} (66%) rename test/{paths.h => test_paths.h} (88%) rename test/{paths_test.cc => test_paths_test.cc} (63%) rename test/{paths_test_data_root.txt => test_paths_test_data_root.txt} (89%) create mode 100644 util/misc/paths.h rename {test => util/misc}/paths_linux.cc (78%) rename {test => util/misc}/paths_mac.cc (76%) create mode 100644 util/misc/paths_test.cc rename {test => util/misc}/paths_win.cc (72%) diff --git a/client/crashpad_client_win_test.cc b/client/crashpad_client_win_test.cc index 8441e6a3..743e930d 100644 --- a/client/crashpad_client_win_test.cc +++ b/client/crashpad_client_win_test.cc @@ -24,10 +24,10 @@ #include "base/logging.h" #include "gtest/gtest.h" #include "test/errors.h" -#include "test/paths.h" #include "test/scoped_temp_dir.h" -#include "util/win/process_info.h" +#include "test/test_paths.h" #include "test/win/win_multiprocess.h" +#include "util/win/process_info.h" #include "util/win/scoped_handle.h" #include "util/win/termination_codes.h" @@ -189,7 +189,7 @@ void WaitForAllChildProcessesOf(HANDLE parent) { } void StartAndUseHandler(const base::FilePath& temp_dir) { - base::FilePath handler_path = Paths::Executable().DirName().Append( + base::FilePath handler_path = TestPaths::Executable().DirName().Append( FILE_PATH_LITERAL("crashpad_handler.com")); CrashpadClient client; @@ -280,7 +280,7 @@ TEST(CrashpadClient, StartWithSameStdoutStderr) { void StartAndUseBrokenHandler(CrashpadClient* client) { ScopedTempDir temp_dir; - base::FilePath handler_path = Paths::Executable().DirName().Append( + base::FilePath handler_path = TestPaths::Executable().DirName().Append( FILE_PATH_LITERAL("fake_handler_that_crashes_at_startup.exe")); ASSERT_TRUE(client->StartHandler(handler_path, temp_dir.path(), diff --git a/doc/developing.md b/doc/developing.md index 65d62356..429fff7f 100644 --- a/doc/developing.md +++ b/doc/developing.md @@ -214,9 +214,9 @@ $ adb push \ /data/local/tmp/ [100%] /data/local/tmp/crashpad_test_test_multiprocess_exec_test_child $ adb shell mkdir -p /data/local/tmp/crashpad_test_data_root/test -$ adb push test/paths_test_data_root.txt \ +$ adb push test/test_paths_test_data_root.txt \ /data/local/tmp/crashpad_test_data_root/test/ -[100%] /data/local/tmp/crashpad_test_data_root/test/paths_test_data_root.txt +[100%] /data/local/tmp/crashpad_test_data_root/test/test_paths_test_data_root.txt $ adb shell device:/ $ cd /data/local/tmp device:/data/local/tmp $ CRASHPAD_TEST_DATA_ROOT=crashpad_test_data_root \ diff --git a/handler/win/crash_other_program.cc b/handler/win/crash_other_program.cc index ced82859..389aee1f 100644 --- a/handler/win/crash_other_program.cc +++ b/handler/win/crash_other_program.cc @@ -20,7 +20,7 @@ #include "base/logging.h" #include "base/strings/stringprintf.h" #include "client/crashpad_client.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/win/scoped_handle.h" @@ -88,7 +88,7 @@ int CrashOtherProgram(int argc, wchar_t* argv[]) { } // Launch another process that hangs. - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName().Append(L"hanging_program.exe").value(); ChildLauncher child(child_test_executable, argv[1]); diff --git a/handler/win/crashy_test_z7_loader.cc b/handler/win/crashy_test_z7_loader.cc index 79802cd5..4ffcc031 100644 --- a/handler/win/crashy_test_z7_loader.cc +++ b/handler/win/crashy_test_z7_loader.cc @@ -19,7 +19,7 @@ #include "base/logging.h" #include "build/build_config.h" #include "client/crashpad_client.h" -#include "test/paths.h" +#include "test/test_paths.h" #if !defined(ARCH_CPU_X86) #error This test is only supported on x86. @@ -43,7 +43,7 @@ int CrashyLoadZ7Main(int argc, wchar_t* argv[]) { // The DLL has /Z7 symbols embedded in the binary (rather than in a .pdb). // There's only an x86 version of this dll as newer x64 toolchains can't // generate this format any more. - base::FilePath z7_path = test::Paths::TestDataRoot().Append( + base::FilePath z7_path = test::TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("handler/win/z7_test.dll")); HMODULE z7_test = LoadLibrary(z7_path.value().c_str()); if (!z7_test) { diff --git a/snapshot/crashpad_info_client_options_test.cc b/snapshot/crashpad_info_client_options_test.cc index af607c32..5a706339 100644 --- a/snapshot/crashpad_info_client_options_test.cc +++ b/snapshot/crashpad_info_client_options_test.cc @@ -21,7 +21,7 @@ #include "client/crashpad_info.h" #include "gtest/gtest.h" #include "test/errors.h" -#include "test/paths.h" +#include "test/test_paths.h" #if defined(OS_MACOSX) #include @@ -187,7 +187,7 @@ TEST(CrashpadInfoClientOptions, TwoModules) { #elif defined(OS_WIN) const base::FilePath::StringType kDlExtension = FILE_PATH_LITERAL(".dll"); #endif - base::FilePath module_path = Paths::Executable().DirName().Append( + base::FilePath module_path = TestPaths::Executable().DirName().Append( FILE_PATH_LITERAL("crashpad_snapshot_test_module") + kDlExtension); #if defined(OS_MACOSX) ScopedDlHandle dl_handle( diff --git a/snapshot/mac/mach_o_image_annotations_reader_test.cc b/snapshot/mac/mach_o_image_annotations_reader_test.cc index 50e646ae..8472e335 100644 --- a/snapshot/mac/mach_o_image_annotations_reader_test.cc +++ b/snapshot/mac/mach_o_image_annotations_reader_test.cc @@ -35,7 +35,7 @@ #include "test/errors.h" #include "test/mac/mach_errors.h" #include "test/mac/mach_multiprocess.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "util/file/file_io.h" #include "util/mac/mac_util.h" #include "util/mach/exc_server_variants.h" @@ -50,12 +50,12 @@ namespace { // \return The path to crashpad_snapshot_test_module_crashy_initializer.so std::string ModuleWithCrashyInitializer() { - return Paths::Executable().value() + "_module_crashy_initializer.so"; + return TestPaths::Executable().value() + "_module_crashy_initializer.so"; } //! \return The path to the crashpad_snapshot_test_no_op executable. base::FilePath NoOpExecutable() { - return base::FilePath(Paths::Executable().value() + "_no_op"); + return base::FilePath(TestPaths::Executable().value() + "_no_op"); } class TestMachOImageAnnotationsReader final diff --git a/snapshot/win/exception_snapshot_win_test.cc b/snapshot/win/exception_snapshot_win_test.cc index c68fb2fd..2a376faa 100644 --- a/snapshot/win/exception_snapshot_win_test.cc +++ b/snapshot/win/exception_snapshot_win_test.cc @@ -23,7 +23,7 @@ #include "gtest/gtest.h" #include "snapshot/win/process_snapshot_win.h" #include "test/errors.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/thread/thread.h" @@ -140,7 +140,7 @@ void TestCrashingChild(const base::string16& directory_modification) { << ErrorMessage("WaitForSingleObject"); // Spawn a child process, passing it the pipe name to connect to. - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName() .Append(directory_modification) @@ -248,7 +248,7 @@ void TestDumpWithoutCrashingChild( << ErrorMessage("WaitForSingleObject"); // Spawn a child process, passing it the pipe name to connect to. - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName() .Append(directory_modification) diff --git a/snapshot/win/extra_memory_ranges_test.cc b/snapshot/win/extra_memory_ranges_test.cc index 57d41bb5..a08fac73 100644 --- a/snapshot/win/extra_memory_ranges_test.cc +++ b/snapshot/win/extra_memory_ranges_test.cc @@ -25,7 +25,7 @@ #include "client/simple_address_range_bag.h" #include "gtest/gtest.h" #include "snapshot/win/process_snapshot_win.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/win/process_info.h" @@ -45,7 +45,7 @@ enum TestType { void TestExtraMemoryRanges(TestType type, const base::string16& directory_modification) { // Spawn a child process, passing it the pipe name to connect to. - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName() .Append(directory_modification) diff --git a/snapshot/win/pe_image_annotations_reader_test.cc b/snapshot/win/pe_image_annotations_reader_test.cc index f791f80e..372297ed 100644 --- a/snapshot/win/pe_image_annotations_reader_test.cc +++ b/snapshot/win/pe_image_annotations_reader_test.cc @@ -29,7 +29,7 @@ #include "gtest/gtest.h" #include "snapshot/win/pe_image_reader.h" #include "snapshot/win/process_reader_win.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/win/process_info.h" @@ -49,7 +49,7 @@ enum TestType { void TestAnnotationsOnCrash(TestType type, const base::string16& directory_modification) { // Spawn a child process, passing it the pipe name to connect to. - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName() .Append(directory_modification) diff --git a/snapshot/win/process_snapshot_win_test.cc b/snapshot/win/process_snapshot_win_test.cc index b041051c..953e9936 100644 --- a/snapshot/win/process_snapshot_win_test.cc +++ b/snapshot/win/process_snapshot_win_test.cc @@ -21,7 +21,7 @@ #include "snapshot/win/pe_image_reader.h" #include "snapshot/win/process_reader_win.h" #include "test/errors.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/win/scoped_handle.h" @@ -38,7 +38,7 @@ void TestImageReaderChild(const base::string16& directory_modification) { CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str())); ASSERT_TRUE(done.is_valid()) << ErrorMessage("CreateEvent"); - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName() .Append(directory_modification) diff --git a/test/multiprocess_exec_test.cc b/test/multiprocess_exec_test.cc index 3e958aa8..477393d3 100644 --- a/test/multiprocess_exec_test.cc +++ b/test/multiprocess_exec_test.cc @@ -18,7 +18,7 @@ #include "base/strings/utf_string_conversions.h" #include "build/build_config.h" #include "gtest/gtest.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "util/file/file_io.h" namespace crashpad { @@ -48,7 +48,7 @@ class TestMultiprocessExec final : public MultiprocessExec { TEST(MultiprocessExec, MultiprocessExec) { TestMultiprocessExec multiprocess_exec; - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); #if defined(OS_POSIX) std::string child_test_executable = test_executable.value(); #elif defined(OS_WIN) diff --git a/test/test.gyp b/test/test.gyp index eb0a28b9..f1adf9c2 100644 --- a/test/test.gyp +++ b/test/test.gyp @@ -49,15 +49,12 @@ 'multiprocess_exec_posix.cc', 'multiprocess_exec_win.cc', 'multiprocess_posix.cc', - 'paths.cc', - 'paths.h', - 'paths_linux.cc', - 'paths_mac.cc', - 'paths_win.cc', 'scoped_temp_dir.cc', 'scoped_temp_dir.h', 'scoped_temp_dir_posix.cc', 'scoped_temp_dir_win.cc', + 'test_paths.cc', + 'test_paths.h', 'win/child_launcher.cc', 'win/child_launcher.h', 'win/win_child_process.cc', @@ -86,13 +83,6 @@ }, }], ], - 'target_conditions': [ - ['OS=="android"', { - 'sources/': [ - ['include', '^paths_linux\\.cc$'], - ], - }], - ], }, { 'target_name': 'crashpad_gtest_main', diff --git a/test/paths.cc b/test/test_paths.cc similarity index 66% rename from test/paths.cc rename to test/test_paths.cc index b5eb60b9..4c256030 100644 --- a/test/paths.cc +++ b/test/test_paths.cc @@ -12,13 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/paths.h" +#include "test/test_paths.h" #include #include #include "base/logging.h" #include "build/build_config.h" +#include "util/misc/paths.h" namespace crashpad { namespace test { @@ -28,7 +29,7 @@ namespace { bool IsTestDataRoot(const base::FilePath& candidate) { const base::FilePath marker_path = candidate.Append(FILE_PATH_LITERAL("test")) - .Append(FILE_PATH_LITERAL("paths_test_data_root.txt")); + .Append(FILE_PATH_LITERAL("test_paths_test_data_root.txt")); #if !defined(OS_WIN) struct stat stat_buf; @@ -59,23 +60,25 @@ base::FilePath TestDataRootInternal() { // In a standalone build, the test executable is usually at // out/{Debug,Release} relative to the Crashpad root. - const base::FilePath executable = Paths::Executable(); - base::FilePath candidate = - base::FilePath(executable.DirName() - .Append(base::FilePath::kParentDirectory) - .Append(base::FilePath::kParentDirectory)); - if (IsTestDataRoot(candidate)) { - return candidate; - } + base::FilePath executable_path; + if (Paths::Executable(&executable_path)) { + base::FilePath candidate = + base::FilePath(executable_path.DirName() + .Append(base::FilePath::kParentDirectory) + .Append(base::FilePath::kParentDirectory)); + if (IsTestDataRoot(candidate)) { + return candidate; + } - // In an in-Chromium build, the test executable is usually at - // out/{Debug,Release} relative to the Chromium root, and the Crashpad root is - // at third_party/crashpad/crashpad relative to the Chromium root. - candidate = candidate.Append(FILE_PATH_LITERAL("third_party")) - .Append(FILE_PATH_LITERAL("crashpad")) - .Append(FILE_PATH_LITERAL("crashpad")); - if (IsTestDataRoot(candidate)) { - return candidate; + // In an in-Chromium build, the test executable is usually at + // out/{Debug,Release} relative to the Chromium root, and the Crashpad root + // is at third_party/crashpad/crashpad relative to the Chromium root. + candidate = candidate.Append(FILE_PATH_LITERAL("third_party")) + .Append(FILE_PATH_LITERAL("crashpad")) + .Append(FILE_PATH_LITERAL("crashpad")); + if (IsTestDataRoot(candidate)) { + return candidate; + } } // If nothing else worked, use the current directory, issuing a warning if it @@ -90,7 +93,14 @@ base::FilePath TestDataRootInternal() { } // namespace // static -base::FilePath Paths::TestDataRoot() { +base::FilePath TestPaths::Executable() { + base::FilePath executable_path; + CHECK(Paths::Executable(&executable_path)); + return executable_path; +} + +// static +base::FilePath TestPaths::TestDataRoot() { static base::FilePath* test_data_root = new base::FilePath(TestDataRootInternal()); return *test_data_root; diff --git a/test/paths.h b/test/test_paths.h similarity index 88% rename from test/paths.h rename to test/test_paths.h index 7643ddd8..261ab911 100644 --- a/test/paths.h +++ b/test/test_paths.h @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef CRASHPAD_TEST_PATHS_H_ -#define CRASHPAD_TEST_PATHS_H_ +#ifndef CRASHPAD_TEST_TEST_PATHS_H_ +#define CRASHPAD_TEST_TEST_PATHS_H_ #include "base/files/file_path.h" #include "base/macros.h" @@ -22,9 +22,11 @@ namespace crashpad { namespace test { //! \brief Functions to obtain paths from within tests. -class Paths { +class TestPaths { public: //! \brief Returns the pathname of the currently-running test executable. + //! + //! On failure, aborts execution. static base::FilePath Executable(); //! \brief Returns the pathname of the test data root. @@ -40,10 +42,10 @@ class Paths { //! files. static base::FilePath TestDataRoot(); - DISALLOW_IMPLICIT_CONSTRUCTORS(Paths); + DISALLOW_IMPLICIT_CONSTRUCTORS(TestPaths); }; } // namespace test } // namespace crashpad -#endif // CRASHPAD_TEST_PATHS_H_ +#endif // CRASHPAD_TEST_TEST_PATHS_H_ diff --git a/test/paths_test.cc b/test/test_paths_test.cc similarity index 63% rename from test/paths_test.cc rename to test/test_paths_test.cc index 7cbca049..9d43b594 100644 --- a/test/paths_test.cc +++ b/test/test_paths_test.cc @@ -12,10 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/paths.h" +#include "test/test_paths.h" #include "base/files/file_path.h" -#include "build/build_config.h" #include "gtest/gtest.h" #include "util/file/file_io.h" @@ -23,22 +22,11 @@ namespace crashpad { namespace test { namespace { -TEST(Paths, Executable) { - base::FilePath executable_path = Paths::Executable(); - base::FilePath executable_name = executable_path.BaseName(); -#if defined(OS_WIN) - EXPECT_EQ(FILE_PATH_LITERAL("crashpad_test_test.exe"), - executable_name.value()); -#else - EXPECT_EQ("crashpad_test_test", executable_name.value()); -#endif // OS_WIN -} - -TEST(Paths, TestDataRoot) { - base::FilePath test_data_root = Paths::TestDataRoot(); +TEST(TestPaths, TestDataRoot) { + base::FilePath test_data_root = TestPaths::TestDataRoot(); ScopedFileHandle file(LoggingOpenFileForRead( test_data_root.Append(FILE_PATH_LITERAL("test")) - .Append(FILE_PATH_LITERAL("paths_test_data_root.txt")))); + .Append(FILE_PATH_LITERAL("test_paths_test_data_root.txt")))); EXPECT_TRUE(file.is_valid()); } diff --git a/test/paths_test_data_root.txt b/test/test_paths_test_data_root.txt similarity index 89% rename from test/paths_test_data_root.txt rename to test/test_paths_test_data_root.txt index 1ddd2642..6ac07806 100644 --- a/test/paths_test_data_root.txt +++ b/test/test_paths_test_data_root.txt @@ -12,5 +12,5 @@ # See the License for the specific language governing permissions and # limitations under the License. -This file is used by Paths::TestDataRoot() to locate the test data root. It +This file is used by TestPaths::TestDataRoot() to locate the test data root. It is present at a known path from the test data root. diff --git a/test/test_test.gyp b/test/test_test.gyp index 053d9c82..a31650da 100644 --- a/test/test_test.gyp +++ b/test/test_test.gyp @@ -39,8 +39,8 @@ 'main_arguments_test.cc', 'multiprocess_exec_test.cc', 'multiprocess_posix_test.cc', - 'paths_test.cc', 'scoped_temp_dir_test.cc', + 'test_paths_test.cc', 'win/win_child_process_test.cc', 'win/win_multiprocess_test.cc', ], diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc index 06ab21b2..4c821a0c 100644 --- a/test/win/win_child_process.cc +++ b/test/win/win_child_process.cc @@ -24,11 +24,11 @@ #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "gtest/gtest.h" +#include "test/test_paths.h" #include "util/stdlib/string_number_conversion.h" #include "util/string/split_string.h" #include "util/win/handle.h" #include "util/win/scoped_local_alloc.h" -#include "test/paths.h" namespace crashpad { namespace test { @@ -70,7 +70,7 @@ ScopedKernelHANDLE LaunchCommandLine(wchar_t* command_line) { startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); startup_info.dwFlags = STARTF_USESTDHANDLES; PROCESS_INFORMATION process_info; - if (!CreateProcess(Paths::Executable().value().c_str(), + if (!CreateProcess(TestPaths::Executable().value().c_str(), &command_line[0], // This cannot be constant, per MSDN. nullptr, nullptr, @@ -186,7 +186,7 @@ std::unique_ptr WinChildProcess::Launch() { const ::testing::TestInfo* const test_info = ::testing::UnitTest::GetInstance()->current_test_info(); std::wstring command_line = - Paths::Executable().value() + L" " + + TestPaths::Executable().value() + L" " + base::UTF8ToUTF16(base::StringPrintf("--gtest_filter=%s.%s %s=0x%x|0x%x", test_info->test_case_name(), test_info->name(), diff --git a/test/win/win_multiprocess.cc b/test/win/win_multiprocess.cc index 6a451f4d..dce171ce 100644 --- a/test/win/win_multiprocess.cc +++ b/test/win/win_multiprocess.cc @@ -22,7 +22,6 @@ #include "base/strings/utf_string_conversions.h" #include "util/stdlib/string_number_conversion.h" #include "util/string/split_string.h" -#include "test/paths.h" namespace crashpad { namespace test { diff --git a/util/misc/paths.h b/util/misc/paths.h new file mode 100644 index 00000000..30bbec50 --- /dev/null +++ b/util/misc/paths.h @@ -0,0 +1,40 @@ +// Copyright 2017 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef CRASHPAD_UTIL_PATHS_H_ +#define CRASHPAD_UTIL_PATHS_H_ + +#include "base/files/file_path.h" +#include "base/macros.h" + +namespace crashpad { + +//! \brief Functions to obtain paths. +class Paths { + public: + //! \brief Obtains the pathname of the currently-running executable. + //! + //! \param[out] path The pathname of the currently-running executable. + //! + //! \return `true` on success. `false` on failure, with a message logged. + //! + //! \note In test code, use test::TestPaths::Executable() instead. + static bool Executable(base::FilePath* path); + + DISALLOW_IMPLICIT_CONSTRUCTORS(Paths); +}; + +} // namespace crashpad + +#endif // CRASHPAD_UTIL_TEST_PATHS_H_ diff --git a/test/paths_linux.cc b/util/misc/paths_linux.cc similarity index 78% rename from test/paths_linux.cc rename to util/misc/paths_linux.cc index 6e6da0e0..569944bc 100644 --- a/test/paths_linux.cc +++ b/util/misc/paths_linux.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/paths.h" +#include "util/misc/paths.h" #include #include @@ -21,42 +21,41 @@ #include #include "base/logging.h" -#include "util/misc/implicit_cast.h" namespace crashpad { -namespace test { // static -base::FilePath Paths::Executable() { +bool Paths::Executable(base::FilePath* path) { // Linux does not provide a straightforward way to size the buffer before // calling readlink(). Normally, the st_size field returned by lstat() could // be used, but this is usually zero for things in /proc. // // The /proc filesystem does not provide any way to read “exe” links for - // pathnames longer than a page. See linux-4.4.27/fs/proc/base.c + // pathnames longer than a page. See linux-4.9.20/fs/proc/base.c // do_proc_readlink(), which allocates a single page to receive the path // string. Coincidentally, the page size and PATH_MAX are normally the same // value, although neither is strictly a limit on the length of a pathname. // // On Android, the smaller of the page size and PATH_MAX actually does serve // as an effective limit on the length of an executable’s pathname. See - // Android 7.0.0 bionic/linker/linker.cpp get_executable_path(), which aborts + // Android 7.1.1 bionic/linker/linker.cpp get_executable_path(), which aborts // via __libc_fatal() if the “exe” link can’t be read into a PATH_MAX-sized // buffer. - std::string exe_path(std::max(implicit_cast(sysconf(_SC_PAGESIZE)), - implicit_cast(PATH_MAX)), + std::string exe_path(std::max(getpagesize(), PATH_MAX), std::string::value_type()); ssize_t exe_path_len = readlink("/proc/self/exe", &exe_path[0], exe_path.size()); if (exe_path_len < 0) { - PLOG(FATAL) << "readlink"; + PLOG(ERROR) << "readlink"; + return false; } else if (static_cast(exe_path_len) >= exe_path.size()) { - LOG(FATAL) << "readlink"; + LOG(ERROR) << "readlink"; + return false; } exe_path.resize(exe_path_len); - return base::FilePath(exe_path); + *path = base::FilePath(exe_path); + return true; } -} // namespace test } // namespace crashpad diff --git a/test/paths_mac.cc b/util/misc/paths_mac.cc similarity index 76% rename from test/paths_mac.cc rename to util/misc/paths_mac.cc index d16049e8..036f84b5 100644 --- a/test/paths_mac.cc +++ b/util/misc/paths_mac.cc @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/paths.h" +#include "util/misc/paths.h" #include #include @@ -20,20 +20,25 @@ #include "base/logging.h" namespace crashpad { -namespace test { // static -base::FilePath Paths::Executable() { +bool Paths::Executable(base::FilePath* path) { uint32_t executable_length = 0; _NSGetExecutablePath(nullptr, &executable_length); - CHECK_GT(executable_length, 1u); + if (executable_length <= 1) { + LOG(ERROR) << "_NSGetExecutablePath"; + return false; + } std::string executable_path(executable_length - 1, std::string::value_type()); int rv = _NSGetExecutablePath(&executable_path[0], &executable_length); - CHECK_EQ(rv, 0); + if (rv != 0) { + LOG(ERROR) << "_NSGetExecutablePath"; + return false; + } - return base::FilePath(executable_path); + *path = base::FilePath(executable_path); + return true; } -} // namespace test } // namespace crashpad diff --git a/util/misc/paths_test.cc b/util/misc/paths_test.cc new file mode 100644 index 00000000..c923d139 --- /dev/null +++ b/util/misc/paths_test.cc @@ -0,0 +1,39 @@ +// Copyright 2014 The Crashpad Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "util/misc/paths.h" + +#include "base/files/file_path.h" +#include "build/build_config.h" +#include "gtest/gtest.h" + +namespace crashpad { +namespace test { +namespace { + +TEST(Paths, Executable) { + base::FilePath executable_path; + ASSERT_TRUE(Paths::Executable(&executable_path)); + const base::FilePath executable_name(executable_path.BaseName()); +#if defined(OS_WIN) + EXPECT_EQ(FILE_PATH_LITERAL("crashpad_util_test.exe"), + executable_name.value()); +#else + EXPECT_EQ("crashpad_util_test", executable_name.value()); +#endif // OS_WIN +} + +} // namespace +} // namespace test +} // namespace crashpad diff --git a/test/paths_win.cc b/util/misc/paths_win.cc similarity index 72% rename from test/paths_win.cc rename to util/misc/paths_win.cc index 947acd5c..4c402fe9 100644 --- a/test/paths_win.cc +++ b/util/misc/paths_win.cc @@ -12,23 +12,29 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "test/paths.h" +#include "util/misc/paths.h" #include #include "base/logging.h" namespace crashpad { -namespace test { // static -base::FilePath Paths::Executable() { +bool Paths::Executable(base::FilePath* path) { wchar_t executable_path[_MAX_PATH]; unsigned int len = GetModuleFileName(nullptr, executable_path, arraysize(executable_path)); - PCHECK(len != 0 && len < arraysize(executable_path)) << "GetModuleFileName"; - return base::FilePath(executable_path); + if (len == 0) { + PLOG(ERROR) << "GetModuleFileName"; + return false; + } else if (len >= arraysize(executable_path)) { + LOG(ERROR) << "GetModuleFileName"; + return false; + } + + *path = base::FilePath(executable_path); + return true; } -} // namespace test } // namespace crashpad diff --git a/util/net/http_body_test.cc b/util/net/http_body_test.cc index eb67f106..9478a224 100644 --- a/util/net/http_body_test.cc +++ b/util/net/http_body_test.cc @@ -17,7 +17,7 @@ #include #include "gtest/gtest.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "util/misc/implicit_cast.h" #include "util/net/http_body_test_util.h" @@ -97,7 +97,7 @@ TEST(StringHTTPBodyStream, MultipleReads) { } TEST(FileHTTPBodyStream, ReadASCIIFile) { - base::FilePath path = Paths::TestDataRoot().Append( + base::FilePath path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt")); FileHTTPBodyStream stream(path); std::string contents = ReadStreamToString(&stream, 32); @@ -115,7 +115,7 @@ TEST(FileHTTPBodyStream, ReadASCIIFile) { TEST(FileHTTPBodyStream, ReadBinaryFile) { // HEX contents of file: |FEEDFACE A11A15|. - base::FilePath path = Paths::TestDataRoot().Append( + base::FilePath path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/binary_http_body.dat")); // This buffer size was chosen so that reading the file takes multiple reads. uint8_t buf[4]; @@ -199,7 +199,7 @@ TEST_P(CompositeHTTPBodyStreamBufferSize, StringsAndFile) { std::vector parts; parts.push_back(new StringHTTPBodyStream(string1)); - base::FilePath path = Paths::TestDataRoot().Append( + base::FilePath path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt")); parts.push_back(new FileHTTPBodyStream(path)); parts.push_back(new StringHTTPBodyStream(string2)); diff --git a/util/net/http_multipart_builder_test.cc b/util/net/http_multipart_builder_test.cc index fa20abe4..621f5aef 100644 --- a/util/net/http_multipart_builder_test.cc +++ b/util/net/http_multipart_builder_test.cc @@ -20,7 +20,7 @@ #include "gtest/gtest.h" #include "test/gtest_death_check.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "util/net/http_body.h" #include "util/net/http_body_test_util.h" @@ -100,7 +100,7 @@ TEST(HTTPMultipartBuilder, ThreeStringFields) { TEST(HTTPMultipartBuilder, ThreeFileAttachments) { HTTPMultipartBuilder builder; - base::FilePath ascii_http_body_path = Paths::TestDataRoot().Append( + base::FilePath ascii_http_body_path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt")); builder.SetFileAttachment("first", "minidump.dmp", @@ -186,7 +186,7 @@ TEST(HTTPMultipartBuilder, OverwriteFileAttachment) { const char kValue[] = "1 2 3 test"; builder.SetFormData("a key", kValue); base::FilePath testdata_path = - Paths::TestDataRoot().Append(FILE_PATH_LITERAL("util/net/testdata")); + TestPaths::TestDataRoot().Append(FILE_PATH_LITERAL("util/net/testdata")); builder.SetFileAttachment("minidump", "minidump.dmp", testdata_path.Append(FILE_PATH_LITERAL( @@ -242,7 +242,7 @@ TEST(HTTPMultipartBuilder, SharedFormDataAndAttachmentKeyNamespace) { HTTPMultipartBuilder builder; const char kValue1[] = "11111"; builder.SetFormData("one", kValue1); - base::FilePath ascii_http_body_path = Paths::TestDataRoot().Append( + base::FilePath ascii_http_body_path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/testdata/ascii_http_body.txt")); builder.SetFileAttachment("minidump", "minidump.dmp", diff --git a/util/net/http_transport_test.cc b/util/net/http_transport_test.cc index cb5c4f1f..bd30b068 100644 --- a/util/net/http_transport_test.cc +++ b/util/net/http_transport_test.cc @@ -31,7 +31,7 @@ #include "build/build_config.h" #include "gtest/gtest.h" #include "test/multiprocess_exec.h" -#include "test/paths.h" +#include "test/test_paths.h" #include "util/file/file_io.h" #include "util/misc/random_string.h" #include "util/net/http_body.h" @@ -56,7 +56,7 @@ class HTTPTransportTestFixture : public MultiprocessExec { body_stream_(std::move(body_stream)), response_code_(http_response_code), request_validator_(request_validator) { - base::FilePath server_path = Paths::TestDataRoot().Append( + base::FilePath server_path = TestPaths::TestDataRoot().Append( FILE_PATH_LITERAL("util/net/http_transport_test_server.py")); #if defined(OS_POSIX) SetChildCommand(server_path.value(), nullptr); diff --git a/util/util.gyp b/util/util.gyp index 5f5e3a38..b31bfb3e 100644 --- a/util/util.gyp +++ b/util/util.gyp @@ -99,6 +99,10 @@ 'misc/initialization_state_dcheck.h', 'misc/metrics.cc', 'misc/metrics.h', + 'misc/paths.h', + 'misc/paths_mac.cc', + 'misc/paths_linux.cc', + 'misc/paths_win.cc', 'misc/pdb_structures.cc', 'misc/pdb_structures.h', 'misc/random_string.cc', @@ -321,6 +325,7 @@ 'target_conditions': [ ['OS=="android"', { 'sources/': [ + ['include', '^misc/paths_linux\\.cc$'], ['include', '^posix/process_info_linux\\.cc$'], ], }], diff --git a/util/util_test.gyp b/util/util_test.gyp index d090a1ff..9749fa0b 100644 --- a/util/util_test.gyp +++ b/util/util_test.gyp @@ -62,6 +62,7 @@ 'misc/clock_test.cc', 'misc/initialization_state_dcheck_test.cc', 'misc/initialization_state_test.cc', + 'misc/paths_test.cc', 'misc/scoped_forbid_return_test.cc', 'misc/random_string_test.cc', 'misc/uuid_test.cc', diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index e0d89697..42ee7249 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -26,8 +26,8 @@ #include "build/build_config.h" #include "gtest/gtest.h" #include "test/errors.h" -#include "test/paths.h" #include "test/scoped_temp_dir.h" +#include "test/test_paths.h" #include "test/win/child_launcher.h" #include "util/file/file_io.h" #include "util/misc/random_string.h" @@ -139,7 +139,7 @@ void TestOtherProcess(const base::string16& directory_modification) { CreateEvent(nullptr, true, false, done_uuid.ToString16().c_str())); ASSERT_TRUE(done.get()) << ErrorMessage("CreateEvent"); - base::FilePath test_executable = Paths::Executable(); + base::FilePath test_executable = TestPaths::Executable(); std::wstring child_test_executable = test_executable.DirName()