commit f7428bc397fd1e5bd6162a592ebc988d6d585eb3
author Chingjun Lau <chingjun@google.com> Thu May 20 16:02:31 2021 -0700
committer Joshua Peraza <jperaza@chromium.org> Fri May 21 18:56:41 2021 +0000
tree 1cad1b4cdc4fe50da1690b9018423fedf186d031
parent b7ce678aec90c0fd5dcd445acb0c9f3e3e42ef36

Fix macos_dump_syms when __eh_frame is included in the dSYM file.

macho_reader assumes that every section in a segment exists, or none of the
sections exist in the file (for dSYM files).

https://reviews.llvm.org/D94460?id=315965 added __eh_frame section to the
__TEXT segments in dSYM files. All the other sections are removed, but still
have non-zero size in the header. macho_reader only looks at `fileoff` and
`filesize` fields to determine the size of the segment, but it looks at `addr`
and `size` to determine the size of the section, therefore it determines that
the sections would not fit in the segment and refused to parse the file.

In this case the removed sections all have offset == 0. Ignore such cases.

Change-Id: Ife771f7b302c1bc81c673b1103492c41321b5e3e
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/2911204
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

src/common/mac/macho_reader.cc

diff --git a/src/common/mac/macho_reader.cc b/src/common/mac/macho_reader.cc
index b42506c..e89e090 100644
--- a/src/common/mac/macho_reader.cc
+++ b/src/common/mac/macho_reader.cc
@@ -518,12 +518,21 @@
       if (offset < size_t(segment.contents.start - buffer_.start) ||
           offset > size_t(segment.contents.end - buffer_.start) ||
           size > size_t(segment.contents.end - buffer_.start - offset)) {
-        reporter_->MisplacedSectionData(section.section_name,
-                                        section.segment_name);
-        return false;
+        if (offset > 0) {
+          reporter_->MisplacedSectionData(section.section_name,
+                                          section.segment_name);
+          return false;
+        } else {
+          // Mach-O files in .dSYM bundles have the contents of the loaded
+          // segments partially removed. The removed sections will have zero as
+          // their offset. MisplacedSectionData should not be called in this
+          // case.
+          section.contents.start = section.contents.end = NULL;
+        }
+      } else {
+        section.contents.start = buffer_.start + offset;
+        section.contents.end = section.contents.start + size;
       }
-      section.contents.start = buffer_.start + offset;
-      section.contents.end = section.contents.start + size;
     }
     if (!handler->HandleSection(section))
       return false;

src/common/mac/macho_reader_unittest.cc

diff --git a/src/common/mac/macho_reader_unittest.cc b/src/common/mac/macho_reader_unittest.cc
index dccda4e..fb2c479 100644
--- a/src/common/mac/macho_reader_unittest.cc
+++ b/src/common/mac/macho_reader_unittest.cc
@@ -1529,6 +1529,51 @@
   // to set all their labels by hand to get the (impossible)
   // configurations we want.
 
+  // A section with 0 as is start address.
+  LoadedSection empty;
+  empty.Append(10, '4');
+  empty.start() = 0;
+  empty.address() = segment.address() + 1;
+  empty.final_size() = empty.Size();
+
+  SegmentLoadCommand command;
+  command.Header("segment", segment, 0x173baa29, 0x8407275d, 0xed8f7057)
+      .AppendSectionEntry("empty", "segment", 0, 0x8b53ae5c, empty);
+
+  LoadCommands commands;
+  commands.Place(&command);
+
+  MachOFile file;
+  file.Header(&commands).Place(&segment);
+
+  ReadFile(&file, true, CPU_TYPE_ANY, 0);
+
+  Segment actual_segment;
+  EXPECT_TRUE(reader.FindSegment("segment", &actual_segment));
+
+  EXPECT_CALL(reporter, MisplacedSectionData("empty", "segment")).Times(0);
+
+  EXPECT_CALL(section_handler,
+              HandleSection(MatchSection(true, "empty", "segment",
+                                         empty.address().Value())))
+      .WillOnce(Return(true));
+
+  EXPECT_TRUE(reader.WalkSegmentSections(actual_segment, &section_handler));
+}
+
+TEST_F(LoadCommand, MisplacedSectionButSectionIsEmpty) {
+  WithConfiguration config(kLittleEndian, 64);
+
+  // The segment.
+  LoadedSection segment;
+  segment.address() = 0x696d83cc;
+  segment.Append(10, '0');
+
+  // The contents of the following sections don't matter, because
+  // we're not really going to Place them in segment; we're just going
+  // to set all their labels by hand to get the (impossible)
+  // configurations we want.
+
   // A section that extends beyond the end of its section.
   LoadedSection too_big;
   too_big.Append(10, '3');