A customer reported that their LLD-linked binary failed to open because of a gap between COFF Groups in the .rdata section. As I investigated that and looked at the binary generally I found a few more issues that seem unique to LLD and its generally-weaker-than-MSVC PDB support.
Briefly summarize what changed
I've given up on trying to get LLD to have only small gaps between COFF Groups. The summation of SymTagCoffGroup symbols in the PDB and the PE directories just isn't filling all the space. In this case, the gap was due to a bunch of strings (symbols with the demangled name "`string'"). MSVC puts these into COFF Groups, but LLD does not and detecting all the strings and synthesizing COFF Groups would be extremely expensive for performance, so instead I suppress the COFF Group gap analysis for LLD-linked binaries.
Some functions in a binary can have 0 RVA and 0 length, such as if COMDAT folding collapsed them, or if they ended up being dead code and removed by things like /OPT:REF. With MSVC PDBs, trying to enumerate the separated blocks of these symbols gives back an IDiaEnumSymbols that is non-null but just has nothing in it. But with LLD it seems I can get back a null enumerator, so just tolerate this as if the enumerator were non-null-but-empty.
When using LANGUAGE_NEUTRAL coupled with SUBLANG_DEFAULT the rsrc language ID can be 0x400. This should be special-cased just like 0 (LANGUAGE_NEUTRAL / SUBLANG_NEUTRAL)
LLD seems to not always emit correct symbols for strings. I found an example where two strings were contiguous in the binary but only represented as one IDiaSymbol with a length that encompassed both. That's not great because it means we get "string 1 contents\0string 2 contents" and when feeding that to IsTextUnicode it says it's unicode because it has embedded nulls, so then marshaling that as unicode creates garbage. Instead, I'll skip the check for embedded nulls and allow those to still be ANSI - this probably fixes a variety of weird cases where SizeBench has historically thought something was Unicode and displayed random international characters instead of ANSI that I've seen over the years and struggled to debug enough.
How was the change tested?
All existing tests pass.
Added a test for the LANGUAGE_NEUTRAL / SUBLANG_DEFAULT case
I could not get a minimal repro of the "string without a symbol" issue so I manually verified it with the customer binary, but can't figure out how to make regression coverage.
I also could not get a minimal repro of the "null DIA enumerator" so again manually verified with customer binary.
PR Checklist
[x] Contributor License Agreement (CLA) has been signed. If not, go here and sign the CLA
[x] Changes have been validated
[ ] ~Documentation updated. Please add or update any docs in the repo as necessary.~
Why is this change being made?
A customer reported that their LLD-linked binary failed to open because of a gap between COFF Groups in the
.rdatasection. As I investigated that and looked at the binary generally I found a few more issues that seem unique to LLD and its generally-weaker-than-MSVC PDB support.Briefly summarize what changed
SymTagCoffGroupsymbols in the PDB and the PE directories just isn't filling all the space. In this case, the gap was due to a bunch of strings (symbols with the demangled name "`string'"). MSVC puts these into COFF Groups, but LLD does not and detecting all the strings and synthesizing COFF Groups would be extremely expensive for performance, so instead I suppress the COFF Group gap analysis for LLD-linked binaries./OPT:REF. With MSVC PDBs, trying to enumerate the separated blocks of these symbols gives back anIDiaEnumSymbolsthat is non-null but just has nothing in it. But with LLD it seems I can get back a null enumerator, so just tolerate this as if the enumerator were non-null-but-empty.LANGUAGE_NEUTRALcoupled withSUBLANG_DEFAULTthe rsrc language ID can be 0x400. This should be special-cased just like 0 (LANGUAGE_NEUTRAL / SUBLANG_NEUTRAL)IDiaSymbolwith a length that encompassed both. That's not great because it means we get "string 1 contents\0string 2 contents" and when feeding that toIsTextUnicodeit says it's unicode because it has embedded nulls, so then marshaling that as unicode creates garbage. Instead, I'll skip the check for embedded nulls and allow those to still be ANSI - this probably fixes a variety of weird cases where SizeBench has historically thought something was Unicode and displayed random international characters instead of ANSI that I've seen over the years and struggled to debug enough.How was the change tested?
LANGUAGE_NEUTRAL / SUBLANG_DEFAULTcasePR Checklist