[lld-macho] Fix parsing of $ld$ symbols with $ in them

[keith]

Summary

Swift symbols have dollar signs in them, so splitting on this
incorrectly resulted in a bunch of symbols just with the name _ being
considered, and extra info ignored. Currently we don't do anything with
these symbol names besides check that they're non-empty, so this doesn't
impact that, but I believe we need to start handling these:

https://github.com/llvm/llvm-project/issues/56074

[int3]

Could we have a test?

[keith]

I don't think we can until we're actually doing something in the case the symbol is non-empty, which will require some more changes

[int3]

Hm so is this actually fixing the handling of these symbols, or just raising an assertion error when we encounter them?

[keith]

Just raising an assertion in the case we hit some that we don't expect. I started looking at actually handling them but I don't think the changes will be very trivial, so I just filed https://github.com/llvm/llvm-project/issues/56074 until I have time

[thakis]

The rsplit() is correct. I don't think the assert adds a ton of value.

Not sure it makes sense to land this without the rest of previous + sym handling as it can't be tested before then.

I have half a patch for that locally, so maybe just wait a few days?

[int3]

@keith gotcha. I think the commit message / title is confusing since this isn't an actual fix

[keith]

In D128020#3591012, @thakis wrote:

The rsplit() is correct. I don't think the assert adds a ton of value.

no strong pref, i just figure this format shouldn't ever really change so it might be safer this way to avoid future breakages.

Not sure it makes sense to land this without the rest of previous + sym handling as it can't be tested before then.

I have half a patch for that locally, so maybe just wait a few days?

yea sounds fine, i just wanted to upload this asap in case someone did want to look into that issue before i did so they wouldn't have to re-fix this bug

lld/MachO/InputFiles.cpp

 #include "llvm/Support/Endian.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TarWriter.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/TextAPI/Architecture.h"
 #include "llvm/TextAPI/InterfaceFile.h"
 
+#include <cassert>
 #include <type_traits>
 
 using namespace llvm;
 using namespace llvm::MachO;
 using namespace llvm::support::endian;
 using namespace llvm::sys;
 using namespace lld;
 using namespace lld::macho;
void DylibFile::handleLDPreviousSymbol(StringRef name, StringRef originalName) {
   StringRef symbolName;
   StringRef rest;
 
   std::tie(installName, name) = name.split('$');
   std::tie(compatVersion, name) = name.split('$');
   std::tie(platformStr, name) = name.split('$');
   std::tie(startVersion, name) = name.split('$');
   std::tie(endVersion, name) = name.split('$');
-  std::tie(symbolName, rest) = name.split('$');
+  std::tie(symbolName, rest) = name.rsplit('$');
+  assert(rest.empty() && "$ld$ previous symbol has unexpected extra content");
   // TODO: ld64 contains some logic for non-empty symbolName as well.
   if (!symbolName.empty())
     return;
   unsigned platform;
   if (platformStr.getAsInteger(10, platform) ||
       platform != static_cast<unsigned>(config->platform()))
     return;