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

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

Patch By: leonid.mashinskiy

Reviewers: zturner, aleksandr.urakov

Reviewed By: zturner

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D56725

llvm-svn: 352615

5 files changed, 102 insertions, 55 deletions

diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/DbiStream.h b/llvm/include/llvm/DebugInfo/PDB/Native/DbiStream.h
index 438b6e730a0..7d75c159b7a 100644
--- a/llvm/include/llvm/DebugInfo/PDB/Native/DbiStream.h
+++ b/llvm/include/llvm/DebugInfo/PDB/Native/DbiStream.h
@@ -10,6 +10,7 @@
 #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"
@@ -79,7 +80,10 @@ public:
 
   FixedStreamArray<object::coff_section> getSectionHeaders() const;
 
-  FixedStreamArray<object::FpoData> getFpoRecords();
+  bool hasOldFpoRecords() const;
+  FixedStreamArray<object::FpoData> getOldFpoRecords() const;
+  bool hasNewFpoRecords() const;
+  const codeview::DebugFrameDataSubsectionRef &getNewFpoRecords() const;
 
   FixedStreamArray<SecMapEntry> getSectionMap() const;
   void visitSectionContributions(ISectionContribVisitor &Visitor) const;
@@ -90,7 +94,11 @@ 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;
 
@@ -116,8 +124,11 @@ private:
   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;
 };
diff --git a/llvm/lib/DebugInfo/PDB/Native/DbiStream.cpp b/llvm/lib/DebugInfo/PDB/Native/DbiStream.cpp
index 248477aa37f..ea15bcd01f5 100644
--- a/llvm/lib/DebugInfo/PDB/Native/DbiStream.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/DbiStream.cpp
@@ -126,8 +126,10 @@ Error DbiStream::reload(PDBFile *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,
@@ -200,8 +202,16 @@ FixedStreamArray<object::coff_section> DbiStream::getSectionHeaders() const {
   return SectionHeaders;
 }
 
-FixedStreamArray<object::FpoData> DbiStream::getFpoRecords() {
-  return FpoRecords;
+bool DbiStream::hasOldFpoRecords() const { return OldFpoStream != nullptr; }
+
+FixedStreamArray<object::FpoData> DbiStream::getOldFpoRecords() const {
+  return OldFpoRecords;
+}
+
+bool DbiStream::hasNewFpoRecords() const { return NewFpoStream != nullptr; }
+
+const DebugFrameDataSubsectionRef &DbiStream::getNewFpoRecords() const {
+  return NewFpoRecords;
 }
 
 const DbiModuleList &DbiStream::modules() const { return Modules; }
@@ -246,22 +256,15 @@ Error DbiStream::initializeSectionContributionData() {
 
 // 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,
@@ -278,39 +281,69 @@ Error DbiStream::initializeSectionHeadersData(PDBFile *Pdb) {
 }
 
 // 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;
 }
diff --git a/llvm/test/tools/llvm-pdbdump/Inputs/FPOTest.pdb b/llvm/test/tools/llvm-pdbdump/Inputs/FPOTest.pdb
new file mode 100644
index 00000000000..7096328b39e
--- /dev/null
+++ b/llvm/test/tools/llvm-pdbdump/Inputs/FPOTest.pdb
diff --git a/llvm/test/tools/llvm-pdbdump/fpo-data.test b/llvm/test/tools/llvm-pdbdump/fpo-data.test
new file mode 100644
index 00000000000..ac909cf5e7c
--- /dev/null
+++ b/llvm/test/tools/llvm-pdbdump/fpo-data.test
@@ -0,0 +1,14 @@
+; RUN: llvm-pdbutil dump -fpo %p/Inputs/FPOTest.pdb \
+; RUN:     | FileCheck %s
+
+CHECK:                        Old FPO Data                        
+CHECK-NEXT: ============================================================
+CHECK-NEXT:   RVA    | Code | Locals | Params | Prolog | Saved Regs | Use BP | Has SEH | Frame Type
+CHECK-NEXT: 0000004E |   19 |      0 |      0 |      0 |          0 |  false |   false |       FPO
+
+CHECK:                        New FPO Data                        
+CHECK-NEXT: ============================================================
+CHECK-NEXT:   RVA    | Code | Locals | Params | Stack | Prolog | Saved Regs | Has SEH | Has C++EH | Start | Program
+CHECK-NEXT: 00001010 |   18 |      0 |      0 |     0 |      4 |          0 |   false |     false |  true | $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + = 
+CHECK-NEXT: 00001011 |   17 |      0 |      0 |     0 |      3 |          4 |   false |     false | false | $T0 .raSearch = $eip $T0 ^ = $esp $T0 4 + = $ebp $T0 4 - ^ = 
+CHECK-NEXT: 00001013 |   15 |      0 |      0 |     0 |      1 |          4 |   false |     false | false | $T0 $ebp 4 + = $eip $T0 ^ = $esp $T0 4 + = $ebp $T0 4 - ^ = 
\ No newline at end of file
diff --git a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
index 65752ac9e05..23f9af6ab10 100644
--- a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
+++ b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
@@ -1010,17 +1010,12 @@ Error DumpOutputStyle::dumpOldFpo(PDBFile &File) {
   ExitOnError Err("Error dumping old fpo data:");
   auto &Dbi = Err(File.getPDBDbiStream());
 
-  uint32_t Index = Dbi.getDebugStreamIndex(DbgHeaderType::FPO);
-  if (Index == kInvalidStreamIndex) {
+  if (!Dbi.hasOldFpoRecords()) {
     printStreamNotPresent("FPO");
     return Error::success();
   }
 
-  std::unique_ptr<MappedBlockStream> OldFpo = File.createIndexedStream(Index);
-  BinaryStreamReader Reader(*OldFpo);
-  FixedStreamArray<object::FpoData> Records;
-  Err(Reader.readArray(Records,
-                       Reader.bytesRemaining() / sizeof(object::FpoData)));
+  const FixedStreamArray<object::FpoData>& Records = Dbi.getOldFpoRecords();
 
   P.printLine("  RVA    | Code | Locals | Params | Prolog | Saved Regs | Use "
               "BP | Has SEH | Frame Type");
@@ -1042,18 +1037,12 @@ Error DumpOutputStyle::dumpNewFpo(PDBFile &File) {
   ExitOnError Err("Error dumping new fpo data:");
   auto &Dbi = Err(File.getPDBDbiStream());
 
-  uint32_t Index = Dbi.getDebugStreamIndex(DbgHeaderType::NewFPO);
-  if (Index == kInvalidStreamIndex) {
+  if (!Dbi.hasNewFpoRecords()) {
     printStreamNotPresent("New FPO");
     return Error::success();
   }
 
-  std::unique_ptr<MappedBlockStream> NewFpo = File.createIndexedStream(Index);
-
-  DebugFrameDataSubsectionRef FDS;
-  if (auto EC = FDS.initialize(*NewFpo))
-    return make_error<RawError>(raw_error_code::corrupt_file,
-                                "Invalid new fpo stream");
+  const DebugFrameDataSubsectionRef& FDS = Dbi.getNewFpoRecords();
 
   P.printLine("  RVA    | Code | Locals | Params | Stack | Prolog | Saved Regs "
               "| Has SEH | Has C++EH | Start | Program");