[PATCH] D79672: [COFF] Move type merging to TpiSource::mergeDebugT virtual method

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added a subscriber: santagada.
aganea added inline comments.


================
Comment at: lld/COFF/DebugTypes.cpp:88
+    if (!f->pchSignature || !*f->pchSignature)
+      fatal(toString(f) + " claims to be a precompiled headers object, but "
+                          "does not have a valid signature!");
----------------
We will need tests for these I think. Same for the other `fatal` below.


================
Comment at: lld/COFF/DebugTypes.cpp:172
+  debugH = debugH.drop_front(sizeof(object::debug_h_header));
+  return header->Magic == COFF::DEBUG_HASHES_SECTION_MAGIC &&
+         header->Version == 0 &&
----------------
I recall @santagada saying that linking `.debug$H` .OBJs with MSVC link.exe would fail. While browsing the microsoft-pdb repo, I was under the impression at one point that this 'Magic' field is actually a size of some sort to read more data. If link.exe treats it as a size, it would certainly fail in some way or another. This is unrelated to this patch, but it'd be good to clarify that situation.


================
Comment at: lld/COFF/DebugTypes.cpp:178
 
-// If existing, return the actual PDB path on disk.
-static Optional<std::string> findPdbPath(StringRef pdbPath,
-                                         const ObjFile *dependentFile) {
-  // Ensure the file exists before anything else. In some cases, if the path
-  // points to a removable device, Driver::enqueuePath() would fail with an
-  // error (EAGAIN, "resource unavailable try again") which we want to skip
-  // silently.
-  if (llvm::sys::fs::exists(pdbPath))
-    return normalizePdbPath(pdbPath);
-  std::string ret = getPdbBaseName(dependentFile, pdbPath);
-  if (llvm::sys::fs::exists(ret))
-    return normalizePdbPath(ret);
-  return None;
+static Optional<ArrayRef<uint8_t>> getDebugH(ObjFile *file) {
+  SectionChunk *sec =
----------------
I started re-basing this patch at some point, and one thing that I wanted to do was to move all this .debug$H calculation into llvm/lib/DebugInfo/CodeView/. This was to be able to re-create .debug$H streams from `llvm-objcopy`, see D59025. The end-goal was to be able to run this step (`llvm-objcopy`) on a remote host, after MSVC cl.exe compilation. That way, you'll speed-up LLD linking when building with MSVC, and your cached .OBJs would already contain .debug$H in all cases. Again, this could be done later, I simply wanted to raise it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79672