[PATCH] D31636: [PDB] Emit index/offset pairs for TPI and IPI streams

[Reid Kleckner via Phabricator via llvm-commits]

rnk marked 2 inline comments as done.
rnk added inline comments.


================
Comment at: lld/test/COFF/pdb.test:10
+# RUN: llvm-pdbdump raw -modules -section-map -section-headers -section-contribs \
+# RUN:   -tpi-records %t.pdb | FileCheck -check-prefix RAW %s
+
----------------
What actually happened with this test is that @ruiu removed the llvm-pdbdump raw FileCheck line in rL291739, and these RAW checks became stale. They were essentially a "golden output" style test that looks at the complete PDB output before I touched it, and that's where all the diffs other than the TypeIndexOffsets diff come from.

I've re-added the RUN line and regenerated the output in this change because I want to look for the TypeIndexOffset goo at the end of the type stream.

Here's what I'm going to do: I'll land this change and update the RAW checks and we can figure out what we want to do about them separately. This change will just add the TypeIndexOffset checks.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp:52-55
+    TypeIndexOffsets.push_back(
+        {codeview::TypeIndex(codeview::TypeIndex::FirstNonSimpleIndex +
+                             TypeRecords.size()),
+         ulittle32_t(TypeRecordBytes)});
----------------
zturner wrote:
> `emplace_back`?  This way you don't have to use the brace initializer syntax.  I think `TypeIndex` constructor is implicit too, so you might even be able to just write `TypeIndexOffsets.emplace_back(codeview::TypeIndex::FirstNonSimpleIndex + TypeRecords.size(), ulittle32_t(TypeRecordBytes));`
That doesn't work because TypeIndexOffset is POD. The braced init syntax allows you to make something of that type but you can't construct one by calling the constructor like emplace_back does. At least, that's how I interpret this spew:
```
[1/4] Building CXX object lib\DebugInfo\PDB\CMakeFiles\LLVMDebugInfoPDB.dir\Native\TpiStreamBuilder.cpp.obj
FAILED: lib/DebugInfo/PDB/CMakeFiles/LLVMDebugInfoPDB.dir/Native/TpiStreamBuilder.cpp.obj
C:\src\llvm-project\build_stage2\bin\clang-cl.exe   /nologo -DGTEST_HAS_RTTI=0 -DLLVM_BUILD_GLOBAL_ISEL -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_DEBUG_POINTER_IMPL="" -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib\DebugInfo\PDB -IC:\src\llvm-project\llvm\lib\DebugInfo\PDB -Iinclude -IC:\src\llvm-project\llvm\include -I"C:\PROGRA~2\MICROS~1.0\DIA SDK\include" /DWIN32 /D_WINDOWS /W3   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Brepro -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wcovered-switch-default -Wdelete-non-virtual-dtor -Wstring-conversion /MD /O2 /Ob2   -UNDEBUG  /EHs-c- /GR- /showIncludes /Folib\DebugInfo\PDB\CMakeFiles\LLVMDebugInfoPDB.dir\Native\TpiStreamBuilder.cpp.obj /Fdlib\DebugInfo\PDB\CMakeFiles\LLVMDebugInfoPDB.dir\ -c C:\src\llvm-project\llvm\lib\DebugInfo\PDB\Native\TpiStreamBuilder.cpp
In file included from C:\src\llvm-project\llvm\lib\DebugInfo\PDB\Native\TpiStreamBuilder.cpp:10:
In file included from C:\src\llvm-project\llvm\include\llvm/DebugInfo/PDB/Native/TpiStreamBuilder.h:14:
In file included from C:\src\llvm-project\llvm\include\llvm/DebugInfo/CodeView/TypeRecord.h:13:
In file included from C:\src\llvm-project\llvm\include\llvm/ADT/APSInt.h:18:
In file included from C:\src\llvm-project\llvm\include\llvm/ADT/APInt.h:20:
In file included from C:\src\llvm-project\llvm\include\llvm/Support/MathExtras.h:19:
In file included from C:/PROGRA~2/MICROS~1.0/VC/include\algorithm:6:
In file included from C:/PROGRA~2/MICROS~1.0/VC/include\xmemory:6:
C:/PROGRA~2/MICROS~1.0/VC/include\xmemory0(737,24):  error: no matching constructor for initialization of 'llvm::pdb::TypeIndexOffset'
                ::new ((void *)_Ptr) _Objty(_STD forward<_Types>(_Args)...);
                                     ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:/PROGRA~2/MICROS~1.0/VC/include\xmemory0(857,7):  note: in instantiation of function template specialization 'std::allocator<llvm::pdb::TypeIndexOffset>::construct<llvm::pdb::TypeIndexOffset, llvm::codeview::TypeIndex, llvm::support::detail::packed_endian_specific_integral<unsigned int, llvm::support::endianness::little, 1> >' requested here
                _Al.construct(_Ptr, _STD forward<_Types>(_Args)...);
                    ^
C:/PROGRA~2/MICROS~1.0/VC/include\xmemory0(995,14):  note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<llvm::pdb::TypeIndexOffset> >::construct<llvm::pdb::TypeIndexOffset, llvm::codeview::TypeIndex, llvm::support::detail::packed_endian_specific_integral<unsigned int, llvm::support::endianness::little, 1> >' requested here
                _Mytraits::construct(*this, _Ptr,
                           ^
C:/PROGRA~2/MICROS~1.0/VC/include\vector(929,18):  note: in instantiation of function template specialization 'std::_Wrap_alloc<std::allocator<llvm::pdb::TypeIndexOffset> >::construct<llvm::pdb::TypeIndexOffset, llvm::codeview::TypeIndex, llvm::support::detail::packed_endian_specific_integral<unsigned int, llvm::support::endianness::little, 1> >' requested here
                this->_Getal().construct(_Unfancy(this->_Mylast()),
                               ^
C:\src\llvm-project\llvm\lib\DebugInfo\PDB\Native\TpiStreamBuilder.cpp(52,22):  note: in instantiation of function template specialization 'std::vector<llvm::pdb::TypeIndexOffset, std::allocator<llvm::pdb::TypeIndexOffset> >::emplace_back<llvm::codeview::TypeIndex, llvm::support::detail::packed_endian_specific_integral<unsigned int, llvm::support::endianness::little, 1> >' requested here
    TypeIndexOffsets.emplace_back(
                     ^
C:\src\llvm-project\llvm\include\llvm/DebugInfo/PDB/Native/RawTypes.h(78,8):  note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 2 were provided
struct TypeIndexOffset {
       ^
C:\src\llvm-project\llvm\include\llvm/DebugInfo/PDB/Native/RawTypes.h(78,8):  note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 2 were provided
C:\src\llvm-project\llvm\include\llvm/DebugInfo/PDB/Native/RawTypes.h(78,8):  note: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 2 were provided

```


================
Comment at: llvm/test/DebugInfo/PDB/pdb-yaml-types.test:29-31
+This test is mostly checking to make sure we include the type index offset
+table, and eventually hash codes. The type index offsets should be similar to
+what are already present in big-read.pdb.
----------------
zturner wrote:
> Maybe I just never knew you could do this, but can you put random text in the middle of a test file with no comment marker?
Yeah, .test isn't really a file type, it's just an extension that identifies a text file that should get run by lit. lit looks for RUN:, and FileCheck looks for CHECK:, and if you don't use the test as a source file, you don't need comment markers. I could get rid of ; before RUN, I guess.


https://reviews.llvm.org/D31636