[LLD] [COFF] Avoid doing repeated fuzzy symbol lookup for each iteration. NFC.

[mstorsjo]

Summary

This is run every time around in the main linker loop. Once a match
has been found, stop trying to rematch such a symbol.

Not sure if this has any actual measurable performance impact though
(SymbolTable::findMangle() iterates over the whole symbol table for
each call and does fuzzy matching on top of that) but this makes the
code more reassuring to read at least. (This is in practice run for def
files listing undecorated stdcall functions to be exported.)

[rnk]

lgtm

[thakis]

Chances are this causes all the red windows boxes on https://chromium-review.googlesource.com/c/chromium/src/+/2976785

[mstorsjo]

In D104529#2833572, @thakis wrote:

Chances are this causes all the red windows boxes on https://chromium-review.googlesource.com/c/chromium/src/+/2976785

Sounds plausible, feel free to revert once you’ve pinpointed it. If you manage to do that, it’d be great to add a test case showing how it broke things too.

I don’t have any direct need for this change per se, but I’m curious about how it broke anything.

[rnk]

lld/COFF/Driver.cpp

564

I think I see the issue: suppose there are two rounds of symbol resolution. This function will be called twice, and on the second call, it returns a different result to the caller. The result of this function is used to populate Export.symbolName, which is used to produce an import library, and that's what changes.

[mstorsjo]

lld/COFF/Driver.cpp

564

Right, if there are two possible matches for the fuzzy search, one to be more preferred than the other one, but the less preferred one is resolved first, then it would indeed break.

I suppose that can be considered intentional behaviour, so yeah, this should indeed be left reverted then. A testcase showing off this mechanism would be nice to have though.

[rnk]

lld/COFF/Driver.cpp

564

Yep, I added a test in rG5bcbc7ee526cea839f, it just took a bit of effort to set up an archive to get a second iteration of symbol resolution.

[mstorsjo]

lld/COFF/Driver.cpp

564

Awesome, thanks! Yeah I can imagine that requires a bit of fiddling.

lld/COFF/Driver.cpp

if (!b->isGCRoot) {
     config->gcroot.push_back(b);
   }
   return b;
 }
 
 StringRef LinkerDriver::mangleMaybe(Symbol *s) {
   // If the plain symbol name has already been resolved, do nothing.
   Undefined *unmangled = dyn_cast<Undefined>(s);
-  if (!unmangled)
+  if (!unmangled || unmangled->getWeakAlias())
     return "";
 
   // Otherwise, see if a similar, mangled symbol exists in the symbol table.
   Symbol *mangled = symtab->findMangle(unmangled->getName());
   if (!mangled)
     return "";
 
   // If we find a similar mangled symbol, make this an alias to it and return
   // its name.
   log(unmangled->getName() + " aliased to " + mangled->getName());
   unmangled->weakAlias = symtab->addUndefined(mangled->getName());
   return mangled->getName();
 }
 
 // Windows specific -- find default entry point name.
 //
 // There are four different entry point functions for Windows executables,
 // each of which corresponds to a user-defined "main" function. This function
 // infers an entry point from a user-defined "main" function.
 StringRef LinkerDriver::findDefaultEntry() {