[PATCH] D75081: [CodeView] Align type records on 4-bytes when emitting PDBs

[Alexandre Ganea via Phabricator via llvm-commits]

aganea updated this revision to Diff 249720.
aganea marked 4 inline comments as done.
aganea added a comment.

In D75081#1917209 <https://reviews.llvm.org/D75081#1917209>, @amccarth wrote:

> I find the assert messages and some of the comments mildly misleading.


Fixed, please check the updated diff. Let me know if that's better.

> Not knowing the details of how the records are actually read back and used, I harbor some concern that padding out the size of the record might confuse the consumer.  If a record actually works out to 43 bytes, this code will copy the 43 bytes, add one pad byte, and change the record size to 44.  Is it important that the consumer know the original size was 43 so that they don't mistake the pad byte for actual record data?

The consumer only uses the `llvm::codeview::RecordPrefix::RecordKind`, which maps to the LF_* types (see `TypeRecord.h`). The record length `llvm::codeview::RecordPrefix::RecordLen` -- which we're modifying in this patch -- is only used for quickly skimming through a PDBs in the Visual Studio debugger: https://github.com/microsoft/microsoft-pdb/blob/082c5290e5aff028ae84e43affa8be717aa7af73/PDB/include/symtypeutils.h#L190. I suppose one reason for this 4 byte alignment is that unaligned reads were more expensive at the time the spec/code were written.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75081/new/

https://reviews.llvm.org/D75081

Files:
  lld/test/COFF/pdb-tpi-aligned-records.test
  llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h
  llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
  llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
  llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75081.249720.patch
Type: text/x-patch
Size: 7010 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200311/f5aa81c5/attachment.bin>