[PATCH] D34898: [PDB] Fill in "Parent" and "End" fields of scope-like symbol records

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/test/COFF/pdb-scopes.test:38
+CHECK: - S_GPROC32_ID [size = 44] `g`
+CHECK:     parent = 0, addr = 0002:0000, code size = 5, end = 196
+CHECK:     debug start = 4, debug end = 4, flags = none
----------------
Would it be helpful to format this (and similar) records so that `parent = 0, end = 196` is at the beginning of the string, so that you can just match that and leave out the rest of the (relatively fragile) debug info which we're not trying to test?


================
Comment at: lld/test/COFF/pdb-scopes.test:40-42
+CHECK: - S_FRAMEPROC [size = 32]
+CHECK: - S_REGREL32 [size = 16] `x`
+CHECK:     type = 0x0074 (int), register = rsp, offset = 8
----------------
Remove these from the test as they aren't relevant?


================
Comment at: lld/test/COFF/pdb-scopes.test:43
+CHECK:     type = 0x0074 (int), register = rsp, offset = 8
+CHECK: - S_END [size = 4]
+CHECK: - S_GPROC32_ID [size = 44] `main`
----------------
I wonder if we should display the offset of each symbol in the output.  For types we write something like:

```
 0x1017 | LF_CLASS [size = 60]
           class name: `MembersTest::A`
           unique name: `.?AVA at MembersTest@@`
           vtable: <no type>, base list: <no type>, field list: <no type>
           options: forward ref | has unique name
```

But we have no such analogue for symbols.  The only way to uniquely identify a symbol is by its offset in the symbol stream, but since this is what the `Parent` and `End` fields do anyway, perhaps we could display it?  This would also allow the test to be more robust, as you could say:

```
CHECK: -    0 | S_GPROC32_ID [size = 44] `g`
CHECK:            parent = 0, addr = 0002:0000, code size = 5, end = [[END:r[0-9]+]]
CHECK:            debug start = 4, debug end = 4, flags = none
CHECK: -  [[END]] | S_END [size = 4]
```

Right now we're verifying that we're writing //some number// for the field, but we're not verifying that it's the //correct// number


================
Comment at: lld/test/COFF/pdb-scopes.test:57-58
+CHECK: - S_BLOCK32 [size = 24] ``
+CHECK:     parent = 200, addr = 0002:0050
+CHECK:     code size = 17, end = 380
+CHECK: - S_REGREL32 [size = 16] `y`
----------------
This ties into what I was saying earlier, but it's not clear to me what this number `200` represents.  It doesn't seem to be the offset of any symbol.  (I did the math by hand, and the offsets are:

```
S_GPROC32     0
S_FRAMEPROC   44
S_REGREL32    76
S_END         92
S_GPROC32_ID  96
S_FRAMEPROC   140
S_REGREL32    172
S_BLOCK32     192
S_REGREL32    216
S_END         232
S_BLOCK32     240
```

Nothing has offset 200.  So what is this number?


https://reviews.llvm.org/D34898