From 6108d2523297a7f3f0ea714b4e6229325f5396ce Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Wed, 14 Jun 2017 13:24:35 -0400 Subject: [PATCH] mac: Update the process_types version of dyld_all_image_infos for 10.13 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 10.13 introduces two new fields to dyld_all_image_infos. Oddly, it doesn’t put them in the “reserved” area that was defined in this structure. This addition made it necessary for the padding problem in the 32-bit structure previously worked around in Crashpad to be addressed in the native structure, so Crashpad’s definition is adapted to match. This fixes tests on 10.13 that verify that dyld_all_image_infos can be interpreted correctly. Note that although the 10.13 SDK includes this structure extension, numbered version 16, 10.13db1 17A264c continues to use version 15 as used on 10.12, at least in crashpad_snapshot_test. Bug: crashpad:185 Test: crashpad_snapshot_test ProcessTypes.DyldImagesSelf Change-Id: I59a80c85bb234ef698c65a0ac5bbeac5b40fda77 Reviewed-on: https://chromium-review.googlesource.com/535394 Reviewed-by: Robert Sesek --- snapshot/mac/process_types.cc | 9 +++++ snapshot/mac/process_types.h | 6 +++- snapshot/mac/process_types/custom.cc | 4 ++- .../mac/process_types/dyld_images.proctype | 24 ++++++++----- snapshot/mac/process_types/traits.h | 1 + snapshot/mac/process_types_test.cc | 34 ++++++++++++++----- 6 files changed, 60 insertions(+), 18 deletions(-) diff --git a/snapshot/mac/process_types.cc b/snapshot/mac/process_types.cc index 53d89035..aeff88bb 100644 --- a/snapshot/mac/process_types.cc +++ b/snapshot/mac/process_types.cc @@ -41,6 +41,15 @@ inline void Assign(Type* destination, const Type& source) { memcpy(destination, &source, sizeof(source)); } +template <> +inline void Assign( + process_types::internal::Reserved32_32Only64* destination, + const process_types::internal::Reserved32_32Only32& source) { + // Reserved32_32Only32 carries no data and has no storage in the 64-bit + // structure. +} + template <> inline void Assign( diff --git a/snapshot/mac/process_types.h b/snapshot/mac/process_types.h index 7274515f..eb397b9b 100644 --- a/snapshot/mac/process_types.h +++ b/snapshot/mac/process_types.h @@ -37,8 +37,10 @@ using Nothing = char[0]; // Some structure definitions differ in 32-bit and 64-bit environments by having // additional “reserved” padding fields present only in the 64-bit environment. -// These Reserved*_64Only* types allow the process_types system to replicate +// These Reserved*_*Only* types allow the process_types system to replicate // these structures more precisely. +using Reserved32_32Only32 = uint32_t; +using Reserved32_32Only64 = Nothing; using Reserved32_64Only32 = Nothing; using Reserved32_64Only64 = uint32_t; using Reserved64_64Only32 = Nothing; @@ -71,6 +73,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) using Pointer = internal::TraitsGeneric::Pointer; \ using IntPtr = internal::TraitsGeneric::IntPtr; \ using UIntPtr = internal::TraitsGeneric::UIntPtr; \ + using Reserved32_32Only = internal::TraitsGeneric::Reserved32_32Only; \ using Reserved32_64Only = internal::TraitsGeneric::Reserved32_64Only; \ using Reserved64_64Only = internal::TraitsGeneric::Reserved64_64Only; \ using Nothing = internal::TraitsGeneric::Nothing; \ @@ -162,6 +165,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) using Pointer = typename Traits::Pointer; \ using IntPtr = typename Traits::IntPtr; \ using UIntPtr = typename Traits::UIntPtr; \ + using Reserved32_32Only = typename Traits::Reserved32_32Only; \ using Reserved32_64Only = typename Traits::Reserved32_64Only; \ using Reserved64_64Only = typename Traits::Reserved64_64Only; \ using Nothing = typename Traits::Nothing; \ diff --git a/snapshot/mac/process_types/custom.cc b/snapshot/mac/process_types/custom.cc index c896ebf6..7839b403 100644 --- a/snapshot/mac/process_types/custom.cc +++ b/snapshot/mac/process_types/custom.cc @@ -80,7 +80,9 @@ size_t dyld_all_image_infos::ExpectedSizeForVersion( offsetof(dyld_all_image_infos, sharedCacheSlide), // 11 offsetof(dyld_all_image_infos, sharedCacheUUID), // 12 offsetof(dyld_all_image_infos, infoArrayChangeTimestamp), // 13 - offsetof(dyld_all_image_infos, end), // 14 + offsetof(dyld_all_image_infos, end_14_15), // 14 + offsetof(dyld_all_image_infos, end_14_15), // 15 + sizeof(dyld_all_image_infos), // 16 }; if (version >= arraysize(kSizeForVersion)) { diff --git a/snapshot/mac/process_types/dyld_images.proctype b/snapshot/mac/process_types/dyld_images.proctype index 3470ee83..f92a8009 100644 --- a/snapshot/mac/process_types/dyld_images.proctype +++ b/snapshot/mac/process_types/dyld_images.proctype @@ -116,20 +116,28 @@ PROCESS_TYPE_STRUCT_BEGIN(dyld_all_image_infos) // Version 14 (OS X 10.9) // As of the 10.12 SDK, this is declared as reserved[9] for 64-bit platforms - // and reserved[4] for 32-bit platforms. + // and reserved[4] for 32-bit platforms. It was expanded to reserved[5] for + // 32-bit platforms in the 10.13 SDK to provide proper padding, but because + // the runtimes that use versions 14 and 15 were built with SDKs that did not + // have this extra padding, it’s necessary to treat the element at index 4 on + // 32-bit systems as outside of the version 14 and 15 structure. This is why + // |reserved| is only declared a 4-element array, with a special end_14_15 + // member (not present in the native definition) available to indicate the + // end of the native version 14 and 15 structure, preceding the padding in the + // 32-bit structure that would natively be addressed at index 4 of |reserved|. + // Treat reserved_4_32 as only available in version 16 of the structure. PROCESS_TYPE_STRUCT_MEMBER(UIntPtr, reserved, [4]) - PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_4) + PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_4_64) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_5) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_6) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_7) PROCESS_TYPE_STRUCT_MEMBER(Reserved64_64Only, reserved_8) + PROCESS_TYPE_STRUCT_MEMBER(Nothing, end_14_15) + PROCESS_TYPE_STRUCT_MEMBER(Reserved32_32Only, reserved_4_32) - // The 32-bit version of the structure will have four extra bytes of tail - // padding when built for 64-bit systems than it does natively and when built - // for 32-bit systems. Instead of using sizeof(dyld_all_image_infos), use - // offsetof(dyld_all_image_infos, end) to avoid taking this tail padding into - // account. - PROCESS_TYPE_STRUCT_MEMBER(Nothing, end) + // Version 16 (macOS 10.13) + PROCESS_TYPE_STRUCT_MEMBER(UIntPtr, compact_dyld_image_info_addr) + PROCESS_TYPE_STRUCT_MEMBER(ULong, compact_dyld_image_info_size) // size_t PROCESS_TYPE_STRUCT_END(dyld_all_image_infos) #endif // ! PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO && diff --git a/snapshot/mac/process_types/traits.h b/snapshot/mac/process_types/traits.h index 396799be..613f4f57 100644 --- a/snapshot/mac/process_types/traits.h +++ b/snapshot/mac/process_types/traits.h @@ -36,6 +36,7 @@ using Pointer = uint##lp_bits##_t __VA_ARGS__; \ using IntPtr = int##lp_bits##_t __VA_ARGS__; \ using UIntPtr = uint##lp_bits##_t __VA_ARGS__; \ + using Reserved32_32Only = Reserved32_32Only##lp_bits; \ using Reserved32_64Only = Reserved32_64Only##lp_bits; \ using Reserved64_64Only = Reserved64_64Only##lp_bits; \ using Nothing = Nothing; \ diff --git a/snapshot/mac/process_types_test.cc b/snapshot/mac/process_types_test.cc index 4f5ce7af..8ab15c8d 100644 --- a/snapshot/mac/process_types_test.cc +++ b/snapshot/mac/process_types_test.cc @@ -48,6 +48,11 @@ TEST(ProcessTypes, DyldImagesSelf) { // Get the in-process view of dyld_all_image_infos, and check it for sanity. const dyld_all_image_infos* self_image_infos = DyldGetAllImageInfos(); int mac_os_x_minor_version = MacOSXMinorVersion(); + + // The 10.13 SDK defines dyld_all_image_infos version 16 and says that it’s + // used on 10.13, but 10.13db1 17A264c uses version 15. + // + // TODO(mark): Recheck later in the beta period, up to the 10.13 release. if (mac_os_x_minor_version >= 12) { EXPECT_GE(self_image_infos->version, 15u); } else if (mac_os_x_minor_version >= 9) { @@ -99,16 +104,18 @@ TEST(ProcessTypes, DyldImagesSelf) { ProcessReader process_reader; ASSERT_TRUE(process_reader.Initialize(mach_task_self())); -#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12 - const uint32_t kDyldAllImageInfosVersionInSDK = 15; +#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_13 + constexpr uint32_t kDyldAllImageInfosVersionInSDK = 16; +#elif MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_12 + constexpr uint32_t kDyldAllImageInfosVersionInSDK = 15; #elif MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_9 - const uint32_t kDyldAllImageInfosVersionInSDK = 14; + constexpr uint32_t kDyldAllImageInfosVersionInSDK = 14; #elif MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_7 - const uint32_t kDyldAllImageInfosVersionInSDK = 12; + constexpr uint32_t kDyldAllImageInfosVersionInSDK = 12; #elif MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_6 - const uint32_t kDyldAllImageInfosVersionInSDK = 7; + constexpr uint32_t kDyldAllImageInfosVersionInSDK = 7; #else - const uint32_t kDyldAllImageInfosVersionInSDK = 1; + constexpr uint32_t kDyldAllImageInfosVersionInSDK = 1; #endif // Make sure that the size of the structure as declared in the SDK matches the @@ -119,7 +126,7 @@ TEST(ProcessTypes, DyldImagesSelf) { // Make sure that the computed sizes of various versions of this structure are // correct at different bitnessses. - const struct { + constexpr struct { uint32_t version; size_t size_32; size_t size_64; @@ -138,6 +145,7 @@ TEST(ProcessTypes, DyldImagesSelf) { {13, 104, 184}, {14, 164, 304}, {15, 164, 304}, + {16, 176, 320}, }; for (size_t index = 0; index < arraysize(kVersionsAndSizes); ++index) { uint32_t version = kVersionsAndSizes[index].version; @@ -289,7 +297,8 @@ TEST(ProcessTypes, DyldImagesSelf) { << "index " << index; } #if defined(ARCH_CPU_64_BITS) - EXPECT_EQ(proctype_image_infos.reserved_4, self_image_infos->reserved[4]); + EXPECT_EQ(proctype_image_infos.reserved_4_64, + self_image_infos->reserved[4]); EXPECT_EQ(proctype_image_infos.reserved_5, self_image_infos->reserved[5]); EXPECT_EQ(proctype_image_infos.reserved_6, self_image_infos->reserved[6]); EXPECT_EQ(proctype_image_infos.reserved_7, self_image_infos->reserved[7]); @@ -298,6 +307,15 @@ TEST(ProcessTypes, DyldImagesSelf) { } #endif +#if MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_X_VERSION_10_13 + if (proctype_image_infos.version >= 16) { + EXPECT_EQ(proctype_image_infos.compact_dyld_image_info_addr, + self_image_infos->compact_dyld_image_info_addr); + EXPECT_EQ(proctype_image_infos.compact_dyld_image_info_size, + self_image_infos->compact_dyld_image_info_size); + } +#endif + if (proctype_image_infos.version >= 1) { std::vector proctype_image_info_vector( proctype_image_infos.infoArrayCount);