[lld] [lld-macho] Implement symbol string deduplication (PR #123874)

[via llvm-commits]

https://github.com/alx32 created https://github.com/llvm/llvm-project/pull/123874

The symbol string table does not have deduplication. 
Here we add code to deduplicate the symbol string table. 
This has a rather large size impact (20-30%) on unstripped binaries (typically debug binaries) but no size impact on stripped binaries(typically release binaries). 

>From dc3fbc06bcbd42aa21d7348524be41c2afa1e9d0 Mon Sep 17 00:00:00 2001
From: Alex Borcan <alexborcan at fb.com>
Date: Fri, 17 Jan 2025 19:54:17 -0800
Subject: [PATCH] [lld-macho] Implement symbol string deduplication

---
 lld/MachO/Config.h              |  2 ++
 lld/MachO/Driver.cpp            |  2 ++
 lld/MachO/Options.td            |  5 +++++
 lld/MachO/SyntheticSections.cpp | 19 +++++++++++++++++--
 lld/MachO/SyntheticSections.h   |  1 +
 lld/test/MachO/cfstring-dedup.s |  8 ++++++++
 6 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index d41ca5382c692a2..edc1246acd3e42d 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -256,6 +256,8 @@ struct Configuration {
   llvm::MachO::PlatformType platform() const {
     return platformInfo.target.Platform;
   }
+
+  bool deduplicateSymbolStrings = true;
 };
 
 extern std::unique_ptr<Configuration> config;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 31630ba7d69de28..9c63ab0739a8a5c 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -1806,6 +1806,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
   config->keepICFStabs = args.hasArg(OPT_keep_icf_stabs);
   config->dedupStrings =
       args.hasFlag(OPT_deduplicate_strings, OPT_no_deduplicate_strings, true);
+  config->deduplicateSymbolStrings =
+      !args.hasArg(OPT_no_deduplicate_symbol_strings);
   config->deadStripDuplicates = args.hasArg(OPT_dead_strip_duplicates);
   config->warnDylibInstallName = args.hasFlag(
       OPT_warn_dylib_install_name, OPT_no_warn_dylib_install_name, false);
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 4c89f96c3ebaadf..9001e85582c1244 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -1476,3 +1476,8 @@ def no_warn_duplicate_libraries : Flag<["-"], "no_warn_duplicate_libraries">,
     HelpText<"Do not warn if the input contains duplicate library options.">,
     Flags<[HelpHidden]>,
     Group<grp_ignored_silently>;
+
+// Add this with the other flags in the rare options group
+def no_deduplicate_symbol_strings : Flag<["-"], "no-deduplicate-symbol-strings">,
+    HelpText<"Do not deduplicate strings in the symbol string table. Might result in larger binaries but slightly faster link times.">,
+    Group<grp_rare>;
diff --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 417b7cf93efa7db..313860128bcafb7 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -1540,9 +1540,24 @@ StringTableSection::StringTableSection()
     : LinkEditSection(segment_names::linkEdit, section_names::stringTable) {}
 
 uint32_t StringTableSection::addString(StringRef str) {
+  // If deduplication is disabled, just add the string
+  if (!config->deduplicateSymbolStrings) {
+    uint32_t strx = size;
+    strings.push_back(str);
+    size += str.size() + 1; // +1 for null terminator
+    return strx;
+  }
+
+  // Deduplicate strings
+  llvm::CachedHashStringRef hashedStr(str);
+  auto it = stringMap.find(hashedStr);
+  if (it != stringMap.end())
+    return it->second;
+
   uint32_t strx = size;
-  strings.push_back(str); // TODO: consider deduplicating strings
-  size += str.size() + 1; // account for null terminator
+  stringMap[hashedStr] = strx;
+  strings.push_back(str);
+  size += str.size() + 1;
   return strx;
 }
 
diff --git a/lld/MachO/SyntheticSections.h b/lld/MachO/SyntheticSections.h
index af99f22788d6e99..5796b0790c83a09 100644
--- a/lld/MachO/SyntheticSections.h
+++ b/lld/MachO/SyntheticSections.h
@@ -447,6 +447,7 @@ class StringTableSection final : public LinkEditSection {
   // match its behavior here since some tools depend on it.
   // Consequently, the empty string will be at index 1, not zero.
   std::vector<StringRef> strings{" "};
+  llvm::DenseMap<llvm::CachedHashStringRef, uint32_t> stringMap;
   size_t size = 2;
 };
 
diff --git a/lld/test/MachO/cfstring-dedup.s b/lld/test/MachO/cfstring-dedup.s
index fb121cde3e9585c..7e1772600b427f2 100644
--- a/lld/test/MachO/cfstring-dedup.s
+++ b/lld/test/MachO/cfstring-dedup.s
@@ -7,6 +7,14 @@
 # RUN: %lld -dylib -framework CoreFoundation %t/foo1.o %t/foo2.o -o %t/foo
 # RUN: llvm-objdump --no-print-imm-hex --macho --rebase --bind --syms -d %t/foo | FileCheck %s --check-prefix=LITERALS
 
+# Check that string deduplication for symbol names is working
+# RUN: %lld -dylib -framework CoreFoundation %t/foo1.o %t/foo2.o -o %t/foo_no_dedup -no-deduplicate-symbol-strings
+# RUN: count1=$((`llvm-strings %t/foo | grep _named_cfstring | wc -l`))
+# RUN: test $count1 -eq 1
+# RUN: count2=$((`llvm-strings %t/foo_no_dedup | grep _named_cfstring | wc -l`))
+# RUN: test $count2 -eq 2
+
+
 # CHECK:       (__TEXT,__text) section
 # CHECK-NEXT:  _foo1:
 # CHECK-NEXT:  _foo2: