From 04036023934373c1fc4bbd8e29da70d7bb4a46b8 Mon Sep 17 00:00:00 2001 From: Scott Graham Date: Fri, 16 Feb 2018 09:55:34 -0800 Subject: [PATCH] Fix CrashpadInfoSizes_ClientOptions/CrashpadInfoSizes_ClientOptions These tests needed to be updated to expose CrashpadInfo in the same way as the main CrashpadInfo g_crashpad_info is found on Linux/Android/Fuchsia. Unfortunately, while the tests pass on Fuchsia when run in isolation, the implementation of dlclose() on Fuchsia currently does nothing. So, if the full test suite is run, there's interference between the test modules (i.e. the values in _small vs. the values in _large), so the tests fail. I filed ZX-1728 upstream about this to see if it might be implemented, or if the test will need to spawn a clean child to do the module load tests in. Bug: crashpad:196 Change-Id: I9ee01b142a29c508c6967dc83da824afa254d379 Reviewed-on: https://chromium-review.googlesource.com/923182 Reviewed-by: Mark Mentovai Commit-Queue: Scott Graham --- client/crashpad_info.cc | 8 ++-- snapshot/BUILD.gn | 22 ++++++--- snapshot/crashpad_info_size_test_module.cc | 13 ++--- snapshot/crashpad_info_size_test_note.S | 56 ++++++++++++++++++++++ snapshot/snapshot_test.gyp | 26 ++++++++++ 5 files changed, 107 insertions(+), 18 deletions(-) create mode 100644 snapshot/crashpad_info_size_test_note.S diff --git a/client/crashpad_info.cc b/client/crashpad_info.cc index 098b819a..7c1316e8 100644 --- a/client/crashpad_info.cc +++ b/client/crashpad_info.cc @@ -59,10 +59,6 @@ __attribute__(( // Put the structure in a well-known section name where it can be easily // found without having to consult the symbol table. section(SEG_DATA ",crashpad_info"), - - // There's no need to expose this as a public symbol from the symbol table. - // All accesses from the outside can locate the well-known section name. - visibility("hidden"), #endif #if defined(ADDRESS_SANITIZER) @@ -74,6 +70,10 @@ __attribute__(( aligned(64), #endif // defined(ADDRESS_SANITIZER) + // There's no need to expose this as a public symbol from the symbol table. + // All accesses from the outside can locate the well-known section name. + visibility("hidden"), + // The “used” attribute prevents the structure from being dead-stripped. used)) diff --git a/snapshot/BUILD.gn b/snapshot/BUILD.gn index f4a592b1..0fe97d3f 100644 --- a/snapshot/BUILD.gn +++ b/snapshot/BUILD.gn @@ -412,10 +412,15 @@ loadable_module("crashpad_snapshot_test_module_large") { sources = [ "crashpad_info_size_test_module.cc", ] + + deps = [] + if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) { + sources += [ "crashpad_info_size_test_note.S" ] + deps += [ "../util" ] + } + defines = [ "CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE" ] - deps = [ - "../third_party/mini_chromium:base", - ] + deps += [ "../third_party/mini_chromium:base" ] } loadable_module("crashpad_snapshot_test_module_small") { @@ -423,10 +428,15 @@ loadable_module("crashpad_snapshot_test_module_small") { sources = [ "crashpad_info_size_test_module.cc", ] + + deps = [] + if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) { + sources += [ "crashpad_info_size_test_note.S" ] + deps += [ "../util" ] + } + defines = [ "CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL" ] - deps = [ - "../third_party/mini_chromium:base", - ] + deps += [ "../third_party/mini_chromium:base" ] } if (crashpad_is_linux || crashpad_is_android || crashpad_is_fuchsia) { diff --git a/snapshot/crashpad_info_size_test_module.cc b/snapshot/crashpad_info_size_test_module.cc index d39fada3..53f4a3ff 100644 --- a/snapshot/crashpad_info_size_test_module.cc +++ b/snapshot/crashpad_info_size_test_module.cc @@ -23,7 +23,6 @@ #endif // OS_MACOSX namespace crashpad { -namespace { #if defined(CRASHPAD_INFO_SIZE_TEST_MODULE_SMALL) == \ defined(CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE) @@ -70,16 +69,12 @@ struct TestCrashpadInfo { __attribute__(( #if defined(OS_MACOSX) section(SEG_DATA ",crashpad_info"), -#elif defined(OS_LINUX) || defined(OS_ANDROID) || defined(OS_FUCHSIA) - section("crashpad_info"), -#else -#error Port #endif #if defined(ADDRESS_SANITIZER) aligned(64), #endif // defined(ADDRESS_SANITIZER) - used, - visibility("hidden"))) + visibility("hidden"), + used)) #elif defined(OS_WIN) #pragma section("CPADinfo", read, write) __declspec(allocate("CPADinfo")) @@ -106,7 +101,6 @@ TestCrashpadInfo g_test_crashpad_info = {'CPad', #endif // CRASHPAD_INFO_SIZE_TEST_MODULE_LARGE }; -} // namespace } // namespace crashpad extern "C" { @@ -119,6 +113,9 @@ __declspec(dllexport) #error Port #endif // OS_POSIX crashpad::TestCrashpadInfo* TestModule_GetCrashpadInfo() { + // Note that there's no need to do the back-reference here to the note on + // POSIX like CrashpadInfo::GetCrashpadInfo() because the note .S file is + // directly included into this test binary. return &crashpad::g_test_crashpad_info; } diff --git a/snapshot/crashpad_info_size_test_note.S b/snapshot/crashpad_info_size_test_note.S new file mode 100644 index 00000000..a355a831 --- /dev/null +++ b/snapshot/crashpad_info_size_test_note.S @@ -0,0 +1,56 @@ +// Copyright 2018 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. + +// This note section is used on ELF platforms to give ElfImageReader a method +// of finding the instance of CrashpadInfo g_crashpad_info without requiring +// that symbol to be in the dynamic symbol table. + +#include "build/build_config.h" +#include "util/misc/elf_note_types.h" + +// namespace crashpad { +// CrashpadInfo g_test_crashpad_info; +// } // namespace crashpad +#define TEST_CRASHPAD_INFO_SYMBOL _ZN8crashpad20g_test_crashpad_infoE + +#define NOTE_ALIGN 4 + + // This section must be "a"llocated so that it appears in the final binary at + // runtime, and "w"ritable so that the relocation to TEST_CRASHPAD_INFO_SYMBOL + // can be performed. + .section .note.crashpad.info,"aw",%note + .balign NOTE_ALIGN + .type info_size_test_note, %object +info_size_test_note: + .long name_end - name // namesz + .long desc_end - desc // descsz + .long CRASHPAD_ELF_NOTE_TYPE_CRASHPAD_INFO // type +name: + .asciz CRASHPAD_ELF_NOTE_NAME +name_end: + .balign NOTE_ALIGN +desc: +#if defined(ARCH_CPU_64_BITS) + .quad TEST_CRASHPAD_INFO_SYMBOL +#else +#if defined(ARCH_CPU_LITTLE_ENDIAN) + .long TEST_CRASHPAD_INFO_SYMBOL + .long 0 +#else + .long 0 + .long TEST_CRASHPAD_INFO_SYMBOL +#endif // ARCH_CPU_LITTLE_ENDIAN +#endif // ARCH_CPU_64_BITS +desc_end: + .size info_size_test_note, .-info_size_test_note diff --git a/snapshot/snapshot_test.gyp b/snapshot/snapshot_test.gyp index 277bf6e8..d8c0c08f 100644 --- a/snapshot/snapshot_test.gyp +++ b/snapshot/snapshot_test.gyp @@ -182,6 +182,19 @@ 'sources': [ 'crashpad_info_size_test_module.cc', ], + 'include_dirs': [ + '..', + ], + 'conditions': [ + ['OS=="linux" or OS=="android"', { + 'sources': [ + 'crashpad_info_size_test_note.S', + ], + 'dependencies': [ + '../util/util.gyp:crashpad_util', + ], + }], + ], }, { 'target_name': 'crashpad_snapshot_test_module_small', @@ -195,6 +208,19 @@ 'sources': [ 'crashpad_info_size_test_module.cc', ], + 'include_dirs': [ + '..', + ], + 'conditions': [ + ['OS=="linux" or OS=="android"', { + 'sources': [ + 'crashpad_info_size_test_note.S', + ], + 'dependencies': [ + '../util/util.gyp:crashpad_util', + ], + }], + ], }, { 'target_name': 'crashpad_snapshot_test_both_dt_hash_styles',