[Sanitizers] Remove freebsd/netbsd from "posix" file #123269
Conversation
FreeBSD and NetBSD have been separated into their own files for years. These references were either missed during the separation or were added afterward during other cleanups and refactorings.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Kyle Lippincott (spectral54) ChangesFreeBSD and NetBSD have been separated into their own files for years. These references were either missed during the separation or were added afterward during other cleanups and refactorings. Full diff: /~https://github.com/llvm/llvm-project/pull/123269.diff 2 Files Affected:
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
index ddd67cb43524d4..e2dc61d38d8f81 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.cpp
@@ -213,10 +213,9 @@ namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
#endif // SANITIZER_HAS_STATFS64
-#if SANITIZER_GLIBC || SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_APPLE
+#if SANITIZER_GLIBC || SANITIZER_APPLE
unsigned struct_fstab_sz = sizeof(struct fstab);
-#endif // SANITIZER_GLIBC || SANITIZER_FREEBSD || SANITIZER_NETBSD ||
- // SANITIZER_APPLE
+#endif // SANITIZER_GLIBC || SANITIZER_APPLE
#if !SANITIZER_ANDROID
unsigned struct_statfs_sz = sizeof(struct statfs);
unsigned struct_sockaddr_sz = sizeof(struct sockaddr);
@@ -324,7 +323,7 @@ namespace __sanitizer {
int shmctl_shm_stat = (int)SHM_STAT;
#endif
-#if !SANITIZER_APPLE && !SANITIZER_FREEBSD
+#if !SANITIZER_APPLE
unsigned struct_utmp_sz = sizeof(struct utmp);
#endif
#if !SANITIZER_ANDROID
@@ -347,8 +346,6 @@ namespace __sanitizer {
#if SANITIZER_LINUX
unsigned struct_ElfW_Phdr_sz = sizeof(ElfW(Phdr));
-#elif SANITIZER_FREEBSD
-unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr);
#endif
#if SANITIZER_GLIBC
@@ -987,7 +984,7 @@ unsigned struct_ElfW_Phdr_sz = sizeof(Elf_Phdr);
unsigned IOCTL_PIO_SCRNMAP = PIO_SCRNMAP;
unsigned IOCTL_SNDCTL_DSP_GETISPACE = SNDCTL_DSP_GETISPACE;
unsigned IOCTL_SNDCTL_DSP_GETOSPACE = SNDCTL_DSP_GETOSPACE;
-#endif // (SANITIZER_LINUX || SANITIZER_FREEBSD) && !SANITIZER_ANDROID
+#endif // SANITIZER_LINUX && !SANITIZER_ANDROID
const int si_SEGV_MAPERR = SEGV_MAPERR;
const int si_SEGV_ACCERR = SEGV_ACCERR;
@@ -1025,7 +1022,7 @@ COMPILER_CHECK(IOC_NR(0x12345678) == _IOC_NR(0x12345678));
COMPILER_CHECK(IOC_TYPE(0x12345678) == _IOC_TYPE(0x12345678));
#endif // SANITIZER_LINUX
-#if SANITIZER_LINUX || SANITIZER_FREEBSD
+#if SANITIZER_LINUX
// There are more undocumented fields in dl_phdr_info that we are not interested
// in.
COMPILER_CHECK(sizeof(__sanitizer_dl_phdr_info) <= sizeof(dl_phdr_info));
@@ -1033,9 +1030,9 @@ CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_addr);
CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_name);
CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_phdr);
CHECK_SIZE_AND_OFFSET(dl_phdr_info, dlpi_phnum);
-#endif // SANITIZER_LINUX || SANITIZER_FREEBSD
+#endif // SANITIZER_LINUX
-#if SANITIZER_GLIBC || SANITIZER_FREEBSD
+#if SANITIZER_GLIBC
CHECK_TYPE_SIZE(glob_t);
CHECK_SIZE_AND_OFFSET(glob_t, gl_pathc);
CHECK_SIZE_AND_OFFSET(glob_t, gl_pathv);
@@ -1046,7 +1043,7 @@ CHECK_SIZE_AND_OFFSET(glob_t, gl_readdir);
CHECK_SIZE_AND_OFFSET(glob_t, gl_opendir);
CHECK_SIZE_AND_OFFSET(glob_t, gl_lstat);
CHECK_SIZE_AND_OFFSET(glob_t, gl_stat);
-#endif // SANITIZER_GLIBC || SANITIZER_FREEBSD
+#endif // SANITIZER_GLIBC
CHECK_TYPE_SIZE(addrinfo);
CHECK_SIZE_AND_OFFSET(addrinfo, ai_flags);
@@ -1103,8 +1100,6 @@ COMPILER_CHECK(sizeof(__sanitizer_dirent) <= sizeof(dirent));
CHECK_SIZE_AND_OFFSET(dirent, d_ino);
#if SANITIZER_APPLE
CHECK_SIZE_AND_OFFSET(dirent, d_seekoff);
-#elif SANITIZER_FREEBSD
-// There is no 'd_off' field on FreeBSD.
#else
CHECK_SIZE_AND_OFFSET(dirent, d_off);
#endif
@@ -1206,15 +1201,10 @@ CHECK_SIZE_AND_OFFSET(mntent, mnt_passno);
CHECK_TYPE_SIZE(ether_addr);
-#if SANITIZER_GLIBC || SANITIZER_FREEBSD
+#if SANITIZER_GLIBC
CHECK_TYPE_SIZE(ipc_perm);
-# if SANITIZER_FREEBSD
-CHECK_SIZE_AND_OFFSET(ipc_perm, key);
-CHECK_SIZE_AND_OFFSET(ipc_perm, seq);
-# else
CHECK_SIZE_AND_OFFSET(ipc_perm, __key);
CHECK_SIZE_AND_OFFSET(ipc_perm, __seq);
-# endif
CHECK_SIZE_AND_OFFSET(ipc_perm, uid);
CHECK_SIZE_AND_OFFSET(ipc_perm, gid);
CHECK_SIZE_AND_OFFSET(ipc_perm, cuid);
@@ -1248,20 +1238,16 @@ CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_next);
CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_name);
CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_addr);
CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_netmask);
-#if SANITIZER_LINUX || SANITIZER_FREEBSD
+#if SANITIZER_LINUX
// Compare against the union, because we can't reach into the union in a
// compliant way.
#ifdef ifa_dstaddr
#undef ifa_dstaddr
#endif
-# if SANITIZER_FREEBSD
-CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_dstaddr);
-# else
COMPILER_CHECK(sizeof(((__sanitizer_ifaddrs *)nullptr)->ifa_dstaddr) ==
sizeof(((ifaddrs *)nullptr)->ifa_ifu));
COMPILER_CHECK(offsetof(__sanitizer_ifaddrs, ifa_dstaddr) ==
offsetof(ifaddrs, ifa_ifu));
-# endif // SANITIZER_FREEBSD
#else
CHECK_SIZE_AND_OFFSET(ifaddrs, ifa_dstaddr);
#endif // SANITIZER_LINUX
@@ -1352,7 +1338,7 @@ CHECK_SIZE_AND_OFFSET(cookie_io_functions_t, seek);
CHECK_SIZE_AND_OFFSET(cookie_io_functions_t, close);
#endif // SANITIZER_GLIBC
-#if SANITIZER_LINUX || SANITIZER_FREEBSD
+#if SANITIZER_LINUX
CHECK_TYPE_SIZE(sem_t);
#endif
@@ -1360,4 +1346,4 @@ CHECK_TYPE_SIZE(sem_t);
COMPILER_CHECK(ARM_VFPREGS_SIZE == ARM_VFPREGS_SIZE_ASAN);
#endif
-#endif // SANITIZER_LINUX || SANITIZER_FREEBSD || SANITIZER_APPLE
+#endif // SANITIZER_LINUX || SANITIZER_APPLE
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
index 7e62dc0e0523ec..6af22180d69bcb 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -655,17 +655,13 @@ struct __sanitizer_sigaction {
};
#else // !SANITIZER_ANDROID
struct __sanitizer_sigaction {
-#if defined(__mips__) && !SANITIZER_FREEBSD
+#if defined(__mips__)
unsigned int sa_flags;
#endif
union {
__sanitizer_sigactionhandler_ptr sigaction;
__sanitizer_sighandler_ptr handler;
};
-#if SANITIZER_FREEBSD
- int sa_flags;
- __sanitizer_sigset_t sa_mask;
-#else
#if defined(__s390x__)
int sa_resv;
#else
@@ -688,7 +684,6 @@ struct __sanitizer_sigaction {
int sa_flags;
#endif
#endif
-#endif
#if SANITIZER_LINUX
void (*sa_restorer)();
#endif
@@ -918,7 +913,7 @@ extern int shmctl_shm_info;
extern int shmctl_shm_stat;
#endif
-#if !SANITIZER_APPLE && !SANITIZER_FREEBSD
+#if !SANITIZER_APPLE
extern unsigned struct_utmp_sz;
#endif
#if !SANITIZER_ANDROID
@@ -1084,7 +1079,7 @@ extern const unsigned long __sanitizer_bufsiz;
#if SANITIZER_LINUX && !SANITIZER_ANDROID
extern unsigned struct_audio_buf_info_sz;
extern unsigned struct_ppp_stats_sz;
-#endif // (SANITIZER_LINUX || SANITIZER_FREEBSD) && !SANITIZER_ANDROID
+#endif // SANITIZER_LINUX && !SANITIZER_ANDROID
#if !SANITIZER_ANDROID && !SANITIZER_APPLE
extern unsigned struct_sioc_sg_req_sz;
|
|
I don't have access to select reviewers, but @thurstond mentioned that I could add them as a reviewer. This is patch 1 of 6 in a series (I also have an alternative way of splitting them, combining 1+2, 3+4, and 5+6 so it's only a series of 3). I couldn't get graphite to work (I think because I'm not part of the llvm organization?), so I was just going to send them manually, with the next one being sent after the previous one lands. The entire series can be seen here: main...spectral54:llvm-project:users/spectral/regexec-06-implement-startend I intentionally didn't run clang-format on this commit, because it introduces a LOT of changes to this file and I didn't think that was correct. |
|
compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_freebsd.h has: so it's not clear whether the It's possible they are redundant, but it'd be useful to have strong confirmation, such as running the regression tests on FreeBSD and NetBSD. |
|
Interesting, I wonder why I didn't see that. I thought I looked for it. It's pretty clearly been there from the beginning (a742193). Well I guess I'll abandon this change then :) I have some freebsd-specific and netbsd-specific changes in later changes in the series, is there a way I can run the test suite as a github action, or do I need to find a VM with freebsd and netbsd to test these with? |
Hmm, VM might be the best way? I couldn't find any BSD sanitizer buildbots, either on github or LLVM. (Google's sanitizer bot dashboard is at http://google.github.io/sanitizers/show_bots.html but there's no BSD bots there; if there were, it'd be possible to "Force Build" at a particular bot.) |
|
P.S. Obviously I'd welcome if your patch supports BSD too, but it'd also be acceptable to support only a subset of platforms, and annotate your new test file with something like: or even more restrictively: (to also exclude Mac, Windows, 32-bit Linux, PowerPC, etc.) |
|
I think what happened was that I checked for netbsd, which doesn't seem to include sanitizer_platform_limits_posix.h ( It'll take me some time to get a VM set up. The changes to freebsd or netbsd that are strictly necessary for the latter commits in the series are pretty small, so I might skip the VM setup if I can visually confirm to my satisfaction that they "should" work properly. In the mean time, I'm abandoning this PR, sorry for the noise! |
No worries! :-) That sounds good. P.S. |
FreeBSD and NetBSD have been separated into their own files for years. These references were either missed during the separation or were added afterward during other cleanups and refactorings.