[PATCH] D128108: [WIP][lld-macho] Add support for objc_msgSend stubs

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/SyntheticSections.cpp:731
+  std::string methname = sym->getName().drop_front(symbolPrefixLength).str();
+  methname.push_back('\0');
+  auto map = in.objcMethnameSection->getDuplicateStringOffsetMap();
----------------
isn't the string already null-terminated? why do we have to add another `\0`?


================
Comment at: lld/MachO/SyntheticSections.cpp:780-781
+      ConcatOutputSection::getOrCreateForInput(in.objcSelrefs);
+  // NOTE: This has to come before any selrefs sections from inputs since
+  // the offset from the start of the section is used in the stubs.
+  inputSections.insert(inputSections.begin(), in.objcSelrefs);
----------------
I'm not sure I understand how InputSection ordering matters... also, which offset are you referring to here?

Plus inserting like this means that we are copying the entire vector, which is pretty large... would prefer if we didn't have to do this


================
Comment at: lld/MachO/SyntheticSections.cpp:1634
+      if (trackOffsets)
+        duplicateStringOffsetMap.insert(
+            std::make_pair(isec->getStringRef(i), offsetInfo.outSecOff));
----------------
Why can't we just use `stringMapOffset` itself? Is it because it isn't available in the non-deduplicated `CStringSection`?

If that's the case, I wonder if we should just always have `__objc_methname` be deduplicated...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128108