[memprof] Export __memprof_default_options_str on Darwin

[ellishg]

The -memprof-runtime-default-options LLVM flag introduced in #118874 creates the __memprof_default_options_str symbol with WeakAnyLinkage on Darwin.

https://github.com/ellishg/llvm-project/blob/fa0202169af23419c4bcbf66eabd1beb6b6e8e34/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp#L573-L576

This ensures Darwin passes -exported_symbol ___memprof_default_options_str to the linker so that the runtime library has visibility into this symbol.

This will replace the earlier PR #128615

[llvmbot]

@llvm/pr-subscribers-pgo
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Ellis Hoag (ellishg)

Changes

The -memprof-runtime-default-options LLVM flag introduced in #118874 creates the __memprof_default_options_str symbol with WeakAnyLinkage on Darwin.

https://github.com/ellishg/llvm-project/blob/fa0202169af23419c4bcbf66eabd1beb6b6e8e34/llvm/lib/Transforms/Instrumentation/MemProfiler.cpp#L573-L576

This ensures Darwin passes -exported_symbol ___memprof_default_options_str to the linker so that the runtime library has visibility into this symbol.

This will replace the earlier PR #128615

---

Full diff: https://github.com/llvm/llvm-project/pull/128920.diff

2 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+4)
• (modified) clang/test/Driver/fmemprof.cpp (+8)

diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 75f126965e0ac..a2aeef4c4c475 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1617,6 +1617,10 @@ void DarwinClang::AddLinkRuntimeLibArgs(const ArgList &Args,
     }
   }
 
+  if (Sanitize.needsMemProfRt())
+    if (hasExportSymbolDirective(Args))
+      addExportedSymbol(CmdArgs, "___memprof_default_options_str");
+
   const XRayArgs &XRay = getXRayArgs();
   if (XRay.needsXRayRt()) {
     AddLinkRuntimeLib(Args, CmdArgs, "xray");
diff --git a/clang/test/Driver/fmemprof.cpp b/clang/test/Driver/fmemprof.cpp
index 5165c4452fd57..b464320e58a94 100644
--- a/clang/test/Driver/fmemprof.cpp
+++ b/clang/test/Driver/fmemprof.cpp
@@ -17,3 +17,11 @@
 
 // RUN: not %clangxx --target=x86_64-linux-gnu -fprofile-generate -fmemory-profile-use=foo %s -### 2>&1 | FileCheck %s --check-prefix=CONFLICTWITHPGOINSTR
 // CONFLICTWITHPGOINSTR: error: invalid argument '-fmemory-profile-use=foo' not allowed with '-fprofile-generate'
+
+// Test that we export the __memprof_default_options_str on Darwin because it has WeakAnyLinkage
+// RUN: %clangxx --target=arm64-apple-ios -fmemory-profile %s -### 2>&1 | FileCheck %s -check-prefix=EXPORT-BASE --implicit-check-not=exported_symbol
+// RUN: %clangxx --target=x86_64-linux-gnu -fmemory-profile %s -### 2>&1 | FileCheck %s -check-prefix=EXPORT-BASE --implicit-check-not=exported_symbol
+// RUN: %clangxx --target=arm64-apple-ios -fmemory-profile -exported_symbols_list /dev/null %s -### 2>&1 | FileCheck %s --check-prefixes=EXPORT-BASE,EXPORT
+// FIXME: Darwin needs to link in the runtime, then we can use the regular CHECK prefix
+// EXPORT-BASE: "-cc1" {{.*}} "-fmemory-profile"
+// EXPORT: "-exported_symbol" "___memprof_default_options_str"

[ellishg]

@teresajohnson I know you asked for this to be refactored into a constexpr string in a header, but I don't think Darwin.cpp and MemProfiler.cpp share any headers in common, so I don't know where I would put it. I don't think I can add MemProfiler.h to Darwin.cpp because it doesn't link in the Instrumentation component

[teresajohnson]

Looks like this file already includes a header from llvm/ProfileData, so maybe put it into MemProf.h in that directory?

I just noticed that the symbol name here has an extra leading underscore (noticed because I couldn't find it searching the code). Which would be avoided by using a common variable. But also makes me wonder how this is working?

[ellishg]

Yes, Darwin symbols are prefixed with _, so this is correct.

[ellishg]

@teresajohnson is this ok? I feel like it may be more trouble than it's worth to refactor out this string

[teresajohnson]

Can you add a comment about the added _ prefix? Also, I think it would be nice to just move the name to ProfileData/MemProf.h. If I look at the ProfileData/InstrProf.h included here for example, it contains helper functions to get runtime function names, so this would seem like a good equivalent place for memprof.

[ellishg]

I plan to follow this up soon. It should be a simple PR. CC @SharonXSharon

[teresajohnson]

lgtm