[PATCH] D122372: [lld][COFF] Fix TypeServerSource lookup on GUID collisions

[Tobias Hieta via Phabricator via llvm-commits]

thieta created this revision.
thieta added reviewers: rnk, mstorsjo, aganea, saudi, MaskRay.
Herald added a subscriber: StephenFan.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: LLVM.

Microsoft shipped a bunch of PDB files with broken/invalid GUIDs
which lead lld to use 0xFF as the key for these files in an internal
cache. When multiple files have this key it will lead to collisions
and confused symbol lookup.

Several approaches to fix this was considered. Including making the key
the path to the PDB file, but this requires some filesystem operations
in order to normalize the file path.

Since this only happens with malformatted PDB files and we haven't
seen this before they malformatted files where shipped with visual
studio we probably shouldn't optimize for this use-case.

Instead we now just don't insert files with Guid == 0xFF into the
cache map and warn if we get collisions so similar problems can be
found in the future instead of being silent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122372

Files:
  lld/COFF/DebugTypes.cpp


Index: lld/COFF/DebugTypes.cpp
===================================================================
--- lld/COFF/DebugTypes.cpp
+++ lld/COFF/DebugTypes.cpp
@@ -34,6 +34,10 @@
 using namespace lld;
 using namespace lld::coff;
 
+static codeview::GUID InvalidGuid{{0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+                                   0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+                                   0xFF, 0xFF}};
+
 namespace {
 class TypeServerIpiSource;
 
@@ -55,9 +59,20 @@
     if (!expectedInfo)
       return;
     Guid = expectedInfo->getGuid();
-    auto it = ctx.typeServerSourceMappings.emplace(Guid, this);
-    assert(it.second);
-    (void)it;
+    // Don't store the PDB in the map here if the Guid just contains
+    // 0xFF since that will lead to many collisions. This happens with
+    // certain Visual Studio versions where the standard library contains
+    // PDB files with broken/invalid Guids. This will just lead to
+    // a path lookup fallback which works fine - but is slower.
+    if (Guid != InvalidGuid) {
+      auto it = ctx.typeServerSourceMappings.emplace(Guid, this);
+      if (!it.second) {
+        TypeServerSource *tSrc = (TypeServerSource *)it.first->second;
+        warn("GUID collision between " + file.getFilePath() + " and " +
+             tSrc->pdbInputFile->session->getPDBFile().getFilePath());
+      }
+      (void)it;
+    }
   }
 
   Error mergeDebugT(TypeMerger *m) override;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122372.417848.patch
Type: text/x-patch
Size: 1453 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220324/9d247d91/attachment.bin>