Merging upstream main from commit 6598c9c3

src/client/linux/microdump_writer/microdump_writer.cc

@@ -475,8 +475,10 @@ class MicrodumpWriter {
 
     // Copy as many bytes of |identifier| as will fit into a MDGUID
     MDGUID module_identifier = {0};
-    memcpy(&module_identifier, &identifier_bytes[0],
-           std::min(sizeof(MDGUID), identifier_bytes.size()));
+    if (!identifier_bytes.empty()) {
+      memcpy(&module_identifier, &identifier_bytes[0],
+             std::min(sizeof(MDGUID), identifier_bytes.size()));
+    }
 
     char file_name[NAME_MAX];
     char file_path[NAME_MAX];

src/common/linux/file_id.cc

@@ -128,20 +128,22 @@ static bool FindElfBuildIDNote(const void* elf_mapped_base,
 // a simple hash by XORing the first page worth of bytes into |identifier|.
 static bool HashElfTextSection(const void* elf_mapped_base,
                                wasteful_vector<uint8_t>& identifier) {
-  identifier.resize(kMDGUIDSize);
-
   void* text_section;
   size_t text_size;
   if ((!FindElfSection(elf_mapped_base, ".text", SHT_PROGBITS,
                        (const void**)&text_section, &text_size) &&
        !FindElfSection(elf_mapped_base, "text", SHT_PROGBITS,
                        (const void**)&text_section, &text_size)) ||
       text_size == 0) {
+    // Leave |identifier| untouched. A caller that sees bytes here would
+    // write them as a build id, turning "cannot identify this module" into
+    // a valid-looking all-zero one.
     return false;
   }
 
   // Only provide |kMDGUIDSize| bytes to keep identifiers produced by this
   // function backwards-compatible.
+  identifier.resize(kMDGUIDSize);
   my_memset(&identifier[0], 0, kMDGUIDSize);
   const uint8_t* ptr = reinterpret_cast<const uint8_t*>(text_section);
   const uint8_t* ptr_end = ptr + std::min(text_size, static_cast<size_t>(4096));

src/common/linux/file_id_unittest.cc

@@ -455,3 +455,19 @@ TYPED_TEST(FileIDTest, ConvertIdentifierToString) {
   ASSERT_EQ(kExpected,
             FileID::ConvertIdentifierToString(identifier));
 }
+
+// A failed identification must leave the identifier EMPTY, so FillRawModule
+// writes no CV record at all. Leaving bytes behind turns "cannot identify
+// this module" into a valid-looking all-zero build id, which reads downstream
+// as a missing symbol upload rather than a broken minidump.
+TEST(FileIDTest, FailedIdentificationLeavesNoBytes) {
+  // Not an ELF: no build-id note to find, and no .text section to hash.
+  uint8_t not_an_elf[4096] = {};
+
+  PageAllocator allocator;
+  auto_wasteful_vector<uint8_t, kDefaultBuildIdSize> identifier(&allocator);
+
+  EXPECT_FALSE(
+      FileID::ElfFileIdentifierFromMappedFile(not_an_elf, identifier));
+  EXPECT_TRUE(identifier.empty());
+}

src/common/memory_allocator.h

@@ -39,6 +39,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#include <cstddef>
 #include <memory>
 #include <vector>
 

src/common/memory_allocator_unittest.cc

@@ -32,6 +32,7 @@
 
 #include <stdint.h>
 
+#include <cstddef>
 #include <limits>
 
 #include "breakpad_googletest_includes.h"
@@ -135,14 +136,21 @@ TEST(WastefulVectorTest, Simple) {
   EXPECT_EQ(0U, allocator_.pages_allocated());
   wasteful_vector<unsigned> v(&allocator_);
 
+  size_t total_used = 0;
   for (unsigned i = 0; i < 256; ++i) {
+    const size_t prev_capacity = v.capacity();
     v.push_back(i);
+    const size_t new_capacity = v.capacity();
+    if (new_capacity > prev_capacity) {
+      total_used += new_capacity * sizeof(unsigned);
+    }
     ASSERT_EQ(i, v.back());
     ASSERT_EQ(&v.back(), &v[i]);
   }
   ASSERT_FALSE(v.empty());
   ASSERT_EQ(v.size(), 256u);
-  EXPECT_EQ(1U, allocator_.pages_allocated());
+  EXPECT_LE(allocator_.pages_allocated(),
+            total_used / 4096 + 1);
   for (unsigned i = 0; i < 256; ++i)
     ASSERT_EQ(v[i], i);
 }

src/processor/simple_serializer.h

@@ -38,6 +38,7 @@
 #define PROCESSOR_SIMPLE_SERIALIZER_H__
 
 #include <stddef.h>
+#include <string.h>
 
 #include "google_breakpad/common/breakpad_types.h"
 
@@ -54,8 +55,9 @@ template<class Type> class SimpleSerializer {
   // Write 'item' to memory location 'dest', and return to the "end" address of
   // data written, i.e., the address after the final byte written.
   static char* Write(const Type& item, char* dest) {
-    new (dest) Type(item);
-    return dest + SizeOf(item);
+    const size_t size = SizeOf(item);
+    memcpy(dest, &item, size);
+    return dest + size;
   }
 };
 

src/tools/linux/md2core/minidump-2-core.cc

@@ -1287,16 +1287,18 @@ AugmentMappings(const Options& options, CrashedProcess* crashinfo,
       }
     }
 
+    const size_t aligned_filename_size =
+        PageAllocator::AlignUp(filename.size(), 8);
     if (std::distance(iter, crashinfo->link_map.end()) == 1) {
       link_map.l_next = 0;
     } else {
       link_map.l_next =
           (struct link_map*)(start_addr + data.size() + sizeof(link_map) +
-                             PageAllocator::AlignUp(filename.size(), 8));
+                             aligned_filename_size);
     }
     data.append((char*)&link_map, sizeof(link_map));
     data.append(filename);
-    data.append(8 - (filename.size() & 7), 0);
+    data.append(aligned_filename_size - filename.size(), 0);
   }
   AddDataToMapping(crashinfo, data, start_addr);
 

src/tools/mac/upload_system_symbols/archive/extract.go

@@ -312,6 +312,10 @@ func (e *installAssistantExtractor) hasCryptexes(plistPath string) (bool, error)
 	}
 }
 
+// imagePatchManifestPath is where an installer's asset archive holds the manifest
+// describing its image patches, keyed by each patch's file name.
+const imagePatchManifestPath = "AssetData/image_patches.plist"
+
 // extractCachesFromZips extracts zips that contain dyld shared caches, and extracts the dyld shared caches from them.
 // The specifics depend on whether this installer uses cryptexes or payload files.
 func (e *installAssistantExtractor) extractCachesFromZips(zips []string, destination string, hasCryptexes bool) error {
@@ -324,10 +328,17 @@ func (e *installAssistantExtractor) extractCachesFromZips(zips []string, destina
 			return fmt.Errorf("couldn't extract caches from %v: %v", containerPath, err)
 		}
 	} else {
+		// The manifest goes somewhere other than `containerPath`, which
+		// extractCachesFromCryptexes reads expecting nothing but image patches.
+		manifestDir := path.Join(e.scratchDir, "image_patch_manifest")
+		if err := e.unarchiveZipsMatching(zips, manifestDir, imagePatchManifestPath); err != nil {
+			return err
+		}
 		if err := e.unarchiveZipsMatching(zips, containerPath, "AssetData/payloadv2/image_patches/cryptex-system-*"); err != nil {
 			return err
 		}
-		if err := e.extractCachesFromCryptexes(containerPath, destination); err != nil {
+		manifest := path.Join(manifestDir, path.Base(imagePatchManifestPath))
+		if err := e.extractCachesFromCryptexes(containerPath, manifest, destination); err != nil {
 			return fmt.Errorf("couldn't extract caches from %v: %v", containerPath, err)
 		}
 	}
@@ -428,8 +439,9 @@ func (e *installAssistantExtractor) copySharedCaches(from, to string) error {
 }
 
 // extractCachesFromCryptexes extracts disk images from any cryptexes at `cryptexesPath`, mounts them,
-// and extracts any dyld shared caches to `destination`.
-func (e *installAssistantExtractor) extractCachesFromCryptexes(cryptexesPath string, destination string) error {
+// and extracts any dyld shared caches to `destination`. `manifest` is the image patch manifest
+// describing those cryptexes.
+func (e *installAssistantExtractor) extractCachesFromCryptexes(cryptexesPath string, manifest string, destination string) error {
 	scratchDir, err := os.MkdirTemp(e.scratchDir, "cryptex_dmg")
 	if err != nil {
 		return err
@@ -446,12 +458,32 @@ func (e *installAssistantExtractor) extractCachesFromCryptexes(cryptexesPath str
 		e.vlog("Mounting %s at %s\n", cryptex, dmgPath)
 		e.mountDMG(dmgPath, cryptexMountpoint)
 		if err := e.copySharedCaches(cryptexMountpoint, destination); err != nil {
+			// A delta image patches another image rather than containing one, so it
+			// can't necessarily be turned into a disk image to take caches from. For now,
+			// ignore the failure and proceed to the next image cache extraction.
+			if e.isDeltaImage(manifest, f.Name()) {
+				e.vlog("Ignoring %v: %v. It patches another image rather than replacing it.\n",
+					cryptex, err)
+				continue
+			}
 			return err
 		}
 	}
 	return nil
 }
 
+// isDeltaImage returns true if the image contains "ImagePatchBaselineTag" in the plist file,
+// indicating that the image is a delta one, built on top of a baseline image.
+func (e *installAssistantExtractor) isDeltaImage(manifestPath string, imageName string) bool {
+	print_cmd := fmt.Sprintf("print :%s:ImagePatchBaselineTag", imageName)
+	baseline, err := exec.Command("/usr/libexec/PlistBuddy", "-c", print_cmd, manifestPath).Output()
+	if err != nil {
+		return false
+	}
+	e.vlog("%v patches %v\n", imageName, strings.TrimSpace(string(baseline)))
+	return true
+}
+
 // extractCryptexDMG extracts the cryptex at `cryptexPath` to `dmgPath` using libParallelCompression.
 func (e *installAssistantExtractor) extractCryptexDMG(cryptexPath, dmgPath string) error {
 	inp := C.CString("")