From 2f481590112b42aa21010abceaa3742e50775875 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Wed, 25 Oct 2017 14:04:46 -0400 Subject: [PATCH] Make crashpad_util_test build without warnings with clang-cl on Windows. This upstreams https://chromium-review.googlesource.com/c/chromium/src/+/735820/ Bug: chromium:777924 Change-Id: I9fe76b839442d73a6c2836ccfe6cbe41acd67fad Reviewed-on: https://chromium-review.googlesource.com/738394 Reviewed-by: Mark Mentovai Commit-Queue: Nico Weber --- test/scoped_temp_dir_win.cc | 2 +- test/win/win_child_process.cc | 2 +- util/stdlib/strlcpy_test.cc | 47 ++++++++---- util/win/capture_context_test.cc | 14 ++-- util/win/command_line_test.cc | 16 ++--- util/win/exception_handler_server_test.cc | 1 - util/win/process_info_test.cc | 87 ++++++++++++----------- util/win/safe_terminate_process_test.cc | 8 +-- 8 files changed, 99 insertions(+), 78 deletions(-) diff --git a/test/scoped_temp_dir_win.cc b/test/scoped_temp_dir_win.cc index 9c220a12..ab4ff957 100644 --- a/test/scoped_temp_dir_win.cc +++ b/test/scoped_temp_dir_win.cc @@ -34,7 +34,7 @@ base::FilePath GenerateCandidateName() { PCHECK(path_len != 0) << "GetTempPath"; base::FilePath system_temp_dir(temp_path); base::string16 new_dir_name = base::UTF8ToUTF16(base::StringPrintf( - "crashpad.test.%d.%s", GetCurrentProcessId(), RandomString().c_str())); + "crashpad.test.%lu.%s", GetCurrentProcessId(), RandomString().c_str())); return system_temp_dir.Append(new_dir_name); } diff --git a/test/win/win_child_process.cc b/test/win/win_child_process.cc index 44177f63..74687ee1 100644 --- a/test/win/win_child_process.cc +++ b/test/win/win_child_process.cc @@ -213,7 +213,7 @@ std::unique_ptr WinChildProcess::Launch() { return std::unique_ptr(); } - return std::move(handles_for_parent); + return handles_for_parent; } FileHandle WinChildProcess::ReadPipeHandle() const { diff --git a/util/stdlib/strlcpy_test.cc b/util/stdlib/strlcpy_test.cc index 5185840f..5d20e193 100644 --- a/util/stdlib/strlcpy_test.cc +++ b/util/stdlib/strlcpy_test.cc @@ -15,6 +15,7 @@ #include "util/stdlib/strlcpy.h" #include +#include #include @@ -28,6 +29,25 @@ namespace crashpad { namespace test { namespace { +// The base::c16 functions only exist if WCHAR_T_IS_UTF32. +#if defined(WCHAR_T_IS_UTF32) +size_t C16Len(const base::char16* s) { + return base::c16len(s); +} + +int C16Memcmp(const base::char16* s1, const base::char16* s2, size_t n) { + return base::c16memcmp(s1, s2, n); +} +#elif defined(WCHAR_T_IS_UTF16) +size_t C16Len(const base::char16* s) { + return wcslen(s); +} + +int C16Memcmp(const base::char16* s1, const base::char16* s2, size_t n) { + return wmemcmp(s1, s2, n); +} +#endif + TEST(strlcpy, c16lcpy) { // Use a destination buffer that’s larger than the length passed to c16lcpy. // The unused portion is a guard area that must not be written to. @@ -67,26 +87,25 @@ TEST(strlcpy, c16lcpy) { std::min(length, arraysize(destination.data) - 1); EXPECT_EQ(destination.data[expected_destination_length], '\0'); - EXPECT_EQ(base::c16len(destination.data), expected_destination_length); - EXPECT_TRUE(base::c16memcmp(test_string.c_str(), - destination.data, - expected_destination_length) == 0); + EXPECT_EQ(C16Len(destination.data), expected_destination_length); + EXPECT_TRUE(C16Memcmp(test_string.c_str(), + destination.data, + expected_destination_length) == 0); // Make sure that the portion of the destination buffer that was not used // was not touched. This includes the guard areas and the unused portion // of the buffer passed to c16lcpy. - EXPECT_TRUE(base::c16memcmp(expected_untouched.lead_guard, - destination.lead_guard, - arraysize(destination.lead_guard)) == 0); + EXPECT_TRUE(C16Memcmp(expected_untouched.lead_guard, + destination.lead_guard, + arraysize(destination.lead_guard)) == 0); size_t expected_untouched_length = arraysize(destination.data) - expected_destination_length - 1; - EXPECT_TRUE( - base::c16memcmp(expected_untouched.data, - &destination.data[expected_destination_length + 1], - expected_untouched_length) == 0); - EXPECT_TRUE(base::c16memcmp(expected_untouched.trail_guard, - destination.trail_guard, - arraysize(destination.trail_guard)) == 0); + EXPECT_TRUE(C16Memcmp(expected_untouched.data, + &destination.data[expected_destination_length + 1], + expected_untouched_length) == 0); + EXPECT_TRUE(C16Memcmp(expected_untouched.trail_guard, + destination.trail_guard, + arraysize(destination.trail_guard)) == 0); } } } diff --git a/util/win/capture_context_test.cc b/util/win/capture_context_test.cc index 4bb6f4f3..ca2729bc 100644 --- a/util/win/capture_context_test.cc +++ b/util/win/capture_context_test.cc @@ -41,7 +41,7 @@ void SanityCheckContext(const CONTEXT& context) { CONTEXT_FLOATING_POINT; ASSERT_EQ(context.ContextFlags & must_have, must_have); constexpr uint32_t may_have = CONTEXT_EXTENDED_REGISTERS; - ASSERT_EQ(context.ContextFlags & ~(must_have | may_have), 0); + ASSERT_EQ(context.ContextFlags & ~(must_have | may_have), 0u); #elif defined(ARCH_CPU_X86_64) ASSERT_EQ(context.ContextFlags, CONTEXT_AMD64 | CONTEXT_CONTROL | CONTEXT_INTEGER | @@ -61,12 +61,12 @@ void SanityCheckContext(const CONTEXT& context) { EXPECT_EQ(context.EFlags & 0xffc0802a, 2u); // CaptureContext() doesn’t capture debug registers, so make sure they read 0. - EXPECT_EQ(context.Dr0, 0); - EXPECT_EQ(context.Dr1, 0); - EXPECT_EQ(context.Dr2, 0); - EXPECT_EQ(context.Dr3, 0); - EXPECT_EQ(context.Dr6, 0); - EXPECT_EQ(context.Dr7, 0); + EXPECT_EQ(context.Dr0, 0u); + EXPECT_EQ(context.Dr1, 0u); + EXPECT_EQ(context.Dr2, 0u); + EXPECT_EQ(context.Dr3, 0u); + EXPECT_EQ(context.Dr6, 0u); + EXPECT_EQ(context.Dr7, 0u); #endif #if defined(ARCH_CPU_X86) diff --git a/util/win/command_line_test.cc b/util/win/command_line_test.cc index 1eb15b88..e7761f12 100644 --- a/util/win/command_line_test.cc +++ b/util/win/command_line_test.cc @@ -43,7 +43,7 @@ void AppendCommandLineArgumentTest(size_t argc, const wchar_t* const argv[]) { ASSERT_TRUE(test_argv) << ErrorMessage("CommandLineToArgvW"); ScopedLocalAlloc test_argv_owner(test_argv); - ASSERT_EQ(test_argc, argc); + ASSERT_EQ(test_argc, static_cast(argc)); for (size_t index = 0; index < argc; ++index) { EXPECT_STREQ(argv[index], test_argv[index]) << "index " << index; @@ -60,7 +60,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("simple"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"argument 1", L"argument 2", @@ -71,7 +71,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("path with spaces"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"argument1", L"argument 2", @@ -83,7 +83,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("argument with embedded quotation marks"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"argument1", L"she said, \"you had me at hello\"", @@ -95,7 +95,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("argument with unbalanced quotation marks"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"argument1", L"argument\"2", @@ -108,7 +108,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("argument ending with backslash"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"\\some\\directory with\\spaces\\", L"argument2", @@ -119,7 +119,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("empty argument"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"", L"argument2", @@ -130,7 +130,7 @@ TEST(CommandLine, AppendCommandLineArgument) { { SCOPED_TRACE("funny nonprintable characters"); - static constexpr wchar_t* const kArguments[] = { + static constexpr const wchar_t* kArguments[] = { L"child.exe", L"argument 1", L"argument\t2", diff --git a/util/win/exception_handler_server_test.cc b/util/win/exception_handler_server_test.cc index c1a076b8..6f880516 100644 --- a/util/win/exception_handler_server_test.cc +++ b/util/win/exception_handler_server_test.cc @@ -72,7 +72,6 @@ class TestDelegate : public ExceptionHandlerServer::Delegate { private: HANDLE server_ready_; // weak - bool started_; DISALLOW_COPY_AND_ASSIGN(TestDelegate); }; diff --git a/util/win/process_info_test.cc b/util/win/process_info_test.cc index 07c40bb0..673f440d 100644 --- a/util/win/process_info_test.cc +++ b/util/win/process_info_test.cc @@ -67,9 +67,9 @@ void VerifyAddressInInCodePage(const ProcessInfo& process_info, for (const auto& mi : memory_info) { if (mi.BaseAddress <= code_address && mi.BaseAddress + mi.RegionSize > code_address) { - EXPECT_EQ(mi.State, MEM_COMMIT); - EXPECT_EQ(mi.Protect, PAGE_EXECUTE_READ); - EXPECT_EQ(mi.Type, MEM_IMAGE); + EXPECT_EQ(mi.State, static_cast(MEM_COMMIT)); + EXPECT_EQ(mi.Protect, static_cast(PAGE_EXECUTE_READ)); + EXPECT_EQ(mi.Type, static_cast(MEM_IMAGE)); EXPECT_FALSE(found_region); found_region = true; } @@ -114,8 +114,8 @@ TEST(ProcessInfo, Self) { EXPECT_EQ(modules[1].dll_base, reinterpret_cast(GetModuleHandle(L"ntdll.dll"))); - EXPECT_GT(modules[0].size, 0); - EXPECT_GT(modules[1].size, 0); + EXPECT_GT(modules[0].size, 0u); + EXPECT_GT(modules[1].size, 0u); EXPECT_EQ(modules[0].timestamp, GetTimestampForLoadedLibrary(GetModuleHandle(nullptr))); @@ -164,7 +164,7 @@ void TestOtherProcess(const base::string16& directory_modification) { // Tell the test it's OK to shut down now that we've read our data. EXPECT_TRUE(SetEvent(done.get())) << ErrorMessage("SetEvent"); - EXPECT_EQ(child.WaitForExit(), 0); + EXPECT_EQ(child.WaitForExit(), 0u); std::vector modules; EXPECT_TRUE(process_info.Modules(&modules)); @@ -231,8 +231,8 @@ TEST(ProcessInfo, AccessibleRangesOneInside) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 2); - EXPECT_EQ(result[0].size(), 4); + EXPECT_EQ(result[0].base(), 2u); + EXPECT_EQ(result[0].size(), 4u); } TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { @@ -254,8 +254,8 @@ TEST(ProcessInfo, AccessibleRangesOneTruncatedSize) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 5); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 5u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, AccessibleRangesOneMovedStart) { @@ -277,8 +277,8 @@ TEST(ProcessInfo, AccessibleRangesOneMovedStart) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 10); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 10u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, ReserveIsInaccessible) { @@ -300,8 +300,8 @@ TEST(ProcessInfo, ReserveIsInaccessible) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 10); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 10u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, PageGuardIsInaccessible) { @@ -325,8 +325,8 @@ TEST(ProcessInfo, PageGuardIsInaccessible) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 10); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 10u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, PageNoAccessIsInaccessible) { @@ -350,8 +350,8 @@ TEST(ProcessInfo, PageNoAccessIsInaccessible) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 10); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 10u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, AccessibleRangesCoalesced) { @@ -378,8 +378,8 @@ TEST(ProcessInfo, AccessibleRangesCoalesced) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 11); - EXPECT_EQ(result[0].size(), 4); + EXPECT_EQ(result[0].base(), 11u); + EXPECT_EQ(result[0].size(), 4u); } TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { @@ -406,10 +406,10 @@ TEST(ProcessInfo, AccessibleRangesMiddleUnavailable) { memory_info); ASSERT_EQ(result.size(), 2u); - EXPECT_EQ(result[0].base(), 5); - EXPECT_EQ(result[0].size(), 5); - EXPECT_EQ(result[1].base(), 15); - EXPECT_EQ(result[1].size(), 35); + EXPECT_EQ(result[0].base(), 5u); + EXPECT_EQ(result[0].size(), 5u); + EXPECT_EQ(result[1].base(), 15u); + EXPECT_EQ(result[1].size(), 35u); } TEST(ProcessInfo, RequestedBeforeMap) { @@ -426,8 +426,8 @@ TEST(ProcessInfo, RequestedBeforeMap) { memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 10); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 10u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, RequestedAfterMap) { @@ -444,8 +444,8 @@ TEST(ProcessInfo, RequestedAfterMap) { CheckedRange(15, 100), memory_info); ASSERT_EQ(result.size(), 1u); - EXPECT_EQ(result[0].base(), 15); - EXPECT_EQ(result[0].size(), 5); + EXPECT_EQ(result[0].base(), 15u); + EXPECT_EQ(result[0].size(), 5u); } TEST(ProcessInfo, ReadableRanges) { @@ -562,7 +562,7 @@ TEST(ProcessInfo, Handles) { ASSERT_TRUE(scoped_key.is_valid()); std::wstring mapping_name = - base::UTF8ToUTF16(base::StringPrintf("Local\\test_mapping_%d_%s", + base::UTF8ToUTF16(base::StringPrintf("Local\\test_mapping_%lu_%s", GetCurrentProcessId(), RandomString().c_str())); ScopedKernelHANDLE mapping(CreateFileMapping(INVALID_HANDLE_VALUE, @@ -584,46 +584,49 @@ TEST(ProcessInfo, Handles) { EXPECT_FALSE(found_file_handle); found_file_handle = true; EXPECT_EQ(handle.type_name, L"File"); - EXPECT_EQ(handle.handle_count, 1); + EXPECT_EQ(handle.handle_count, 1u); EXPECT_NE(handle.pointer_count, 0u); EXPECT_EQ(handle.granted_access & STANDARD_RIGHTS_ALL, - STANDARD_RIGHTS_READ | STANDARD_RIGHTS_WRITE | SYNCHRONIZE); - EXPECT_EQ(handle.attributes, 0); + static_cast(STANDARD_RIGHTS_READ | + STANDARD_RIGHTS_WRITE | SYNCHRONIZE)); + EXPECT_EQ(handle.attributes, 0u); } if (handle.handle == HandleToInt(inherited_file.get())) { EXPECT_FALSE(found_inherited_file_handle); found_inherited_file_handle = true; EXPECT_EQ(handle.type_name, L"File"); - EXPECT_EQ(handle.handle_count, 1); + EXPECT_EQ(handle.handle_count, 1u); EXPECT_NE(handle.pointer_count, 0u); EXPECT_EQ(handle.granted_access & STANDARD_RIGHTS_ALL, - STANDARD_RIGHTS_READ | STANDARD_RIGHTS_WRITE | SYNCHRONIZE); + static_cast(STANDARD_RIGHTS_READ | + STANDARD_RIGHTS_WRITE | SYNCHRONIZE)); // OBJ_INHERIT from ntdef.h, but including that conflicts with other // headers. - constexpr int kObjInherit = 0x2; + constexpr uint32_t kObjInherit = 0x2; EXPECT_EQ(handle.attributes, kObjInherit); } if (handle.handle == HandleToInt(scoped_key.get())) { EXPECT_FALSE(found_key_handle); found_key_handle = true; EXPECT_EQ(handle.type_name, L"Key"); - EXPECT_EQ(handle.handle_count, 1); + EXPECT_EQ(handle.handle_count, 1u); EXPECT_NE(handle.pointer_count, 0u); EXPECT_EQ(handle.granted_access & STANDARD_RIGHTS_ALL, - STANDARD_RIGHTS_READ); - EXPECT_EQ(handle.attributes, 0); + static_cast(STANDARD_RIGHTS_READ)); + EXPECT_EQ(handle.attributes, 0u); } if (handle.handle == HandleToInt(mapping.get())) { EXPECT_FALSE(found_mapping_handle); found_mapping_handle = true; EXPECT_EQ(handle.type_name, L"Section"); - EXPECT_EQ(handle.handle_count, 1); + EXPECT_EQ(handle.handle_count, 1u); EXPECT_NE(handle.pointer_count, 0u); EXPECT_EQ(handle.granted_access & STANDARD_RIGHTS_ALL, - DELETE | READ_CONTROL | WRITE_DAC | WRITE_OWNER | - STANDARD_RIGHTS_READ | STANDARD_RIGHTS_WRITE); - EXPECT_EQ(handle.attributes, 0); + static_cast(DELETE | READ_CONTROL | WRITE_DAC | + WRITE_OWNER | STANDARD_RIGHTS_READ | + STANDARD_RIGHTS_WRITE)); + EXPECT_EQ(handle.attributes, 0u); } } EXPECT_TRUE(found_file_handle); diff --git a/util/win/safe_terminate_process_test.cc b/util/win/safe_terminate_process_test.cc index d176002e..e3bfde7f 100644 --- a/util/win/safe_terminate_process_test.cc +++ b/util/win/safe_terminate_process_test.cc @@ -117,13 +117,13 @@ TEST(SafeTerminateProcess, PatchBadly) { // Make sure that TerminateProcess() works as a baseline. SetLastError(ERROR_SUCCESS); EXPECT_FALSE(TerminateProcess(process, 0)); - EXPECT_EQ(GetLastError(), ERROR_ACCESS_DENIED); + EXPECT_EQ(GetLastError(), static_cast(ERROR_ACCESS_DENIED)); // Make sure that SafeTerminateProcess() works, calling through to // TerminateProcess() properly. SetLastError(ERROR_SUCCESS); EXPECT_FALSE(SafeTerminateProcess(process, 0)); - EXPECT_EQ(GetLastError(), ERROR_ACCESS_DENIED); + EXPECT_EQ(GetLastError(), static_cast(ERROR_ACCESS_DENIED)); { // Patch TerminateProcess() badly. This turns it into a no-op that returns 0 @@ -152,14 +152,14 @@ TEST(SafeTerminateProcess, PatchBadly) { // patched with a no-op stub, GetLastError() shouldn’t be modified. SetLastError(ERROR_SUCCESS); EXPECT_FALSE(SafeTerminateProcess(process, 0)); - EXPECT_EQ(GetLastError(), ERROR_SUCCESS); + EXPECT_EQ(GetLastError(), static_cast(ERROR_SUCCESS)); } // Now that the real TerminateProcess() has been restored, verify that it // still works properly. SetLastError(ERROR_SUCCESS); EXPECT_FALSE(SafeTerminateProcess(process, 0)); - EXPECT_EQ(GetLastError(), ERROR_ACCESS_DENIED); + EXPECT_EQ(GetLastError(), static_cast(ERROR_ACCESS_DENIED)); } TEST(SafeTerminateProcess, TerminateChild) {