From 402bb216fb2be24d7a81cb7c003ef59ad3c6394d Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Fri, 7 Aug 2015 16:31:27 -0400 Subject: [PATCH] Provide a properly-typed ExpectedSizeForVersion() for types that need it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than declaring ExpectedSizeForVersion() for all process_types types and providing a default NOTREACHED() implementation, this only declares it for process_types that request it by stating PROCESS_TYPE_STRUCT_VERSIONED() in their proctype definition. This also allows the argument to have the correct type, matching the type of the struct’s version field. TEST=crashpad_snapshot_test R=rsesek@chromium.org Review URL: https://codereview.chromium.org/1274663005 . --- snapshot/mac/process_types.cc | 161 +++++++++++------- snapshot/mac/process_types.h | 28 +-- .../crashreporterclient.proctype | 1 + snapshot/mac/process_types/custom.cc | 15 +- .../mac/process_types/dyld_images.proctype | 1 + 5 files changed, 128 insertions(+), 78 deletions(-) diff --git a/snapshot/mac/process_types.cc b/snapshot/mac/process_types.cc index 196a01dd..0fc14f0e 100644 --- a/snapshot/mac/process_types.cc +++ b/snapshot/mac/process_types.cc @@ -17,7 +17,6 @@ #include #include -#include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "snapshot/mac/process_types/internal.h" #include "util/mach/task_memory.h" @@ -83,68 +82,58 @@ inline void Assign(uuid_t* destination, const uuid_t& source) { // operates on each member in the struct. #define PROCESS_TYPE_STRUCT_IMPLEMENT 1 -#define PROCESS_TYPE_STRUCT_BEGIN(struct_name) \ - namespace crashpad { \ - namespace process_types { \ - \ - /* static */ \ - size_t struct_name::ExpectedSize(ProcessReader* process_reader) { \ - if (!process_reader->Is64Bit()) { \ - return internal::struct_name::Size(); \ - } else { \ - return internal::struct_name::Size(); \ - } \ - } \ - \ - /* static */ \ - size_t struct_name::ExpectedSizeForVersion(ProcessReader* process_reader, \ - uint64_t version) { \ - if (!process_reader->Is64Bit()) { \ - return internal::struct_name< \ - internal::Traits32>::ExpectedSizeForVersion(version); \ - } else { \ - return internal::struct_name< \ - internal::Traits64>::ExpectedSizeForVersion(version); \ - } \ - } \ - \ - /* static */ \ - bool struct_name::ReadInto(ProcessReader* process_reader, \ - mach_vm_address_t address, \ - struct_name* generic) { \ - if (!process_reader->Is64Bit()) { \ - return ReadIntoInternal >( \ - process_reader, address, generic); \ - } else { \ - return ReadIntoInternal >( \ - process_reader, address, generic); \ - } \ - } \ - \ - /* static */ \ - template \ - bool struct_name::ReadIntoInternal(ProcessReader* process_reader, \ - mach_vm_address_t address, \ - struct_name* generic) { \ - T specific; \ - if (!specific.Read(process_reader, address)) { \ - return false; \ - } \ - specific.GenericizeInto(generic, &generic->size_); \ - return true; \ - } \ - \ - namespace internal { \ - \ - template \ - void struct_name::GenericizeInto( \ - process_types::struct_name* generic, \ - size_t* specific_size) { \ +#define PROCESS_TYPE_STRUCT_BEGIN(struct_name) \ + namespace crashpad { \ + namespace process_types { \ + \ + /* static */ \ + size_t struct_name::ExpectedSize(ProcessReader* process_reader) { \ + if (!process_reader->Is64Bit()) { \ + return internal::struct_name::Size(); \ + } else { \ + return internal::struct_name::Size(); \ + } \ + } \ + \ + /* static */ \ + bool struct_name::ReadInto(ProcessReader* process_reader, \ + mach_vm_address_t address, \ + struct_name* generic) { \ + if (!process_reader->Is64Bit()) { \ + return ReadIntoInternal >( \ + process_reader, address, generic); \ + } else { \ + return ReadIntoInternal >( \ + process_reader, address, generic); \ + } \ + } \ + \ + /* static */ \ + template \ + bool struct_name::ReadIntoInternal(ProcessReader* process_reader, \ + mach_vm_address_t address, \ + struct_name* generic) { \ + T specific; \ + if (!specific.Read(process_reader, address)) { \ + return false; \ + } \ + specific.GenericizeInto(generic, &generic->size_); \ + return true; \ + } \ + \ + namespace internal { \ + \ + template \ + void struct_name::GenericizeInto( \ + process_types::struct_name* generic, \ + size_t* specific_size) { \ *specific_size = Size(); #define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) \ Assign(&generic->member_name, member_name); +#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field) + #define PROCESS_TYPE_STRUCT_END(struct_name) \ } \ } /* namespace internal */ \ @@ -155,6 +144,7 @@ inline void Assign(uuid_t* destination, const uuid_t& source) { #undef PROCESS_TYPE_STRUCT_BEGIN #undef PROCESS_TYPE_STRUCT_MEMBER +#undef PROCESS_TYPE_STRUCT_VERSIONED #undef PROCESS_TYPE_STRUCT_END #undef PROCESS_TYPE_STRUCT_IMPLEMENT @@ -185,12 +175,15 @@ inline void Assign(uuid_t* destination, const uuid_t& source) { #define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) +#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field) + #define PROCESS_TYPE_STRUCT_END(struct_name) #include "snapshot/mac/process_types/all.proctype" #undef PROCESS_TYPE_STRUCT_BEGIN #undef PROCESS_TYPE_STRUCT_MEMBER +#undef PROCESS_TYPE_STRUCT_VERSIONED #undef PROCESS_TYPE_STRUCT_END #undef PROCESS_TYPE_STRUCT_IMPLEMENT_INTERNAL_READ_INTO @@ -216,12 +209,6 @@ inline void Assign(uuid_t* destination, const uuid_t& source) { address, sizeof(struct_name[count]), specific); \ } \ \ - /* static */ \ - template \ - size_t struct_name::ExpectedSizeForVersion(uint64_t version) { \ - NOTREACHED(); \ - return 0; \ - } \ } /* namespace internal */ \ \ /* static */ \ @@ -261,11 +248,57 @@ inline void Assign(uuid_t* destination, const uuid_t& source) { #define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) +#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field) + #define PROCESS_TYPE_STRUCT_END(struct_name) #include "snapshot/mac/process_types/all.proctype" #undef PROCESS_TYPE_STRUCT_BEGIN #undef PROCESS_TYPE_STRUCT_MEMBER +#undef PROCESS_TYPE_STRUCT_VERSIONED #undef PROCESS_TYPE_STRUCT_END #undef PROCESS_TYPE_STRUCT_IMPLEMENT_ARRAY + +// Implement the generic crashpad::process_types::struct_name +// ExpectedSizeForVersion(), which delegates to the templatized +// ExpectedSizeForVersion(), which returns the expected size of a versioned +// structure given a version parameter. This is only implemented for structures +// that use PROCESS_TYPE_STRUCT_VERSIONED(), and implementations of the internal +// templatized functions must be provided in +// snapshot/mac/process_types/custom.cc. +#define PROCESS_TYPE_STRUCT_IMPLEMENT_VERSIONED 1 + +#define PROCESS_TYPE_STRUCT_BEGIN(struct_name) + +#define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) + +#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field) \ + namespace crashpad { \ + namespace process_types { \ + \ + /* static */ \ + size_t struct_name::ExpectedSizeForVersion( \ + ProcessReader* process_reader, \ + decltype(struct_name::version_field) version) { \ + if (!process_reader->Is64Bit()) { \ + return internal::struct_name< \ + internal::Traits32>::ExpectedSizeForVersion(version); \ + } else { \ + return internal::struct_name< \ + internal::Traits64>::ExpectedSizeForVersion(version); \ + } \ + } \ + \ + } /* namespace process_types */ \ + } /* namespace crashpad */ + +#define PROCESS_TYPE_STRUCT_END(struct_name) + +#include "snapshot/mac/process_types/all.proctype" + +#undef PROCESS_TYPE_STRUCT_BEGIN +#undef PROCESS_TYPE_STRUCT_MEMBER +#undef PROCESS_TYPE_STRUCT_VERSIONED +#undef PROCESS_TYPE_STRUCT_END +#undef PROCESS_TYPE_STRUCT_IMPLEMENT_VERSIONED diff --git a/snapshot/mac/process_types.h b/snapshot/mac/process_types.h index 62b46636..a1039dd0 100644 --- a/snapshot/mac/process_types.h +++ b/snapshot/mac/process_types.h @@ -86,16 +86,18 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) * on the process’ bitness. This can be used prior to reading any data \ * from a process. */ \ static size_t ExpectedSize(ProcessReader* process_reader); \ - \ + +#define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) \ + member_type member_name __VA_ARGS__; + +#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field) \ /* Similar to ExpectedSize(), but computes the expected size of a \ * structure based on the process’ bitness and a custom value, such as a \ * structure version number. This can be used prior to reading any data \ * from a process. */ \ - static size_t ExpectedSizeForVersion(ProcessReader* process_reader, \ - uint64_t version); - -#define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) \ - member_type member_name __VA_ARGS__; + static size_t ExpectedSizeForVersion( \ + ProcessReader* process_reader, \ + decltype(struct_name::version_field) version); #define PROCESS_TYPE_STRUCT_END(struct_name) \ private: \ @@ -122,6 +124,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) #undef PROCESS_TYPE_STRUCT_BEGIN #undef PROCESS_TYPE_STRUCT_MEMBER +#undef PROCESS_TYPE_STRUCT_VERSIONED #undef PROCESS_TYPE_STRUCT_END #undef PROCESS_TYPE_STRUCT_DECLARE @@ -131,7 +134,6 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) // shared with the generic declaration above because both the generic and // templatized specific structs need all of the struct members declared. // -// // GenericizeInto() translates a struct from the representation used in the // remote process into the generic form. #define PROCESS_TYPE_STRUCT_DECLARE_INTERNAL 1 @@ -150,8 +152,8 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) using UIntPtr = typename Traits::UIntPtr; \ using Reserved64Only = typename Traits::Reserved64Only; \ \ - /* Read(), ReadArrayInto(), Size(), and ExpectedSizeForVersion() are as in \ - * the generic user-visible struct above. */ \ + /* Read(), ReadArrayInto(), and Size() are as in the generic user-visible \ + * struct above. */ \ bool Read(ProcessReader* process_reader, mach_vm_address_t address) { \ return ReadInto(process_reader, address, this); \ } \ @@ -160,7 +162,6 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) size_t count, \ struct_name* specific); \ static size_t Size() { return sizeof(struct_name); } \ - static size_t ExpectedSizeForVersion(uint64_t version); \ \ /* Translates a struct from the representation used in the remote process \ * into the generic form. */ \ @@ -170,6 +171,12 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) #define PROCESS_TYPE_STRUCT_MEMBER(member_type, member_name, ...) \ member_type member_name __VA_ARGS__; +#define PROCESS_TYPE_STRUCT_VERSIONED(struct_name, version_field) \ + /* ExpectedSizeForVersion() is as in the generic user-visible struct \ + * above. */ \ + static size_t ExpectedSizeForVersion( \ + decltype(struct_name::version_field) version); + #define PROCESS_TYPE_STRUCT_END(struct_name) \ private: \ /* ReadInto() is as in the generic user-visible struct above. */ \ @@ -185,6 +192,7 @@ DECLARE_PROCESS_TYPE_TRAITS_CLASS(Generic, 64) #undef PROCESS_TYPE_STRUCT_BEGIN #undef PROCESS_TYPE_STRUCT_MEMBER +#undef PROCESS_TYPE_STRUCT_VERSIONED #undef PROCESS_TYPE_STRUCT_END #undef PROCESS_TYPE_STRUCT_DECLARE_INTERNAL diff --git a/snapshot/mac/process_types/crashreporterclient.proctype b/snapshot/mac/process_types/crashreporterclient.proctype index 7afaddea..b40aa358 100644 --- a/snapshot/mac/process_types/crashreporterclient.proctype +++ b/snapshot/mac/process_types/crashreporterclient.proctype @@ -41,6 +41,7 @@ PROCESS_TYPE_STRUCT_BEGIN(crashreporter_annotations_t) PROCESS_TYPE_STRUCT_MEMBER(uint64_t, version) // unsigned long + PROCESS_TYPE_STRUCT_VERSIONED(crashreporter_annotations_t, version) // Version 4 (Mac OS X 10.7) PROCESS_TYPE_STRUCT_MEMBER(uint64_t, message) // char* diff --git a/snapshot/mac/process_types/custom.cc b/snapshot/mac/process_types/custom.cc index ba055c0e..6d6fd0db 100644 --- a/snapshot/mac/process_types/custom.cc +++ b/snapshot/mac/process_types/custom.cc @@ -24,6 +24,8 @@ namespace crashpad { namespace process_types { namespace internal { +namespace { + template bool ReadIntoVersioned(ProcessReader* process_reader, mach_vm_address_t address, @@ -51,9 +53,12 @@ bool ReadIntoVersioned(ProcessReader* process_reader, return true; } +} // namespace + // static template -size_t dyld_all_image_infos::ExpectedSizeForVersion(uint64_t version) { +size_t dyld_all_image_infos::ExpectedSizeForVersion( + decltype(dyld_all_image_infos::version) version) { if (version >= 14) { return sizeof(dyld_all_image_infos); } @@ -108,7 +113,7 @@ bool dyld_all_image_infos::ReadInto( // static template size_t crashreporter_annotations_t::ExpectedSizeForVersion( - uint64_t version) { + decltype(crashreporter_annotations_t::version) version) { if (version >= 5) { return sizeof(crashreporter_annotations_t); } @@ -127,16 +132,18 @@ bool crashreporter_annotations_t::ReadInto( return ReadIntoVersioned(process_reader, address, specific); } +// Explicit template instantiation of the above. #define PROCESS_TYPE_FLAVOR_TRAITS(lp_bits) \ template size_t \ - dyld_all_image_infos::ExpectedSizeForVersion(uint64_t); \ + dyld_all_image_infos::ExpectedSizeForVersion( \ + decltype(dyld_all_image_infos::version)); \ template bool dyld_all_image_infos::ReadInto( \ ProcessReader*, \ mach_vm_address_t, \ dyld_all_image_infos*); \ template size_t \ crashreporter_annotations_t::ExpectedSizeForVersion( \ - uint64_t); \ + decltype(crashreporter_annotations_t::version)); \ template bool crashreporter_annotations_t::ReadInto( \ ProcessReader*, \ mach_vm_address_t, \ diff --git a/snapshot/mac/process_types/dyld_images.proctype b/snapshot/mac/process_types/dyld_images.proctype index 3e45c992..99be27c5 100644 --- a/snapshot/mac/process_types/dyld_images.proctype +++ b/snapshot/mac/process_types/dyld_images.proctype @@ -45,6 +45,7 @@ PROCESS_TYPE_STRUCT_END(dyld_uuid_info) PROCESS_TYPE_STRUCT_BEGIN(dyld_all_image_infos) PROCESS_TYPE_STRUCT_MEMBER(uint32_t, version) + PROCESS_TYPE_STRUCT_VERSIONED(dyld_all_image_infos, version) // Version 1 (Mac OS X 10.4) PROCESS_TYPE_STRUCT_MEMBER(uint32_t, infoArrayCount)