Commit `9434bab313`

mlugg <mlugg@mlugg.co.uk>

2025-09-18 13:54:33

std: work around crash parsing LLVM PDB

This crash exists on master, and seems to have existed since 2019; I think it's just very rare and depends on the exact binary generated. In theory, a stream block should always be a "data" block rather than a FPM block; the FPMs use blocks `1, 4097, 8193, ...` and `2, 4097, 8194, ...` respectively. However, I have observed LLVM emitting an otherwise valid PDB which maps FPM blocks into streams. This is not a bug in `std.debug.Pdb`, because `llvm-pdbutil` agrees with our stream indices. I think this is arguably an LLVM bug; however, we don't really lose anything from just weakening this check. To be fair, MSF doesn't have an explicit specification, and LLVM's documentation (which is the closest thing we have) does not explicitly state that FPM blocks cannot be mapped into streams, so perhaps this is actually valid. In the rare case that LLVM emits this, previously, stack traces would have been completely useless; now, stack traces will work okay.

master

1 parent 23d6381

Changed files (1)

lib

std

debug

Pdb.zig

@@ -413,8 +413,7 @@ const Msf = struct {
             return error.InvalidDebugInfo;
         if (superblock.free_block_map_block != 1 and superblock.free_block_map_block != 2)
             return error.InvalidDebugInfo;
-        const file_len = try file_reader.getSize();
-        if (superblock.num_blocks * superblock.block_size != file_len)
+        if (superblock.num_blocks * superblock.block_size != try file_reader.getSize())
             return error.InvalidDebugInfo;
         switch (superblock.block_size) {
             // llvm only supports 4096 but we can handle any of these values
@@ -428,6 +427,7 @@ const Msf = struct {
 
         try file_reader.seekTo(superblock.block_size * superblock.block_map_addr);
         const dir_blocks = try gpa.alloc(u32, dir_block_count);
+        errdefer gpa.free(dir_blocks);
         for (dir_blocks) |*b| {
             b.* = try file_reader.interface.takeInt(u32, .little);
         }
@@ -451,25 +451,25 @@ const Msf = struct {
         const streams = try gpa.alloc(MsfStream, stream_count);
         errdefer gpa.free(streams);
 
-        for (streams, 0..) |*stream, i| {
-            const size = stream_sizes[i];
+        for (streams, stream_sizes) |*stream, size| {
             if (size == 0) {
                 stream.* = .empty;
-            } else {
-                const blocks = try gpa.alloc(u32, size);
-                errdefer gpa.free(blocks);
-                for (blocks) |*block| {
-                    const block_id = try directory.interface.takeInt(u32, .little);
-                    const n = (block_id % superblock.block_size);
-                    // 0 is for pdb.SuperBlock, 1 and 2 for FPMs.
-                    if (block_id == 0 or n == 1 or n == 2 or block_id * superblock.block_size > file_len)
-                        return error.InvalidBlockIndex;
-                    block.* = block_id;
-                }
-                const buffer = try gpa.alloc(u8, 64);
-                errdefer gpa.free(buffer);
-                stream.* = .init(superblock.block_size, file_reader, blocks, buffer);
+                continue;
+            }
+            const blocks = try gpa.alloc(u32, size);
+            errdefer gpa.free(blocks);
+            for (blocks) |*block| {
+                const block_id = try directory.interface.takeInt(u32, .little);
+                // Index 0 is reserved for the superblock.
+                // In theory, every page which is `n * block_size + 1` or `n * block_size + 2`
+                // is also reserved, for one of the FPMs. However, LLVM has been observed to map
+                // these into actual streams, so allow it for compatibility.
+                if (block_id == 0 or block_id >= superblock.num_blocks) return error.InvalidBlockIndex;
+                block.* = block_id;
             }
+            const buffer = try gpa.alloc(u8, 64);
+            errdefer gpa.free(buffer);
+            stream.* = .init(superblock.block_size, file_reader, blocks, buffer);
         }
 
         const end = directory.logicalPos();

Commit 9434bab313

Commit `9434bab313`