Apart from the nits already pointed out by @jhenderson, LGTM.

This change broke one of our downstream tests and the issue can be reproduced with the following test:

# REQUIRES: x86

Update to clarify the purpose of the initial call to writeSymbolTableHeader.

In D89789#2345015, @MaskRay wrote:

Putting together, I think this option cannot be accepted.
On the good side, the place you modify is very stable and you usually will not get merge conflict.

In D89891#2345374, @MaskRay wrote:

I understand that a test may be problematic because it requires 4+GiB output. Do you have a script to craft such a test case so that people working on llvm-ar can run it on demand? The test can have UNSUPPORTED: *.

Updated to address review comments and updated to add better testing. Whilst adding this testing, found another bug in the detection code; it was also not accounting for the size of the file signature!

Ping.

In D88830#2313269, @MaskRay wrote:

The description can be shortened a bit. The important part is that variableEq returns false if two relocations reference a pair of aliased symbols in an ineligible section.

Address review comments.

Abandoning due to lack of interest and in favour of D88830 which requires these changes.

I may be missing something about how the ICF code works, but whilst looking at a separate ICF performance issue, I noticed these oddities in the code. I haven't actually seen any issues myself but reading through the code, it feels like there could be issues. Although I suspect they would be highly unlikely or perhaps not possible in actual practice.

Update for review suggestion.

Update for review comments.

Update to address review comments.

In D87469#2267576, @grimar wrote:

I think this is fine approach in general. I wonder if it should/could be splitted into 2 patches though:
one for InputSectionDescription and one for SectionPattern?

Update to address review comments and suggestions.

Updated to address review comments and suggestions.

In D87468#2266310, @MaskRay wrote:

Can you demonstrate how the old code has overhead related to the input?

In D87468#2266228, @MaskRay wrote:

GlobPattern::matchOne has a fast path for '*'. Isn't it fast enough?

This change requires D87468.

This is a prerequisite of D87469.

In D87245#2259958, @MaskRay wrote:

Thanks for noting this issue! Were you profiling ld.lld?

Address review comments.

LGTM with that last one change.

That test is for widenPath and it does cover a similar situation (I know because I wrote it). The code you have changed relates to directory_iterator_construct which does not have coverage of the situation where the UTF16 length is less than the UTF8 length, i.e. the issue that you are fixing.

In D83321#2140620, @danix800 wrote:

Full context attached @andrewng .

The usual practice is to include the entire context in the diff. However, the change itself LGTM.

In D77750#1980538, @thakis wrote:

Can you take a look, please?

Add comment to explain the Windows deficiencies (one of many)...

Updated to address review comments.

Updated to address review comments.

In D77184#1960437, @thakis wrote:

grimar, andrewng: You both have checkout and build on different drives too, yes?

The following patch fixes my issues on Windows, but I haven't tested that it doesn't break anything else:

Ping.

Ping.

Updated to address review comments and suggestions.