[PATCH] D144385: [PDB] Error on too large stream directories

[Hans Wennborg via Phabricator via llvm-commits]

hans marked 2 inline comments as done.
hans added a comment.

In D144385#4145277 <https://reviews.llvm.org/D144385#4145277>, @aganea wrote:

> I'm seeing that Zach added quite a few tests for MSF <https://github.com/llvm/llvm-project/blob/main/llvm/unittests/DebugInfo/MSF/MSFBuilderTest.cpp>, 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.



================
Comment at: lld/COFF/PDB.cpp:1695
+          if (me.isPageOverflow())
+            error("try setting a larger /pdbpagesize");
+        });
----------------
aganea wrote:
> 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?
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.


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

https://reviews.llvm.org/D144385