Skip to content

Conversation

@michaelrj-google
Copy link
Contributor

Downstream ther'es a user that needs the syscall wrappers to be weak. I
intend to set up a proper mechanism for just listing which functions
should be weak eventually, but for now this is necessary.

Downstream ther'es a user that needs the syscall wrappers to be weak. I
intend to set up a proper mechanism for just listing which functions
should be weak eventually, but for now this is necessary.
@llvmbot llvmbot added libc bazel "Peripheral" support tier build system: utils/bazel labels Nov 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 5, 2024

@llvm/pr-subscribers-libc

Author: Michael Jones (michaelrj-google)

Changes

Downstream ther'es a user that needs the syscall wrappers to be weak. I
intend to set up a proper mechanism for just listing which functions
should be weak eventually, but for now this is necessary.


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

1 Files Affected:

  • (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+8)
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index f6648c9bffa730..089592bc0bc17c 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -4650,6 +4650,7 @@ libc_function(
     name = "socket",
     srcs = ["src/sys/socket/linux/socket.cpp"],
     hdrs = ["src/sys/socket/socket.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4661,6 +4662,7 @@ libc_function(
     name = "socketpair",
     srcs = ["src/sys/socket/linux/socketpair.cpp"],
     hdrs = ["src/sys/socket/socketpair.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4672,6 +4674,7 @@ libc_function(
     name = "send",
     srcs = ["src/sys/socket/linux/send.cpp"],
     hdrs = ["src/sys/socket/send.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4686,6 +4689,7 @@ libc_function(
     name = "sendto",
     srcs = ["src/sys/socket/linux/sendto.cpp"],
     hdrs = ["src/sys/socket/sendto.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4700,6 +4704,7 @@ libc_function(
     name = "sendmsg",
     srcs = ["src/sys/socket/linux/sendmsg.cpp"],
     hdrs = ["src/sys/socket/sendmsg.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4713,6 +4718,7 @@ libc_function(
     name = "recv",
     srcs = ["src/sys/socket/linux/recv.cpp"],
     hdrs = ["src/sys/socket/recv.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4727,6 +4733,7 @@ libc_function(
     name = "recvfrom",
     srcs = ["src/sys/socket/linux/recvfrom.cpp"],
     hdrs = ["src/sys/socket/recvfrom.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",
@@ -4741,6 +4748,7 @@ libc_function(
     name = "recvmsg",
     srcs = ["src/sys/socket/linux/recvmsg.cpp"],
     hdrs = ["src/sys/socket/recvmsg.h"],
+    weak = True,
     deps = [
         ":__support_common",
         ":__support_osutil_syscall",

@vonosmas
Copy link
Contributor

vonosmas commented Nov 6, 2024

We also define weak = True for various non-syscall wrappers, such as memset, memcmp, etc.

Is there any reason why we shouldn't just define all libc functions as "weak"? Would this mean that in static link mode we allow the user to provide their own implementations / wrappers / proxies of libc functions? (they would also be able to call real implementations by calling a namespace-d version, I think?)

@nickdesaulniers
Copy link
Member

Is there any reason why we shouldn't just define all libc functions as "weak"?

Note that declaring a function as having weak linkage is an optimization barrier that inhibits inter-proceedural optimizations such as inlining and constant propagation. If downstream users ever hope to LTO llvm-libc, then marking everything as weak will prevent such optimizations.

@vonosmas
Copy link
Contributor

vonosmas commented Nov 6, 2024

Right, I agree with Nick's comment that we probably don't want to blanket-define all functions as weak. I do think we need to let Bazel build downstream users decide which functions should be made weak, however -- but let's postpone this update to libc_function logic to future changes. OK with this for now.

@michaelrj-google michaelrj-google merged commit aae5a38 into llvm:main Nov 6, 2024
9 checks passed
@michaelrj-google michaelrj-google deleted the libcWeakSocket branch November 6, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bazel "Peripheral" support tier build system: utils/bazel libc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants