[PATCH] D104919: [lld-macho] Preserve alignment for non-deduplicated cstrings

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Nice!

Add "Fixes PR50637" to the commit message, since it fixes llvm.org/PR50637

I'd word the commit message as "__cstring is special in that it is split at null bytes instead of at symbol boundaries with `.subsections_via_symbols`" or something like that.

Please mention in the patch description why you're removing subsections-section-relocs.s.



================
Comment at: lld/MachO/SyntheticSections.cpp:1191
+      // TODO: Is it worth introducing another code path to not bother with the
+      // hash?
+      CachedHashStringRef string = isec->getCachedHashStringRef(i);
----------------
I think that'd be nice. You just need

```
% git diff
diff --git a/lld/MachO/InputSection.h b/lld/MachO/InputSection.h
index 3dd31d27be91..413b7aed70e7 100644
--- a/lld/MachO/InputSection.h
+++ b/lld/MachO/InputSection.h
@@ -183,11 +183,15 @@ public:
   // Returns i'th piece as a CachedHashStringRef. This function is very hot when
   // string merging is enabled, so we want to inline.
   LLVM_ATTRIBUTE_ALWAYS_INLINE
-  llvm::CachedHashStringRef getCachedHashStringRef(size_t i) const {
+  StringRef getStringRef(size_t i) const {
     size_t begin = pieces[i].inSecOff;
     size_t end =
         (pieces.size() - 1 == i) ? data.size() : pieces[i + 1].inSecOff;
-    return {toStringRef(data.slice(begin, end - begin)), pieces[i].hash};
+    return toStringRef(data.slice(begin, end - begin));
+  }
+  LLVM_ATTRIBUTE_ALWAYS_INLINE
+  llvm::CachedHashStringRef getCachedHashStringRef(size_t i) const {
+    return {getStringRef(i), pieces[i].hash};
   }

   static bool classof(const InputSection *isec) {
```

and then you could call `getStringRef()` here, right?


================
Comment at: lld/MachO/SyntheticSections.h:530
+// TODO: Bad name, but I don't have any better ideas
+class VanillaCStringSection final : public CStringSection {
+public:
----------------
Maybe this should just be the default impl in CStringSection and DedupStringSection overrides it?


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

https://reviews.llvm.org/D104919