Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libcxx] Support providing symbol suffix through compiler define #122570

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

petrhosek
Copy link
Member

When -funique-internal-linkage-names option is enabled, Clang will append a unique suffix .uniq.<MD5 hash of the source file> to all internal symbols. Since we cannot detect when this option is enabled or compute the source file hash from within the source file itself, the unique suffix needs to be supplied by the build system. The _LIBCPP_SYMBOL_SUFFIX can be used for that purpose.

When -funique-internal-linkage-names option is enabled, Clang will
append a unique suffix `.uniq.<MD5 hash of the source file>` to all
internal symbols. Since we cannot detect when this option is enabled or
compute the source file hash from within the source file itself, the
unique suffix needs to be supplied by the build system. The
`_LIBCPP_SYMBOL_SUFFIX` can be used for that purpose.
@petrhosek petrhosek added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 11, 2025
@petrhosek petrhosek requested a review from a team as a code owner January 11, 2025 04:38
@llvmbot
Copy link
Member

llvmbot commented Jan 11, 2025

@llvm/pr-subscribers-libcxx

Author: Petr Hosek (petrhosek)

Changes

When -funique-internal-linkage-names option is enabled, Clang will append a unique suffix .uniq.&lt;MD5 hash of the source file&gt; to all internal symbols. Since we cannot detect when this option is enabled or compute the source file hash from within the source file itself, the unique suffix needs to be supplied by the build system. The _LIBCPP_SYMBOL_SUFFIX can be used for that purpose.


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

1 Files Affected:

  • (modified) libcxx/src/include/overridable_function.h (+12-3)
diff --git a/libcxx/src/include/overridable_function.h b/libcxx/src/include/overridable_function.h
index 7372e347831bb4..7eee4ecd06a98d 100644
--- a/libcxx/src/include/overridable_function.h
+++ b/libcxx/src/include/overridable_function.h
@@ -56,6 +56,15 @@
 // with this macro must be defined at global scope.
 //
 
+// When -funique-internal-linkage-names option is enabled, Clang will append a unique suffix
+// `.uniq.<MD5 hash of the source file>` to all internal symbols. Since we cannot detect when
+// this option is enabled or compute the source file hash from within the source file itself,
+// the unique suffix needs to be supplied by the build system. The `_LIBCPP_SYMBOL_SUFFIX` can
+// be used for that purpose.
+#ifndef _LIBCPP_SYMBOL_SUFFIX
+#define _LIBCPP_SYMBOL_SUFFIX
+#endif
+
 #if defined(_LIBCPP_OBJECT_FORMAT_MACHO)
 
 _LIBCPP_BEGIN_NAMESPACE_STD
@@ -68,8 +77,8 @@ _LIBCPP_END_NAMESPACE_STD
 #  define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
 #  define _LIBCPP_OVERRIDABLE_FUNCTION(symbol, type, name, arglist)                                                    \
     static __attribute__((used)) type symbol##_impl__ arglist __asm__("_" _LIBCPP_TOSTRING(symbol));                   \
-    __asm__(".globl _" _LIBCPP_TOSTRING(symbol));                                                                      \
-    __asm__(".weak_definition _" _LIBCPP_TOSTRING(symbol));                                                            \
+    __asm__(".globl _" _LIBCPP_TOSTRING(symbol) _LIBCPP_SYMBOL_SUFFIX);                                                \
+    __asm__(".weak_definition _" _LIBCPP_TOSTRING(symbol) _LIBCPP_SYMBOL_SUFFIX);                                      \
     extern __typeof(symbol##_impl__) name __attribute__((weak_import));                                                \
     _LIBCPP_BEGIN_NAMESPACE_STD                                                                                        \
     template <>                                                                                                        \
@@ -91,7 +100,7 @@ _LIBCPP_END_NAMESPACE_STD
 #  define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
 #  define _LIBCPP_OVERRIDABLE_FUNCTION(symbol, type, name, arglist)                                                    \
     static type symbol##_impl__ arglist __asm__(_LIBCPP_TOSTRING(symbol##_impl__));                                    \
-    [[gnu::weak, gnu::alias(_LIBCPP_TOSTRING(symbol##_impl__))]] type name arglist;                                    \
+    [[gnu::weak, gnu::alias(_LIBCPP_TOSTRING(symbol##_impl__) _LIBCPP_SYMBOL_SUFFIX)]] type name arglist;              \
     _LIBCPP_BEGIN_NAMESPACE_STD                                                                                        \
     template <>                                                                                                        \
     inline bool __is_function_overridden<static_cast<type(*) arglist>(name)>() {                                       \

Copy link

github-actions bot commented Jan 11, 2025

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 833a17489dd96f35df3a17ad231ada82acf38ef9 f1e0e27ba9db807ae9f1471323193852500be781 --extensions h -- libcxx/src/include/overridable_function.h
View the diff from clang-format here.
diff --git a/libcxx/src/include/overridable_function.h b/libcxx/src/include/overridable_function.h
index 9071140516..ed3a3062af 100644
--- a/libcxx/src/include/overridable_function.h
+++ b/libcxx/src/include/overridable_function.h
@@ -62,7 +62,7 @@
 // the unique suffix needs to be supplied by the build system. The `_LIBCPP_SYMBOL_SUFFIX` can
 // be used for that purpose.
 #ifndef _LIBCPP_SYMBOL_SUFFIX
-#define _LIBCPP_SYMBOL_SUFFIX
+#  define _LIBCPP_SYMBOL_SUFFIX
 #endif
 
 #if defined(_LIBCPP_OBJECT_FORMAT_MACHO)
@@ -100,7 +100,7 @@ _LIBCPP_END_NAMESPACE_STD
 #  define _LIBCPP_CAN_DETECT_OVERRIDDEN_FUNCTION 1
 #  define _LIBCPP_OVERRIDABLE_FUNCTION(symbol, type, name, arglist)                                                    \
     static type symbol##_impl__ arglist __asm__(_LIBCPP_TOSTRING(symbol##_impl__) _LIBCPP_SYMBOL_SUFFIX);              \
-    [[gnu::weak, gnu::alias(_LIBCPP_TOSTRING(symbol##_impl__) _LIBCPP_SYMBOL_SUFFIX)]] type name arglist;              \
+    [[ gnu::weak, gnu::alias(_LIBCPP_TOSTRING(symbol##_impl__) _LIBCPP_SYMBOL_SUFFIX) ]] type name arglist;            \
     _LIBCPP_BEGIN_NAMESPACE_STD                                                                                        \
     template <>                                                                                                        \
     inline bool __is_function_overridden<static_cast<type(*) arglist>(name)>() {                                       \

@@ -56,6 +56,15 @@
// with this macro must be defined at global scope.
//

// When -funique-internal-linkage-names option is enabled, Clang will append a unique suffix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like a generally missing feature that Clang doesn't provide any way to access that. I would prefer if we instead introduced either a macro that contains this suffix, or even better some kind of __builtin_symbol_name(name) that returns a string representing the symbol name for that entity while taking into account -funique-internal-linkage-names if needed.

That's something we could also use when defining operator new itself, i.e. we would not have to hardcode the mangled name in the macro. Similarly, we could refactor horrors like https://github.com/llvm/llvm-project/blob/main/libcxx/src/iostream.cpp#L22 where we hardcode mangled names, something @philnik777 was looking to do recently.

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants