[PATCH] D94267: [PDB] Defer relocating .debug$S until commit time and parallelize it

Wed Jan 20 10:00:10 PST 2021

rnk added a comment.

I debugged this yesterday. The bug is from container overflow, which is only detectable with an ASan instrumented build of libc++, which I don't have. I made some local source changes to replace a std::vector with a temporary array, and I was able to observe the bug.

================
Comment at: lld/COFF/Chunks.cpp:451
+    // Stop if the relocation does not apply to this subsection.
+    if (rel.VirtualAddress >= vaEnd)
       break;
----------------
This is the buggy bounds check. At this point, we don't know how large the relocation is, so it's hard to check. We have an implicit assumption here that all relocations are entirely within or outside of the subsection that is being relocated. The assumption is correct for well-formed debug information.

However, we have what appear to be invalid input objects in the pdb-file-static test case. These input objects are yaml-ified object files from MSVC. It seems that obj2yaml/yambl2obj of MSVC object files does not round trip. The relocations, which are maintained separately from the symbol records, do not correspond to the symbol record offsets that yaml2obj generates. We know that MSVC does not align symbol records to 4 bytes, but LLVM does. This may be the source of the discrepancy.

This bug is actually already present in LLD. An object file with a relocation that points to the last byte of a section will cause LLD to do an OOB write. Since LLD applies relocations directly to the output file memory, it is hard to observe or exploit this bug. However, given this recent refactoring, I think it might be worth going back and fixing it.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94267/new/

https://reviews.llvm.org/D94267