[NativePDB] Fix access to both old & new fpo data entries from dbi stream

[leonid.mashinskiy]

Summary

This patch fixes access to fpo streams in native pdb from DbiStream and makes code consistent with DbiStreamBuilder.

Required for D55122

[aleksandr.urakov]

LGTM except the comment. Let's also wait for what Zachary will say.

lib/DebugInfo/PDB/Native/DbiStream.cpp

306–323

May be it would be worth to create a private method for this to avoid duplication (e.g. CreateIndexedStreamForHeaderType)?

[zturner]

Sorry about this, it slipped off my radar.

Two things I think we should do:

1. Dump these records as part of llvm-pdbutil dump -fpo

2. Using the results of #1, we can add a test for this. Check in a very small PDB that was built with /nodefaultlib by linking in an assembly file with a .debug$F section (if you can't figure out how to produce one, libcmtd has some. You can just use obj2yaml on them and strip out everything but the .debug$F section). This resulting PDB shoudl have both new and old fpo records, then we can test the output using llvm-pdbutil dump -fpo.

[leonid.mashinskiy]

• used new fpo related methods from DbiStream in llvm-pdbutil
• added test to check fpo data using llvm-pdbutil

Please note that diff contains artificially constructed binary pdb file for test

[leonid.mashinskiy]

Thank you for the review!
Please, commit this for me? I have no commit access but I need to attach pdb from patch that phabricator removed from raw diff.

include/llvm/DebugInfo/PDB/Native/DbiStream.h

 //===- DbiStream.h - PDB Dbi Stream (Stream 3) Access -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_DEBUGINFO_PDB_RAW_PDBDBISTREAM_H
 #define LLVM_DEBUGINFO_PDB_RAW_PDBDBISTREAM_H
 
 #include "llvm/DebugInfo/CodeView/DebugSubsection.h"
+#include "llvm/DebugInfo/CodeView/DebugFrameDataSubsection.h"
 #include "llvm/DebugInfo/MSF/MappedBlockStream.h"
 #include "llvm/DebugInfo/PDB/Native/DbiModuleDescriptor.h"
 #include "llvm/DebugInfo/PDB/Native/DbiModuleList.h"
 #include "llvm/DebugInfo/PDB/Native/PDBStringTable.h"
 #include "llvm/DebugInfo/PDB/Native/RawConstants.h"
 #include "llvm/DebugInfo/PDB/Native/RawTypes.h"
 #include "llvm/DebugInfo/PDB/PDBTypes.h"
 #include "llvm/Support/BinaryStreamArray.h"
public:
   /// If the given stream type is present, returns its stream index. If it is
   /// not present, returns InvalidStreamIndex.
   uint32_t getDebugStreamIndex(DbgHeaderType Type) const;
 
   const DbiModuleList &modules() const;
 
   FixedStreamArray<object::coff_section> getSectionHeaders() const;
 
-  FixedStreamArray<object::FpoData> getFpoRecords();
+  FixedStreamArray<object::FpoData> getOldFpoRecords() const;
+  const codeview::DebugFrameDataSubsectionRef &getNewFpoRecords() const;
 
   FixedStreamArray<SecMapEntry> getSectionMap() const;
   void visitSectionContributions(ISectionContribVisitor &Visitor) const;
 
   Expected<StringRef> getECName(uint32_t NI) const;
 
 private:
   Error initializeSectionContributionData();
   Error initializeSectionHeadersData(PDBFile *Pdb);
   Error initializeSectionMapData();
-  Error initializeFpoRecords(PDBFile *Pdb);
+  Error initializeOldFpoRecords(PDBFile *Pdb);
+  Error initializeNewFpoRecords(PDBFile *Pdb);
+
+  Expected<std::unique_ptr<msf::MappedBlockStream>>
+  createIndexedStreamForHeaderType(PDBFile *Pdb, DbgHeaderType Type) const;
 
   std::unique_ptr<BinaryStream> Stream;
 
   PDBStringTable ECNames;
 
   BinarySubstreamRef SecContrSubstream;
   BinarySubstreamRef SecMapSubstream;
   BinarySubstreamRef ModiSubstream;
PdbRaw_DbiSecContribVer SectionContribVersion =
       PdbRaw_DbiSecContribVer::DbiSecContribVer60;
   FixedStreamArray<SectionContrib> SectionContribs;
   FixedStreamArray<SectionContrib2> SectionContribs2;
   FixedStreamArray<SecMapEntry> SectionMap;
 
   std::unique_ptr<msf::MappedBlockStream> SectionHeaderStream;
   FixedStreamArray<object::coff_section> SectionHeaders;
 
-  std::unique_ptr<msf::MappedBlockStream> FpoStream;
-  FixedStreamArray<object::FpoData> FpoRecords;
+  std::unique_ptr<msf::MappedBlockStream> OldFpoStream;
+  FixedStreamArray<object::FpoData> OldFpoRecords;
+  
+  std::unique_ptr<msf::MappedBlockStream> NewFpoStream;
+  codeview::DebugFrameDataSubsectionRef NewFpoRecords;
 
   const DbiStreamHeader *Header;
 };
 }
 }
 
 #endif

lib/DebugInfo/PDB/Native/DbiStream.cpp

if (auto EC = Modules.initialize(ModiSubstream.StreamData,
     return EC;
 
   if (auto EC = initializeSectionContributionData())
     return EC;
   if (auto EC = initializeSectionHeadersData(Pdb))
     return EC;
   if (auto EC = initializeSectionMapData())
     return EC;
-  if (auto EC = initializeFpoRecords(Pdb))
+  if (auto EC = initializeOldFpoRecords(Pdb))
+    return EC;
+  if (auto EC = initializeNewFpoRecords(Pdb))
      return EC;
 
   if (Reader.bytesRemaining() > 0)
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Found unexpected bytes in DBI Stream.");
 
   if (!ECSubstream.empty()) {
     BinaryStreamReader ECReader(ECSubstream.StreamData);
     if (auto EC = ECNames.reload(ECReader))
PDB_Machine DbiStream::getMachineType() const {
   uint16_t Machine = Header->MachineType;
   return static_cast<PDB_Machine>(Machine);
 }
 
 FixedStreamArray<object::coff_section> DbiStream::getSectionHeaders() const {
   return SectionHeaders;
 }
 
-FixedStreamArray<object::FpoData> DbiStream::getFpoRecords() {
-  return FpoRecords;
+FixedStreamArray<object::FpoData> DbiStream::getOldFpoRecords() const {
+  return OldFpoRecords;
+}
+
+const DebugFrameDataSubsectionRef& DbiStream::getNewFpoRecords() const {
+  return NewFpoRecords;
 }
 
 const DbiModuleList &DbiStream::modules() const { return Modules; }
 
 FixedStreamArray<SecMapEntry> DbiStream::getSectionMap() const {
   return SectionMap;
 }
 
if (SectionContribVersion == DbiSecContribV2)
     return loadSectionContribs<SectionContrib2>(SectionContribs2, SCReader);
 
   return make_error<RawError>(raw_error_code::feature_unsupported,
                               "Unsupported DBI Section Contribution version");
 }
 
 // Initializes this->SectionHeaders.
 Error DbiStream::initializeSectionHeadersData(PDBFile *Pdb) {
-  if (!Pdb)
-    return Error::success();
-
-  if (DbgStreams.size() == 0)
-    return Error::success();
+  Expected<std::unique_ptr<msf::MappedBlockStream>> ExpectedStream =
+      createIndexedStreamForHeaderType(Pdb, DbgHeaderType::SectionHdr);
+  if (auto EC = ExpectedStream.takeError())
+    return EC;
 
-  uint32_t StreamNum = getDebugStreamIndex(DbgHeaderType::SectionHdr);
-  if (StreamNum == kInvalidStreamIndex)
+  auto &SHS = *ExpectedStream;
+  if (!SHS)
     return Error::success();
 
-  if (StreamNum >= Pdb->getNumStreams())
-    return make_error<RawError>(raw_error_code::no_stream);
-
-  auto SHS = MappedBlockStream::createIndexedStream(
-      Pdb->getMsfLayout(), Pdb->getMsfBuffer(), StreamNum, Pdb->getAllocator());
-
   size_t StreamLen = SHS->getLength();
   if (StreamLen % sizeof(object::coff_section))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Corrupted section header stream.");
 
   size_t NumSections = StreamLen / sizeof(object::coff_section);
   BinaryStreamReader Reader(*SHS);
   if (auto EC = Reader.readArray(SectionHeaders, NumSections))
     return make_error<RawError>(raw_error_code::corrupt_file,
                                 "Could not read a bitmap.");
 
   SectionHeaderStream = std::move(SHS);
   return Error::success();
 }
 
 // Initializes this->Fpos.
-Error DbiStream::initializeFpoRecords(PDBFile *Pdb) {
-  if (!Pdb)
-    return Error::success();
-
-  if (DbgStreams.size() == 0)
-    return Error::success();
-
-  uint32_t StreamNum = getDebugStreamIndex(DbgHeaderType::NewFPO);
+Error DbiStream::initializeOldFpoRecords(PDBFile *Pdb) {
+  Expected<std::unique_ptr<msf::MappedBlockStream>> ExpectedStream =
+      createIndexedStreamForHeaderType(Pdb, DbgHeaderType::FPO);
+  if (auto EC = ExpectedStream.takeError())
+    return EC;
 
-  // This means there is no FPO data.
-  if (StreamNum == kInvalidStreamIndex)
+  auto &FS = *ExpectedStream;
+  if (!FS)
     return Error::success();
 
-  if (StreamNum >= Pdb->getNumStreams())
-    return make_error<RawError>(raw_error_code::no_stream);
-
-  auto FS = MappedBlockStream::createIndexedStream(
-      Pdb->getMsfLayout(), Pdb->getMsfBuffer(), StreamNum, Pdb->getAllocator());
-
   size_t StreamLen = FS->getLength();
   if (StreamLen % sizeof(object::FpoData))
     return make_error<RawError>(raw_error_code::corrupt_file,
-                                "Corrupted New FPO stream.");
+                                "Corrupted Old FPO stream.");
 
   size_t NumRecords = StreamLen / sizeof(object::FpoData);
   BinaryStreamReader Reader(*FS);
-  if (auto EC = Reader.readArray(FpoRecords, NumRecords))
+  if (auto EC = Reader.readArray(OldFpoRecords, NumRecords))
     return make_error<RawError>(raw_error_code::corrupt_file,
-                                "Corrupted New FPO stream.");
-  FpoStream = std::move(FS);
+                                "Corrupted Old FPO stream.");
+  OldFpoStream = std::move(FS);
+  return Error::success();
+}
+
+Error DbiStream::initializeNewFpoRecords(PDBFile *Pdb) {
+  Expected<std::unique_ptr<msf::MappedBlockStream>> ExpectedStream =
+      createIndexedStreamForHeaderType(Pdb, DbgHeaderType::NewFPO);
+  if (auto EC = ExpectedStream.takeError())
+    return EC;
+
+  auto &FS = *ExpectedStream;
+  if (!FS)
+    return Error::success();
+
+  if (auto EC = NewFpoRecords.initialize(*FS))
+    return EC;
+
+  NewFpoStream = std::move(FS);
   return Error::success();
 }
 
+Expected<std::unique_ptr<msf::MappedBlockStream>>
+DbiStream::createIndexedStreamForHeaderType(PDBFile *Pdb,
+                                            DbgHeaderType Type) const {
+  if (!Pdb)
+    return nullptr;
+
+  if (DbgStreams.empty())
+    return nullptr;
+
+  uint32_t StreamNum = getDebugStreamIndex(Type);
+
+  // This means there is no such stream
+  if (StreamNum == kInvalidStreamIndex)
+    return nullptr;
+
+  if (StreamNum >= Pdb->getNumStreams())
+    return make_error<RawError>(raw_error_code::no_stream);
+
+  return MappedBlockStream::createIndexedStream(
+      Pdb->getMsfLayout(), Pdb->getMsfBuffer(), StreamNum, Pdb->getAllocator());
+}
+
 BinarySubstreamRef DbiStream::getSectionContributionData() const {
   return SecContrSubstream;
 }
 
 BinarySubstreamRef DbiStream::getSecMapSubstreamData() const {
   return SecMapSubstream;
 }