Skip to content

[compiler-rt] Don't handle Linux-specific shmctl commands in sanitizer #143116

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jrtc27
Copy link
Collaborator

@jrtc27 jrtc27 commented Jun 6, 2025

Despite being defined in the system headers, these commands are not in
fact part of the FreeBSD system call interface. They exist solely for
the Linuxulator, i.e. running Linux binaries on FreeBSD, and any attempt
to use them from a FreeBSD binary will return EINVAL. The fact we needed
to define _KERNEL (which, as the name implies, means we are compiling
the kernel) to even get the definition of shminfo should have been a
strong indicator that IPC_INFO at least was not a userspace interface.

@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2025

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Jessica Clarke (jrtc27)

Changes

Defining _KERNEL is a hack that can cause all kinds of breakage if
you're not careful, since the actual environment is not a kernel build,
and so should be avoided. Since FreeBSD 12 (MFC'ed for 11.3) you can
define _WANT_SYSVSHM_INTERNALS to expose shminfo (and various other
things) to userspace, so use that instead.


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

1 Files Affected:

  • (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp (+1-2)
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
index 4940062eeae47..8e83ef9ae61df 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
@@ -87,9 +87,8 @@
 #include <wchar.h>
 #include <wordexp.h>
 
-#define _KERNEL  // to declare 'shminfo' structure
+#define _WANT_SYSVSHM_INTERNALS  // to declare 'shminfo' structure
 #include <sys/shm.h>
-#undef _KERNEL
 
 #undef IOC_DIRMASK
 

@jrtc27 jrtc27 requested a review from DimitryAndric June 6, 2025 11:44
Copy link

github-actions bot commented Jun 6, 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 HEAD~1 HEAD --extensions h,cpp -- compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h
View the diff from clang-format here.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
index 4c1e00528..252f17efc 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.cpp
@@ -29,23 +29,23 @@
 #include <sys/mtio.h>
 #include <sys/ptrace.h>
 #include <sys/resource.h>
-#include <sys/shm.h>
-#include <sys/signal.h>
-#include <sys/socket.h>
-#include <sys/sockio.h>
-#include <sys/soundcard.h>
-#include <sys/stat.h>
-#include <sys/statvfs.h>
-#include <sys/time.h>
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-W#warnings"
-#include <sys/timeb.h>
-#pragma clang diagnostic pop
-#include <sys/times.h>
-#include <sys/timespec.h>
-#include <sys/types.h>
-#include <sys/ucontext.h>
-#include <sys/utsname.h>
+#  include <sys/shm.h>
+#  include <sys/signal.h>
+#  include <sys/socket.h>
+#  include <sys/sockio.h>
+#  include <sys/soundcard.h>
+#  include <sys/stat.h>
+#  include <sys/statvfs.h>
+#  include <sys/time.h>
+#  pragma clang diagnostic push
+#  pragma clang diagnostic ignored "-W#warnings"
+#  include <sys/timeb.h>
+#  pragma clang diagnostic pop
+#  include <sys/times.h>
+#  include <sys/timespec.h>
+#  include <sys/types.h>
+#  include <sys/ucontext.h>
+#  include <sys/utsname.h>
 //
 #include <arpa/inet.h>
 #include <net/ethernet.h>
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h
index 382b67ce7..91ca73d29 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h
@@ -422,11 +422,11 @@ typedef void __sanitizer_FILE;
 extern int shmctl_ipc_stat;
 
 // This simplifies generic code
-#define struct_shminfo_sz -1
-#define struct_shm_info_sz -1
-#define shmctl_shm_stat -1
-#define shmctl_ipc_info -1
-#define shmctl_shm_info -1
+#  define struct_shminfo_sz -1
+#  define struct_shm_info_sz -1
+#  define shmctl_shm_stat -1
+#  define shmctl_ipc_info -1
+#  define shmctl_shm_info -1
 
 extern unsigned struct_utmpx_sz;
 

@jrtc27
Copy link
Collaborator Author

jrtc27 commented Jun 6, 2025

Hm, actually, we shouldn't even be trying to handle this (nor some others), they're Linux-specific and only defined so the Linuxulator can use them.

Despite being defined in the system headers, these commands are not in
fact part of the FreeBSD system call interface. They exist solely for
the Linuxulator, i.e. running Linux binaries on FreeBSD, and any attempt
to use them from a FreeBSD binary will return EINVAL. The fact we needed
to define _KERNEL (which, as the name implies, means we are compiling
the kernel) to even get the definition of shminfo should have been a
strong indicator that IPC_INFO at least was not a userspace interface.
@jrtc27 jrtc27 force-pushed the freebsd-shminfo-avoid-_KERNEL branch from bb30a78 to a9235ae Compare June 6, 2025 12:15
@jrtc27 jrtc27 changed the title [compiler-rt] Avoid defining _KERNEL on FreeBSD [compiler-rt] Don't handle Linux-specific shmctl commands in sanitizer Jun 6, 2025
Copy link
Collaborator

@DimitryAndric DimitryAndric left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants