From 8e82f6fde05499eb8385d25ea5f4c3c5cab77e24 Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Tue, 7 Mar 2017 17:12:22 -0500 Subject: [PATCH] mac: Update test and comments with feedback from Apple bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apple has responded to their bug 29079442 with a resolution stating that these are not corpse ports but task ports that have changed after execve(), as part of the large task port and execve() strategy rewrite from 10.12.1. The comments being replaced were written before we had 10.12.1 source code. Now that we can see what’s going on, revise the comments, and re-enable the task port check for the non-execve() test variants. https://openradar.appspot.com/29079442 https://googleprojectzero.blogspot.com/2016/10/taskt-considered-harmful.html Bug: crashpad:137 Test: crashpad_snapshot_test MachOImageAnnotationsReader.CrashDyld Change-Id: I463637816085f4165b92b85a5b98bfeddcdf4094 Reviewed-on: https://chromium-review.googlesource.com/451120 Reviewed-by: Robert Sesek Commit-Queue: Mark Mentovai --- .../mac/mach_o_image_annotations_reader_test.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/snapshot/mac/mach_o_image_annotations_reader_test.cc b/snapshot/mac/mach_o_image_annotations_reader_test.cc index f4f53494..7a94eb2c 100644 --- a/snapshot/mac/mach_o_image_annotations_reader_test.cc +++ b/snapshot/mac/mach_o_image_annotations_reader_test.cc @@ -117,14 +117,14 @@ class TestMachOImageAnnotationsReader final bool* destroy_complex_request) override { *destroy_complex_request = true; - // In 10.12, dyld fatal errors as tested by test_type_ = kCrashDyld are via - // abort_with_payload(). In 10.12.1, the task port delivered in an exception - // message for this termination type is a corpse, even when the exception is - // EXC_CRASH and not EXC_CORPSE_NOTIFY. The corpse task port (here, |task|) - // is distinct from the process’ original task port (ChildTask()). This is - // filed as https://openradar.appspot.com/29079442. - // - // Instead of comparing task ports, compare PIDs. + if (test_type_ != kCrashDyld) { + // In 10.12.1 and later, the task port will not match ChildTask() in the + // kCrashDyld case, because kCrashDyld uses execl(), which results in a + // new task port being assigned. + EXPECT_EQ(ChildTask(), task); + } + + // The process ID should always compare favorably. pid_t task_pid; kern_return_t kr = pid_for_task(task, &task_pid); EXPECT_EQ(KERN_SUCCESS, kr) << MachErrorMessage(kr, "pid_for_task");