Handle BadImageFormatException in OpenAssociatedSymbolFile

[stephentoub]

Catch BadImageFormatException from PEReader.ReadCodeViewDebugDirectoryData and skip malformed CodeView entries instead of crashing. This is consistent with how other PDB loading failures are handled in the same method.

Fixes https://github.com/dotnet/dotnet/issues/4733

[stephentoub]

🤖 Copilot Code Review — PR #124228

Holistic Assessment

Motivation: The crash is real — issue #124181 shows crossgen2 fatally crashing with BadImageFormatException: Unexpected CodeView data signature value during SDK builds on macOS. The OpenAssociatedSymbolFile method is a best-effort PDB lookup that already returns null when no PDB is found, so crashing the entire compilation over a bad debug entry is disproportionate. That said, the intermittent, macOS-arm64-only nature of the corruption suggests a build infrastructure issue that this fix will mask.

Approach: Catching BadImageFormatException and continuing is consistent with the method's existing best-effort semantics and with the precedent set by SafeReadDebugDirectory(). The .NET BCL's own PEReader.TryOpenCodeViewPortablePdb also catches BadImageFormatException from ReadCodeViewDebugDirectoryData. The fix cannot produce incorrect compilation output since PDB data is purely for debug symbols.

Summary: ⚠️ Needs Human Review. The code itself is correct for its stated purpose, but (1) silently swallowing the exception may permanently mask a build infrastructure issue that deserves investigation, and (2) there is a third unprotected call site in dotnet-pgo/Program.cs:1306 that is also vulnerable to the same crash. A human reviewer should decide whether diagnostic visibility and the missed call site matter.

---

Detailed Findings

✅ Code Correctness — Exception handling is appropriate

The continue statement correctly skips to the next debug directory entry. If no valid CodeView entry is found, pdbFileName remains null and the method returns null — identical to the existing "no PDB found" behavior. BadImageFormatException is the documented exception from ReadCodeViewDebugDirectoryData for invalid signatures. The change is applied consistently to both CompilerTypeSystemContext.cs and TraceTypeSystemContext.cs.

⚠️ Silent Exception Swallowing — May mask build infrastructure issues

The issue comments from ViktorHofer confirm this only happens on "official builds which use macos-arm64" — intermittent corruption exclusive to one platform is typically a sign of a build infrastructure problem (file races, incomplete copies, NFS issues) rather than genuinely malformed input. With this fix, builds will silently produce SDKs with missing debug symbols for corrupted assemblies, and no one will notice.

For comparison, the BCL's own PEReader.TryOpenCodeViewPortablePdb catches the same exception but preserves it in errorToReport for potential later surfacing. Consider at minimum adding a comment explaining the intentional suppression. If a logging mechanism is available in these contexts, a low-noise diagnostic would be even better. Whether this is a merge blocker is a human judgment call.

⚠️ Missed Call Site — dotnet-pgo/Program.cs:1306

There is a third call to ReadCodeViewDebugDirectoryData in src/coreclr/tools/dotnet-pgo/Program.cs:1306 that follows the exact same pattern — SafeReadDebugDirectory() loop, CodeView type check, then unprotected ReadCodeViewDebugDirectoryData — but is NOT covered by this fix:

foreach (DebugDirectoryEntry debugEntry in ecmaModule.PEReader.SafeReadDebugDirectory())
{
    if (debugEntry.Type == DebugDirectoryEntryType.CodeView)
    {
        var codeViewData = ecmaModule.PEReader.ReadCodeViewDebugDirectoryData(debugEntry);

The context there is module validation (comparing PDB GUIDs to trace data), not PDB loading, so the consequence differs — but the vulnerability to the same BadImageFormatException crash is identical. Whether this needs the same fix or a different approach (e.g., a centralized TryReadCodeViewDebugDirectoryData helper) is worth considering.

💡 Pre-existing Code Duplication

The OpenAssociatedSymbolFile methods in CompilerTypeSystemContext.cs and TraceTypeSystemContext.cs are near-identical copies (the only difference is the TraceTypeSystemContext version lacks the UnmanagedPdbSymbolReader fallback). This is pre-existing tech debt — not introduced by this PR — but means any future fix to one must be manually applied to the other.

[Copilot]

Pull request overview

Updates symbol/PDB association logic in CoreCLR tooling to tolerate malformed CodeView debug directory entries by skipping them instead of crashing, aligning behavior with other PDB-loading failure handling.

Changes:

• Catch BadImageFormatException from PEReader.ReadCodeViewDebugDirectoryData and continue to the next debug entry.
• Apply the same robustness fix in both dotnet-pgo and the common compiler type system context.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/tools/dotnet-pgo/TraceTypeSystemContext.cs	Skips malformed CodeView entries when reading CodeView debug directory data while locating associated PDBs.
src/coreclr/tools/Common/Compiler/CompilerTypeSystemContext.cs	Matches the same exception-handling behavior for CodeView debug directory parsing during PDB association.

[Copilot]

There’s an extra double-space before the '=' in the candidatePath declaration; please normalize spacing to a single space for consistency/readability.