3 files changed, 51 insertions, 21 deletions
diff --git a/unittests/DebugInfo/MSF/CMakeLists.txt b/unittests/DebugInfo/MSF/CMakeLists.txt
index 20f3b2ab3dcd..0e48ab8f2f6b 100644
--- a/unittests/DebugInfo/MSF/CMakeLists.txt
+++ b/unittests/DebugInfo/MSF/CMakeLists.txt
@@ -2,14 +2,10 @@ set(LLVM_LINK_COMPONENTS
   DebugInfoMSF
   )
 
-set(DebugInfoMSFSources
+add_llvm_unittest(DebugInfoMSFTests
   MappedBlockStreamTest.cpp
   MSFBuilderTest.cpp
   MSFCommonTest.cpp
   )
 
-add_llvm_unittest(DebugInfoMSFTests
-  ${DebugInfoMSFSources}
-  )
-
 target_link_libraries(DebugInfoMSFTests PRIVATE LLVMTestingSupport)
diff --git a/unittests/DebugInfo/MSF/MSFBuilderTest.cpp b/unittests/DebugInfo/MSF/MSFBuilderTest.cpp
index a91ac8d443fe..16247951804c 100644
--- a/unittests/DebugInfo/MSF/MSFBuilderTest.cpp
+++ b/unittests/DebugInfo/MSF/MSFBuilderTest.cpp
@@ -112,7 +112,7 @@ TEST_F(MSFBuilderTest, TestAddStreamNoDirectoryBlockIncrease) {
   EXPECT_THAT_EXPECTED(ExpectedMsf, Succeeded());
   auto &Msf = *ExpectedMsf;
 
-  auto ExpectedL1 = Msf.build();
+  auto ExpectedL1 = Msf.generateLayout();
   EXPECT_THAT_EXPECTED(ExpectedL1, Succeeded());
   MSFLayout &L1 = *ExpectedL1;
 
@@ -129,7 +129,7 @@ TEST_F(MSFBuilderTest, TestAddStreamNoDirectoryBlockIncrease) {
   auto Blocks = Msf2.getStreamBlocks(0);
   EXPECT_EQ(1U, Blocks.size());
 
-  auto ExpectedL2 = Msf2.build();
+  auto ExpectedL2 = Msf2.generateLayout();
   EXPECT_THAT_EXPECTED(ExpectedL2, Succeeded());
   MSFLayout &L2 = *ExpectedL2;
   auto NewDirBlocks = L2.DirectoryBlocks;
@@ -149,7 +149,7 @@ TEST_F(MSFBuilderTest, TestAddStreamWithDirectoryBlockIncrease) {
   EXPECT_THAT_EXPECTED(Msf.addStream(4096 * 4096 / sizeof(uint32_t)),
                        Succeeded());
 
-  auto ExpectedL1 = Msf.build();
+  auto ExpectedL1 = Msf.generateLayout();
   EXPECT_THAT_EXPECTED(ExpectedL1, Succeeded());
   MSFLayout &L1 = *ExpectedL1;
   auto DirBlocks = L1.DirectoryBlocks;
@@ -289,7 +289,7 @@ TEST_F(MSFBuilderTest, BuildMsfLayout) {
   }
   ++ExpectedNumBlocks; // The directory itself should use 1 block
 
-  auto ExpectedLayout = Msf.build();
+  auto ExpectedLayout = Msf.generateLayout();
   EXPECT_THAT_EXPECTED(ExpectedLayout, Succeeded());
   MSFLayout &L = *ExpectedLayout;
   EXPECT_EQ(4096U, L.SB->BlockSize);
@@ -316,7 +316,7 @@ TEST_F(MSFBuilderTest, UseDirectoryBlockHint) {
   EXPECT_THAT_ERROR(Msf.setDirectoryBlocksHint({B + 1}), Succeeded());
   EXPECT_THAT_EXPECTED(Msf.addStream(2048, {B + 2}), Succeeded());
 
-  auto ExpectedLayout = Msf.build();
+  auto ExpectedLayout = Msf.generateLayout();
   EXPECT_THAT_EXPECTED(ExpectedLayout, Succeeded());
   MSFLayout &L = *ExpectedLayout;
   EXPECT_EQ(msf::getMinimumBlockCount() + 2, L.SB->NumBlocks);
@@ -338,7 +338,7 @@ TEST_F(MSFBuilderTest, DirectoryBlockHintInsufficient) {
   uint32_t Size = 4096 * 4096 / 4;
   EXPECT_THAT_EXPECTED(Msf.addStream(Size), Succeeded());
 
-  auto ExpectedLayout = Msf.build();
+  auto ExpectedLayout = Msf.generateLayout();
   EXPECT_THAT_EXPECTED(ExpectedLayout, Succeeded());
   MSFLayout &L = *ExpectedLayout;
   EXPECT_EQ(2U, L.DirectoryBlocks.size());
@@ -356,7 +356,7 @@ TEST_F(MSFBuilderTest, DirectoryBlockHintOverestimated) {
 
   ASSERT_THAT_EXPECTED(Msf.addStream(2048), Succeeded());
 
-  auto ExpectedLayout = Msf.build();
+  auto ExpectedLayout = Msf.generateLayout();
   ASSERT_THAT_EXPECTED(ExpectedLayout, Succeeded());
   MSFLayout &L = *ExpectedLayout;
   EXPECT_EQ(1U, L.DirectoryBlocks.size());
@@ -376,7 +376,7 @@ TEST_F(MSFBuilderTest, StreamDoesntUseFpmBlocks) {
   Expected<uint32_t> SN = Msf.addStream(StreamSize);
   ASSERT_THAT_EXPECTED(SN, Succeeded());
 
-  auto ExpectedLayout = Msf.build();
+  auto ExpectedLayout = Msf.generateLayout();
   ASSERT_THAT_EXPECTED(ExpectedLayout, Succeeded());
   MSFLayout &L = *ExpectedLayout;
   auto BlocksRef = L.StreamMap[*SN];
@@ -384,10 +384,9 @@ TEST_F(MSFBuilderTest, StreamDoesntUseFpmBlocks) {
   EXPECT_EQ(StreamSize, L.StreamSizes[*SN]);
 
   for (uint32_t I = 0; I <= 3; ++I) {
-    // Pages from the regular FPM are allocated, while pages from the alt fpm
-    // are free.
+    // Pages from both FPMs are always allocated.
+    EXPECT_FALSE(L.FreePageMap.test(2 + I * 4096));
     EXPECT_FALSE(L.FreePageMap.test(1 + I * 4096));
-    EXPECT_TRUE(L.FreePageMap.test(2 + I * 4096));
   }
 
   for (uint32_t I = 1; I <= 3; ++I) {
diff --git a/unittests/DebugInfo/MSF/MSFCommonTest.cpp b/unittests/DebugInfo/MSF/MSFCommonTest.cpp
index 144f5b113fb5..ee9ac75d15ce 100644
--- a/unittests/DebugInfo/MSF/MSFCommonTest.cpp
+++ b/unittests/DebugInfo/MSF/MSFCommonTest.cpp
@@ -46,12 +46,47 @@ TEST(MSFCommonTest, FpmIntervals) {
   EXPECT_EQ(1u, getNumFpmIntervals(L, true));
   SB.NumBlocks = SB.BlockSize;
   EXPECT_EQ(1u, getNumFpmIntervals(L, true));
-  SB.NumBlocks = SB.BlockSize + 1;
-  EXPECT_EQ(2u, getNumFpmIntervals(L, true));
   SB.NumBlocks = SB.BlockSize * 8;
   EXPECT_EQ(8u, getNumFpmIntervals(L, true));
-  SB.NumBlocks = SB.BlockSize * 8 + 1;
-  EXPECT_EQ(9u, getNumFpmIntervals(L, true));
+
+  // The FPM is going to look like this:
+  // |  0  |   1   |   2   |  ...  |  4096  |  4097  |  4098  | ... |
+  // | SB  |  FPM0 | FPM1  | Data  |  Data  |  FPM0  |  FPM1  | ... |
+  //
+  // So when there are up to 4097 blocks (last index 4096), the final blocks
+  // are data blocks.  When there are 4098 blocks (last index 4097), there is
+  // one terminating FPM block, and when there are 4099 blocks, there are two
+  // terminating FPM blocks.  Make sure all these cases are handled.
+
+  // With 4096 or 4097 blocks, the last block is a data block so we only have
+  // 1 interval.
+  for (uint32_t I : {4096, 4097}) {
+    // 1 FPM0 interval
+    EXPECT_EQ(1U, getNumFpmIntervals(4096, I, true, 1));
+    EXPECT_EQ(1U, getNumFpmIntervals(4096, I, false, 1));
+
+    // 1 FPM1 interval
+    EXPECT_EQ(1U, getNumFpmIntervals(4096, I, true, 2));
+    EXPECT_EQ(1U, getNumFpmIntervals(4096, I, false, 2));
+  }
+
+  // With 4098 blocks, the last block belongs to FPM0 so we should have 2 FPM0
+  // intervals.
+  EXPECT_EQ(2U, getNumFpmIntervals(4096, 4098, true, 1));
+  EXPECT_EQ(1U, getNumFpmIntervals(4096, 4098, false, 1));
+
+  // And 1 FPM1 interval.
+  EXPECT_EQ(1U, getNumFpmIntervals(4096, 4098, true, 2));
+  EXPECT_EQ(1U, getNumFpmIntervals(4096, 4098, false, 2));
+
+  // With 4099 blocks, the last block belongs to FPM1 so we should have 2
+  // FPM0 intervals.
+  EXPECT_EQ(2U, getNumFpmIntervals(4096, 4099, true, 1));
+  EXPECT_EQ(1U, getNumFpmIntervals(4096, 4099, false, 1));
+
+  // And 2 FPM1 intervals.
+  EXPECT_EQ(2U, getNumFpmIntervals(4096, 4099, true, 2));
+  EXPECT_EQ(1U, getNumFpmIntervals(4096, 4099, false, 2));
 }
 
 TEST(MSFCommonTest, FpmStreamLayout) {
@@ -95,7 +130,7 @@ TEST(MSFCommonTest, FpmStreamLayout) {
   // 2. When we are including unused FPM data, there should be one FPM block
   //    at every BlockSize interval in the file, even if entire FPM blocks are
   //    unused.
-  SB.NumBlocks = SB.BlockSize * 8 + 1;
+  SB.NumBlocks = SB.BlockSize * 8 + 3;
   SL = getFpmStreamLayout(L, true, false);
   EXPECT_EQ(SB.BlockSize * 9, SL.Length);
   EXPECT_EQ(9u, SL.Blocks.size());