[llvm] r344081 - [PDB] Fix another bug in globals stream name lookup.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 9 14:19:03 PDT 2018


Author: zturner
Date: Tue Oct  9 14:19:03 2018
New Revision: 344081

URL: http://llvm.org/viewvc/llvm-project?rev=344081&view=rev
Log:
[PDB] Fix another bug in globals stream name lookup.

When we're on the last bucket the computation is tricky.
We were failing when the last bucket contained multiple
matches.  Added a new test for this.

Added:
    llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.cpp
    llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.pdb   (with props)
Modified:
    llvm/trunk/lib/DebugInfo/PDB/Native/GlobalsStream.cpp
    llvm/trunk/test/DebugInfo/PDB/pdbdump-global-lookup.test

Modified: llvm/trunk/lib/DebugInfo/PDB/Native/GlobalsStream.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/PDB/Native/GlobalsStream.cpp?rev=344081&r1=344080&r2=344081&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/PDB/Native/GlobalsStream.cpp (original)
+++ llvm/trunk/lib/DebugInfo/PDB/Native/GlobalsStream.cpp Tue Oct  9 14:19:03 2018
@@ -56,22 +56,28 @@ GlobalsStream::findRecordsByName(StringR
   if (CompressedBucketIndex == -1)
     return Result;
 
-  uint32_t ChainStartOffset = GlobalsTable.HashBuckets[CompressedBucketIndex];
-  uint32_t NextChainOffset = GlobalsTable.HashBuckets.size() * 12;
   uint32_t LastBucketIndex = GlobalsTable.HashBuckets.size() - 1;
-  if (static_cast<uint32_t>(CompressedBucketIndex) < LastBucketIndex) {
-    NextChainOffset = GlobalsTable.HashBuckets[CompressedBucketIndex + 1];
+  uint32_t StartRecordIndex =
+      GlobalsTable.HashBuckets[CompressedBucketIndex] / 12;
+  uint32_t EndRecordIndex = 0;
+  if (LLVM_LIKELY(uint32_t(CompressedBucketIndex) < LastBucketIndex)) {
+    EndRecordIndex = GlobalsTable.HashBuckets[CompressedBucketIndex + 1];
+  } else {
+    // If this is the last bucket, it consists of all hash records until the end
+    // of the HashRecords array.
+    EndRecordIndex = GlobalsTable.HashRecords.size() * 12;
   }
-  ChainStartOffset /= 12;
-  NextChainOffset /= 12;
 
-  while (ChainStartOffset < NextChainOffset) {
-    PSHashRecord PSH = GlobalsTable.HashRecords[ChainStartOffset];
+  EndRecordIndex /= 12;
+
+  assert(EndRecordIndex <= GlobalsTable.HashRecords.size());
+  while (StartRecordIndex < EndRecordIndex) {
+    PSHashRecord PSH = GlobalsTable.HashRecords[StartRecordIndex];
     uint32_t Off = PSH.Off - 1;
     codeview::CVSymbol Record = Symbols.readRecord(Off);
     if (codeview::getSymbolName(Record) == Name)
       Result.push_back(std::make_pair(Off, std::move(Record)));
-    ++ChainStartOffset;
+    ++StartRecordIndex;
   }
   return Result;
 }

Added: llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.cpp?rev=344081&view=auto
==============================================================================
--- llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.cpp (added)
+++ llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.cpp Tue Oct  9 14:19:03 2018
@@ -0,0 +1,23 @@
+// Build with "cl.exe /Z7 /GR- /GS- -EHs-c- every-function.cpp /link /debug /nodefaultlib /incremental:no /entry:main"
+
+void __cdecl operator delete(void *, unsigned int) {}
+void __cdecl operator delete(void *, unsigned __int64) {}
+
+// Note: It's important that this particular function hashes to a higher bucket
+// number than any other function in the PDB.  When changing this test, ensure
+// that this requirement still holds.  This is because we need to test lookup
+// in the edge case where we try to look something up in the final bucket, which
+// has special logic.
+int OvlGlobalFn(int X) { return X + 42; }
+int OvlGlobalFn(int X, int Y) { return X + Y + 42; }
+int OvlGlobalFn(int X, int Y, int Z) { return X + Y + Z + 42; }
+
+static int StaticFn(int X) {
+  return X + 42;
+}
+
+int main(int argc, char **argv) {
+  // Make sure they don't get optimized out.
+  int Result = OvlGlobalFn(argc) + OvlGlobalFn(argc, argc) + OvlGlobalFn(argc, argc, argc) + StaticFn(argc);
+  return Result;
+}

Added: llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.pdb
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.pdb?rev=344081&view=auto
==============================================================================
Binary file - no diff available.

Propchange: llvm/trunk/test/DebugInfo/PDB/Inputs/global-name-lookup.pdb
------------------------------------------------------------------------------
    svn:mime-type = application/octet-stream

Modified: llvm/trunk/test/DebugInfo/PDB/pdbdump-global-lookup.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/PDB/pdbdump-global-lookup.test?rev=344081&r1=344080&r2=344081&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/PDB/pdbdump-global-lookup.test (original)
+++ llvm/trunk/test/DebugInfo/PDB/pdbdump-global-lookup.test Tue Oct  9 14:19:03 2018
@@ -1,30 +1,32 @@
 ; RUN: llvm-pdbutil dump -globals \
-; RUN:    -global-name="operator delete" \
 ; RUN:    -global-name=main \
+; RUN:    -global-name="operator delete" \
 ; RUN:    -global-name=abcdefg \
-; RUN:    %p/Inputs/every-function.pdb | FileCheck %s
+; RUN:    %p/Inputs/global-name-lookup.pdb | FileCheck %s
 
-; This is a separate command line invocation because B::PureFunc
-; is special.  It's in the last hash bucket, so it exercises a special
-; calculation path.
-; RUN: llvm-pdbutil dump -globals -global-name=B::PureFunc \
-; RUN:    %p/Inputs/symbolformat.pdb | FileCheck --check-prefix=PURE %s
+; RUN: llvm-pdbutil dump -globals \
+; RUN:    -global-name=OvlGlobalFn \
+; RUN:    %p/Inputs/global-name-lookup.pdb | FileCheck --check-prefix=LASTBUCKET %s
 
 CHECK:                        Global Symbols
 CHECK-NEXT: ============================================================
-CHECK-NEXT:     Global Name `operator delete`
-CHECK-NEXT:       1516 | S_PROCREF [size = 32] `operator delete`
-CHECK-NEXT:              module = 1, sum name = 0, offset = 324
-CHECK-NEXT:       1484 | S_PROCREF [size = 32] `operator delete`
-CHECK-NEXT:              module = 1, sum name = 0, offset = 184
 CHECK-NEXT:     Global Name `main`
-CHECK-NEXT:       2016 | S_PROCREF [size = 20] `main`
-CHECK-NEXT:              module = 1, sum name = 0, offset = 1952
+CHECK-NEXT:        344 | S_PROCREF [size = 20] `main`
+CHECK-NEXT:              module = 1, sum name = 0, offset = 780
+CHECK-NEXT:     Global Name `operator delete`
+CHECK-NEXT:        228 | S_PROCREF [size = 32] `operator delete`
+CHECK-NEXT:              module = 1, sum name = 0, offset = 200
+CHECK-NEXT:        196 | S_PROCREF [size = 32] `operator delete`
+CHECK-NEXT:              module = 1, sum name = 0, offset = 52
 CHECK-NEXT:     Global Name `abcdefg`
-CHECK-NEXT:       (no matching records found)
+CHECK-NEXT:        (no matching records found)
 
-PURE:                        Global Symbols
-PURE-NEXT: ============================================================
-PURE-NEXT:     Global Name `B::PureFunc`
-PURE-NEXT:        980 | S_PROCREF [size = 28] `B::PureFunc`
-PURE-NEXT:              module = 1, sum name = 0, offset = 800
+LASTBUCKET:                        Global Symbols
+LASTBUCKET-NEXT: ============================================================
+LASTBUCKET-NEXT:     Global Name `OvlGlobalFn`
+LASTBUCKET-NEXT:        316 | S_PROCREF [size = 28] `OvlGlobalFn`
+LASTBUCKET-NEXT:              module = 1, sum name = 0, offset = 608
+LASTBUCKET-NEXT:        288 | S_PROCREF [size = 28] `OvlGlobalFn`
+LASTBUCKET-NEXT:              module = 1, sum name = 0, offset = 464
+LASTBUCKET-NEXT:        260 | S_PROCREF [size = 28] `OvlGlobalFn`
+LASTBUCKET-NEXT:              module = 1, sum name = 0, offset = 348




More information about the llvm-commits mailing list