[PDB] Error on too large stream directories

[hans]

Summary

We hit this in Chromium builds where the PDB file was just under 4GB, but the stream directory was actually too large to be correctly represented.

llvm-pdbutil would error about this in llvm::msf::validateSuperBlock, but lld should not write such PDB files in the first place.

[aganea]

In addition to these changes, do you think you can add a more descriptive error at a higher level in LLD, maybe in PDBLinker::commit(). It is not clear from the message what the corrective measure is. I suppose adding /pdbpagesize: with a higher block size would fix the problem, but this isn't obvious to everyone.

[hans]

In D144385#4139641, @aganea wrote:

In addition to these changes, do you think you can add a more descriptive error at a higher level in LLD, maybe in PDBLinker::commit(). It is not clear from the message what the corrective measure is. I suppose adding /pdbpagesize: with a higher block size would fix the problem, but this isn't obvious to everyone.

I've updated the patch. Please take another look.

[aganea]

Thanks for the changes!

I'm seeing that Zach added quite a few tests for MSF, not sure if you had the time/if it's worth crafting a test for this.

Otherwise, looks good.

lld/COFF/PDB.cpp

1695

Just a nit: How does the errors look like in stderr? I wonder if we shouldn't use joinErrors to have both errors contiguous on a single line?

llvm/lib/DebugInfo/MSF/MSFError.cpp

47

s/Stream/stream/ to be consistent with the other messages.

[hans]

In D144385#4145277, @aganea wrote:

I'm seeing that Zach added quite a few tests for MSF, not sure if you had the time/if it's worth crafting a test for this.

Thanks! I had a look, and it doesn't seem that those hit MSFBuilder::commit, and creating one is probably not worth the effort for this check.

lld/COFF/PDB.cpp

1695

It currently looks like this:

lld-link: error: PDB Stream directory too large. The directory block map (4148 bytes) doesn't fit in a block (4096 bytes)
lld-link: error: try using a larger /pdbpagesize:
lld-link: error: failed to write PDB file ./chrome.dll.pdb

I see your point about joining them, but I think with the separate line it's easier to read.

lld/COFF/PDB.cpp

 #include "llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h"
 #include "llvm/DebugInfo/CodeView/RecordName.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecordHelpers.h"
 #include "llvm/DebugInfo/CodeView/SymbolSerializer.h"
 #include "llvm/DebugInfo/CodeView/TypeIndexDiscovery.h"
 #include "llvm/DebugInfo/MSF/MSFBuilder.h"
 #include "llvm/DebugInfo/MSF/MSFCommon.h"
+#include "llvm/DebugInfo/MSF/MSFError.h"
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.h"
 #include "llvm/DebugInfo/PDB/Native/DbiStream.h"
 #include "llvm/DebugInfo/PDB/Native/DbiStreamBuilder.h"
 #include "llvm/DebugInfo/PDB/Native/GSIStreamBuilder.h"
 #include "llvm/DebugInfo/PDB/Native/InfoStream.h"
 #include "llvm/DebugInfo/PDB/Native/InfoStreamBuilder.h"
 #include "llvm/DebugInfo/PDB/Native/NativeSession.h"
exitOnErr(
       dbiBuilder.addDbgStream(pdb::DbgHeaderType::SectionHdr, sectionTable));
 }
 
 void PDBLinker::commit(codeview::GUID *guid) {
   // Print an error and continue if PDB writing fails. This is done mainly so
   // the user can see the output of /time and /summary, which is very helpful
   // when trying to figure out why a PDB file is too large.
   if (Error e = builder.commit(ctx.config.pdbPath, guid)) {
+    e = handleErrors(std::move(e),
+        [](const llvm::msf::MSFError &me) {
+          error(me.message());
+          if (me.isPageOverflow())
+            error("try setting a larger /pdbpagesize");
+        });
     checkError(std::move(e));
     error("failed to write PDB file " + Twine(ctx.config.pdbPath));
   }
 }
 
 static uint32_t getSecrelReloc(llvm::COFF::MachineTypes machine) {
   switch (machine) {
   case AMD64:

llvm/include/llvm/DebugInfo/MSF/MSFError.h

 
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace msf {
 enum class msf_error_code {
   unspecified = 1,
   insufficient_buffer,
+  not_writable,
+  no_stream,
+  invalid_format,
+  block_in_use,
   size_overflow_4096,
   size_overflow_8192,
   size_overflow_16384,
   size_overflow_32768,
-  not_writable,
-  no_stream,
-  invalid_format,
-  block_in_use
+  stream_directory_overflow,
 };
 } // namespace msf
 } // namespace llvm
 
 namespace std {
 template <>
 struct is_error_code_enum<llvm::msf::msf_error_code> : std::true_type {};
 } // namespace std
 
 namespace llvm {
 namespace msf {
 const std::error_category &MSFErrCategory();
 
 inline std::error_code make_error_code(msf_error_code E) {
   return std::error_code(static_cast<int>(E), MSFErrCategory());
 }
 
 /// Base class for errors originating when parsing raw PDB files
 class MSFError : public ErrorInfo<MSFError, StringError> {
 public:
   using ErrorInfo<MSFError, StringError>::ErrorInfo; // inherit constructors
   MSFError(const Twine &S) : ErrorInfo(S, msf_error_code::unspecified) {}
+
+  bool isPageOverflow() const {
+    switch (static_cast<msf_error_code>(convertToErrorCode().value())) {
+    case msf_error_code::unspecified:
+    case msf_error_code::insufficient_buffer:
+    case msf_error_code::not_writable:
+    case msf_error_code::no_stream:
+    case msf_error_code::invalid_format:
+    case msf_error_code::block_in_use:
+      return false;
+    case msf_error_code::size_overflow_4096:
+    case msf_error_code::size_overflow_8192:
+    case msf_error_code::size_overflow_16384:
+    case msf_error_code::size_overflow_32768:
+    case msf_error_code::stream_directory_overflow:
+      return true;
+    }
+  }
+
   static char ID;
 };
 } // namespace msf
 } // namespace llvm
 
 #endif // LLVM_DEBUGINFO_MSF_MSFERROR_H

llvm/lib/DebugInfo/MSF/MSFBuilder.cpp

if (FileSize > getMaxFileSizeFromBlockSize(Layout.SB->BlockSize)) {
     }(Layout.SB->BlockSize);
 
     return make_error<MSFError>(
         error_code,
         formatv("File size {0,1:N} too large for current PDB page size {1}",
                 FileSize, Layout.SB->BlockSize));
   }
 
+  uint64_t NumDirectoryBlocks =
+      bytesToBlocks(Layout.SB->NumDirectoryBytes, Layout.SB->BlockSize);
+  uint64_t DirectoryBlockMapSize =
+      NumDirectoryBlocks * sizeof(support::ulittle32_t);
+  if (DirectoryBlockMapSize > Layout.SB->BlockSize) {
+    return make_error<MSFError>(msf_error_code::stream_directory_overflow,
+                                formatv("The directory block map ({0} bytes) "
+                                        "doesn't fit in a block ({1} bytes)",
+                                        DirectoryBlockMapSize,
+                                        Layout.SB->BlockSize));
+  }
+
   auto OutFileOrError = FileOutputBuffer::create(Path, FileSize);
   if (auto EC = OutFileOrError.takeError())
     return std::move(EC);
 
   FileBufferByteStream Buffer(std::move(*OutFileOrError),
                               llvm::support::little);
   BinaryStreamWriter Writer(Buffer);
 

llvm/lib/DebugInfo/MSF/MSFError.cpp

std::string message(int Condition) const override {
     case msf_error_code::not_writable:
       return "The specified stream is not writable.";
     case msf_error_code::no_stream:
       return "The specified stream does not exist.";
     case msf_error_code::invalid_format:
       return "The data is in an unexpected format.";
     case msf_error_code::block_in_use:
       return "The block is already in use.";
+    case msf_error_code::stream_directory_overflow:
+      return "PDB stream directory too large.";
     }
     llvm_unreachable("Unrecognized msf_error_code");
   }
 };
 } // namespace
 
 const std::error_category &llvm::msf::MSFErrCategory() {
   static MSFErrorCategory MSFCategory;
   return MSFCategory;
 }
 
 char MSFError::ID;