From 983264848d5372d8e64d62eb67f672c71e4b6470 Mon Sep 17 00:00:00 2001 From: waylonis Date: Wed, 7 Feb 2007 18:57:09 +0000 Subject: Fix bug with mach-o walker not properly walking universal binary (Issue #125) Fix exception handler so that it will properly forward exceptions (Issue #126) r=mmentovai git-svn-id: http://google-breakpad.googlecode.com/svn/trunk@119 4c0a9323-5329-0410-9bdc-e9ce6186880e --- src/client/mac/handler/exception_handler.cc | 72 ++++++++++++----------------- src/common/mac/macho_id.cc | 17 ++++++- src/common/mac/macho_walker.cc | 39 ++++++++++++++-- src/common/mac/macho_walker.h | 12 +++++ 4 files changed, 92 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/client/mac/handler/exception_handler.cc b/src/client/mac/handler/exception_handler.cc index 16a0c322..b2c804ed 100644 --- a/src/client/mac/handler/exception_handler.cc +++ b/src/client/mac/handler/exception_handler.cc @@ -66,11 +66,10 @@ struct ExceptionReplyMessage { kern_return_t return_code; }; -// Each thread needs to keep track of its ExceptionParameters. Since the time -// that they are needed is when calling through exc_server(), we have no way -// of retrieving the values from the class. Therefore, we'll create a map -// that will allows storage per thread. -static map *s_exception_parameter_map = NULL; +// Only catch these three exceptions. The other ones are nebulously defined +// and may result in treating a non-fatal exception as fatal. +exception_mask_t s_exception_mask = EXC_MASK_BAD_ACCESS | +EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC; extern "C" { @@ -224,33 +223,38 @@ kern_return_t ForwardException(mach_port_t task, mach_port_t failed_thread, exception_type_t exception, exception_data_t code, mach_msg_type_number_t code_count) { - map::iterator previous_location = - s_exception_parameter_map->find(pthread_self()); - - // If we don't have the previous data, we need to just exit - if (previous_location == (*s_exception_parameter_map).end()) - exit(exception); - - ExceptionParameters *previous = (*previous_location).second; - + // At this time, we should have called Uninstall() on the exception handler + // so that the current exception ports are the ones that we should be + // forwarding to. + ExceptionParameters current; + + current.count = EXC_TYPES_COUNT; + mach_port_t current_task = mach_task_self(); + kern_return_t result = task_get_exception_ports(current_task, + s_exception_mask, + current.masks, + ¤t.count, + current.ports, + current.behaviors, + current.flavors); + // Find the first exception handler that matches the exception unsigned int found; - for (found = 0; found < previous->count; ++found) { - if (previous->masks[found] & (1 << exception)) { + for (found = 0; found < current.count; ++found) { + if (current.masks[found] & (1 << exception)) { break; } } // Nothing to forward - if (found == previous->count) { + if (found == current.count) { fprintf(stderr, "** No previous ports for forwarding!! \n"); exit(KERN_FAILURE); } - mach_port_t target_port = previous->ports[found]; - exception_behavior_t target_behavior = previous->behaviors[found]; - thread_state_flavor_t target_flavor = previous->flavors[found]; - kern_return_t result; + mach_port_t target_port = current.ports[found]; + exception_behavior_t target_behavior = current.behaviors[found]; + thread_state_flavor_t target_flavor = current.flavors[found]; mach_msg_type_number_t thread_state_count = THREAD_STATE_MAX; thread_state_data_t thread_state; @@ -291,6 +295,7 @@ kern_return_t ForwardException(mach_port_t task, mach_port_t failed_thread, break; default: + fprintf(stderr, "** Unknown exception behavior\n"); result = KERN_FAILURE; break; } @@ -313,19 +318,6 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) { reinterpret_cast(exception_handler_class); ExceptionMessage receive; - // Save a pointer to our instance so that it will be available in the - // routines that are called from exc_server(); - if (!s_exception_parameter_map) { - try { - s_exception_parameter_map = new map; - } - catch (std::bad_alloc) { - return NULL; - } - } - - (*s_exception_parameter_map)[pthread_self()] = self->previous_; - // Wait for the exception info while (1) { receive.header.msgh_local_port = self->handler_port_; @@ -388,11 +380,6 @@ void *ExceptionHandler::WaitForMessage(void *exception_handler_class) { } bool ExceptionHandler::InstallHandler() { - // Only catch these three exceptions. The other ones are nebulously defined - // and may result in treating a non-fatal exception as fatal. - exception_mask_t exception_mask = EXC_MASK_BAD_ACCESS | - EXC_MASK_BAD_INSTRUCTION | EXC_MASK_ARITHMETIC; - try { previous_ = new ExceptionParameters(); } @@ -403,7 +390,8 @@ bool ExceptionHandler::InstallHandler() { // Save the current exception ports so that we can forward to them previous_->count = EXC_TYPES_COUNT; mach_port_t current_task = mach_task_self(); - kern_return_t result = task_get_exception_ports(current_task, exception_mask, + kern_return_t result = task_get_exception_ports(current_task, + s_exception_mask, previous_->masks, &previous_->count, previous_->ports, @@ -412,7 +400,7 @@ bool ExceptionHandler::InstallHandler() { // Setup the exception ports on this task if (result == KERN_SUCCESS) - result = task_set_exception_ports(current_task, exception_mask, + result = task_set_exception_ports(current_task, s_exception_mask, handler_port_, EXCEPTION_DEFAULT, THREAD_STATE_NONE); @@ -493,8 +481,6 @@ bool ExceptionHandler::Teardown() { } else { return false; } - if (s_exception_parameter_map) - s_exception_parameter_map->erase(handler_thread_); handler_thread_ = NULL; handler_port_ = NULL; diff --git a/src/common/mac/macho_id.cc b/src/common/mac/macho_id.cc index ca155ed4..59cd1e21 100644 --- a/src/common/mac/macho_id.cc +++ b/src/common/mac/macho_id.cc @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -246,6 +247,12 @@ bool MachoID::WalkerCB(MachoWalker *walker, load_command *cmd, off_t offset, if (swap) swap_segment_command(&seg, NXHostByteOrder()); + struct mach_header_64 header; + off_t header_offset; + + if (!walker->CurrentHeader(&header, &header_offset)) + return false; + // Process segments that have sections: // (e.g., __TEXT, __DATA, __IMPORT, __OBJC) offset += sizeof(struct segment_command); @@ -257,7 +264,7 @@ bool MachoID::WalkerCB(MachoWalker *walker, load_command *cmd, off_t offset, if (swap) swap_section(&sec, 1, NXHostByteOrder()); - macho_id->Update(walker, sec.offset, sec.size); + macho_id->Update(walker, header_offset + sec.offset, sec.size); offset += sizeof(struct section); } } else if (cmd->cmd == LC_SEGMENT_64) { @@ -269,6 +276,12 @@ bool MachoID::WalkerCB(MachoWalker *walker, load_command *cmd, off_t offset, if (swap) swap_segment_command_64(&seg64, NXHostByteOrder()); + struct mach_header_64 header; + off_t header_offset; + + if (!walker->CurrentHeader(&header, &header_offset)) + return false; + // Process segments that have sections: // (e.g., __TEXT, __DATA, __IMPORT, __OBJC) offset += sizeof(struct segment_command_64); @@ -280,7 +293,7 @@ bool MachoID::WalkerCB(MachoWalker *walker, load_command *cmd, off_t offset, if (swap) swap_section_64(&sec64, 1, NXHostByteOrder()); - macho_id->Update(walker, sec64.offset, sec64.size); + macho_id->Update(walker, header_offset + sec64.offset, sec64.size); offset += sizeof(struct section_64); } } diff --git a/src/common/mac/macho_walker.cc b/src/common/mac/macho_walker.cc index 5e9de5f4..81ba3bb5 100644 --- a/src/common/mac/macho_walker.cc +++ b/src/common/mac/macho_walker.cc @@ -38,6 +38,7 @@ #include #include #include +#include #include #include "common/mac/macho_walker.h" @@ -93,6 +94,16 @@ bool MachoWalker::ReadBytes(void *buffer, size_t size, off_t offset) { return pread(file_, buffer, size, offset) == (ssize_t)size; } +bool MachoWalker::CurrentHeader(struct mach_header_64 *header, off_t *offset) { + if (current_header_) { + memcpy(header, current_header_, sizeof(mach_header_64)); + *offset = current_header_offset_; + return true; + } + + return false; +} + bool MachoWalker::FindHeader(int cpu_type, off_t &offset) { int valid_cpu_type = ValidateCPUType(cpu_type); // Read the magic bytes that's common amongst all mach-o files @@ -165,8 +176,22 @@ bool MachoWalker::WalkHeaderAtOffset(off_t offset) { bool swap = (header.magic == MH_CIGAM); if (swap) swap_mach_header(&header, NXHostByteOrder()); - - return WalkHeaderCore(offset + sizeof(header), header.ncmds, swap); + + // Copy the data into the mach_header_64 structure. Since the 32-bit and + // 64-bit only differ in the last field (reserved), this is safe to do. + struct mach_header_64 header64; + memcpy((void *)&header64, (const void *)&header, sizeof(header)); + header64.reserved = 0; + + current_header_ = &header64; + current_header_size_ = sizeof(header); // 32-bit, not 64-bit + current_header_offset_ = offset; + offset += current_header_size_; + bool result = WalkHeaderCore(offset, header.ncmds, swap); + current_header_ = NULL; + current_header_size_ = 0; + current_header_offset_ = 0; + return result; } bool MachoWalker::WalkHeader64AtOffset(off_t offset) { @@ -178,7 +203,15 @@ bool MachoWalker::WalkHeader64AtOffset(off_t offset) { if (swap) swap_mach_header_64(&header, NXHostByteOrder()); - return WalkHeaderCore(offset + sizeof(header), header.ncmds, swap); + current_header_ = &header; + current_header_size_ = sizeof(header); + current_header_offset_ = offset; + offset += current_header_size_; + bool result = WalkHeaderCore(offset, header.ncmds, swap); + current_header_ = NULL; + current_header_size_ = 0; + current_header_offset_ = 0; + return result; } bool MachoWalker::WalkHeaderCore(off_t offset, uint32_t number_of_commands, diff --git a/src/common/mac/macho_walker.h b/src/common/mac/macho_walker.h index 0d55dd79..eb9575b0 100644 --- a/src/common/mac/macho_walker.h +++ b/src/common/mac/macho_walker.h @@ -68,6 +68,9 @@ class MachoWalker { // Read |size| bytes from the opened file at |offset| into |buffer| bool ReadBytes(void *buffer, size_t size, off_t offset); + + // Return the current header and header offset + bool CurrentHeader(struct mach_header_64 *header, off_t *offset); private: // Validate the |cpu_type| @@ -87,6 +90,15 @@ class MachoWalker { // User specified callback & context LoadCommandCallback callback_; void *callback_context_; + + // Current header, size, and offset. The mach_header_64 is used for both + // 32-bit and 64-bit headers because they only differ in their last field + // (reserved). By adding the |current_header_size_| and the + // |current_header_offset_|, you can determine the offset in the file just + // after the header. + struct mach_header_64 *current_header_; + unsigned long current_header_size_; + off_t current_header_offset_; }; } // namespace MacFileUtilities -- cgit v1.2.1