[PATCH] D34906: [llvm-pdbutil] Output the symbol offset when dumping symbols

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/test/COFF/pdb-comdat.test:44
 CHECK-LABEL:   Mod 0000 | `{{.*}}pdb_comdat_main.obj`:
-CHECK:   - S_OBJNAME [size = 56] sig=0, `C:\src\llvm-project\build\pdb_comdat_main.obj`
-CHECK:   - S_COMPILE3 [size = 60]
-CHECK:       machine = intel x86-x64, Ver = Microsoft (R) Optimizing Compiler, language = c
-CHECK:       frontend = 19.0.24215.1, backend = 19.0.24215.1
-CHECK:       flags = security checks | hot patchable
-CHECK:   - S_GPROC32_ID [size = 44] `main`
-    FIXME: We need to fill in "end".
-CHECK:       parent = 0, addr = 0002:0000, code size = 24, end = 0
-CHECK:       debug start = 4, debug end = 19, flags = none
-CHECK:   - S_FRAMEPROC [size = 32]
-CHECK:       size = 40, padding size = 0, offset to padding = 0
-CHECK:       bytes of callee saved registers = 0, exception handler addr = 0000:0000
-CHECK:       flags = has async eh | opt speed
-CHECK:   - S_END [size = 4]
-CHECK:   - S_GDATA32 [size = 24] `global`
-CHECK:       type = 0x0074 (int), addr = 0000:0000
-CHECK:   - S_BUILDINFO [size = 8] BuildId = `4106`
-CHECK:   - S_GPROC32_ID [size = 44] `foo`
-CHECK:       parent = 0, addr = 0002:0032, code size = 15, end = 0
-CHECK:       debug start = 0, debug end = 14, flags = none
-CHECK:   - S_FRAMEPROC [size = 32]
-CHECK:       size = 0, padding size = 0, offset to padding = 0
-CHECK:       bytes of callee saved registers = 0, exception handler addr = 0000:0000
-CHECK:       flags = marked inline | has async eh | opt speed
-CHECK:   - S_END [size = 4]
+CHECK:     4 | S_OBJNAME [size = 56] sig=0, `C:\src\llvm-project\build\pdb_comdat_main.obj`
+CHECK:    60 | S_COMPILE3 [size = 60]
----------------
rnk wrote:
> I'd prefer to see offsets in hex.
I'm kind of of two minds here.  I wanted them to look visually distinct from type indices, and there's also the issue of the size fields.  Someone should be able to look at the size field (which is also printed) and easily compute the next offset.  It's harder to mentally do hex math than decimal math so but this way someone can easily see that Offset(N+1) = Offset(N) + Size(N)


================
Comment at: llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp:874
 
-  if (auto EC = Visitor.visitSymbolStream(*ExpectedSymbols))
+  if (auto EC = Visitor.visitSymbolStream(*ExpectedSymbols, 0))
     P.formatLine("Error while processing public symbol records.  {0}",
----------------
rnk wrote:
> Not 4? I guess it won't matter, the publics stream never has scope records.
Nope, it's actually 0 here.  Publics has a bunch of header information and then a field which says "the symbol records are over here somewhere in another stream".  And in that stream, the symbols are at offset 0.

I guess the conceptual difference is that the public symbol record stream is literally nothing but symbol records, whereas the module debug stream (which is where module symbols come from) contains an embedded //substream// which contains symbol records, so they put a 4 byte identifier before it that says "symbols are up next!".  Not really necessary, but that's about the only thing I can think of.


https://reviews.llvm.org/D34906