| Message ID | adcf682b-bbb4-b07c-50a5-0231f6868414@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892). | expand |
On 11/8/18 1:27 AM, Martin Liška wrote: > Hi. > > The GetNumberOfCPUs functionality is only used from Scudo allocator: > https://llvm.org/docs/ScudoHardenedAllocator.html > > The hardening allocator is not used from GCC, thus I recommend to remove > the function. > > Ready for trunk? > Martin > > libsanitizer/ChangeLog: > > 2018-11-08 Martin Liska <mliska@suse.cz> > > PR sanitizer/87892 > * (all files): Revert upstream r318802. Is it causing a build failure or somesuch? ie, why specifically are you wanting to remove it? jeff
On Thu, Nov 08, 2018 at 01:43:29PM -0700, Jeff Law wrote: > On 11/8/18 1:27 AM, Martin Liška wrote: > > libsanitizer/ChangeLog: > > > > 2018-11-08 Martin Liska <mliska@suse.cz> > > > > PR sanitizer/87892 > > * (all files): Revert upstream r318802. > Is it causing a build failure or somesuch? ie, why specifically are you > wanting to remove it? Yes. But perhaps it would be enough to just guard the CPU_COUNT use with #ifdef CPU_COUNT and have some fallback, including say returning just 1. If we care about Scudo or whatever is (what would be needed for that on the compiler side?), then we'd need a proper implementation, one that doesn't fail if a machine has more CPUs than fit into cpu_set_t, or if old glibc is used and CPU_COUNT isn't defined, or even if the kernel doesn't have affinity stuff at all. Jakub
On 11/8/18 1:48 PM, Jakub Jelinek wrote: > On Thu, Nov 08, 2018 at 01:43:29PM -0700, Jeff Law wrote: >> On 11/8/18 1:27 AM, Martin Liška wrote: >>> libsanitizer/ChangeLog: >>> >>> 2018-11-08 Martin Liska <mliska@suse.cz> >>> >>> PR sanitizer/87892 >>> * (all files): Revert upstream r318802. >> Is it causing a build failure or somesuch? ie, why specifically are you >> wanting to remove it? > > Yes. But perhaps it would be enough to just guard the CPU_COUNT use with > #ifdef CPU_COUNT and have some fallback, including say returning just 1. > If we care about Scudo or whatever is (what would be needed for that on the > compiler side?), then we'd need a proper implementation, one that doesn't > fail if a machine has more CPUs than fit into cpu_set_t, or if old glibc is > used and CPU_COUNT isn't defined, or even if the kernel doesn't have > affinity stuff at all. The obvious idea being to disable it with a more minimal patch making future merges easier -- if there's a less intrusive way to disable the bits, then that's fine with me. jeff
On 11/8/18 9:50 PM, Jeff Law wrote: > On 11/8/18 1:48 PM, Jakub Jelinek wrote: >> On Thu, Nov 08, 2018 at 01:43:29PM -0700, Jeff Law wrote: >>> On 11/8/18 1:27 AM, Martin Liška wrote: >>>> libsanitizer/ChangeLog: >>>> >>>> 2018-11-08 Martin Liska <mliska@suse.cz> >>>> >>>> PR sanitizer/87892 >>>> * (all files): Revert upstream r318802. >>> Is it causing a build failure or somesuch? ie, why specifically are you >>> wanting to remove it? >> >> Yes. But perhaps it would be enough to just guard the CPU_COUNT use with >> #ifdef CPU_COUNT and have some fallback, including say returning just 1. >> If we care about Scudo or whatever is (what would be needed for that on the >> compiler side?), then we'd need a proper implementation, one that doesn't >> fail if a machine has more CPUs than fit into cpu_set_t, or if old glibc is >> used and CPU_COUNT isn't defined, or even if the kernel doesn't have >> affinity stuff at all. > The obvious idea being to disable it with a more minimal patch making > future merges easier -- if there's a less intrusive way to disable the > bits, then that's fine with me. > > jeff > Ok, then I'm going to install following patch. Martin From 40766a658e15019c8fd03f678a988ce53c2495b1 Mon Sep 17 00:00:00 2001 From: marxin <mliska@suse.cz> Date: Fri, 9 Nov 2018 09:44:15 +0100 Subject: [PATCH] Fallback in libsanitizer for scudo sanitizer (PR sanitizer/87892). libsanitizer/ChangeLog: 2018-11-09 Martin Liska <mliska@suse.cz> PR sanitizer/87892 * sanitizer_common/sanitizer_linux_libcdep.cc (defined): Return 1 when CPU_COUNT macro is not defined. --- libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc index 32f335eaf23..28360f5656a 100644 --- a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc +++ b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc @@ -694,9 +694,13 @@ u32 GetNumberOfCPUs() { #elif SANITIZER_SOLARIS return sysconf(_SC_NPROCESSORS_ONLN); #else +#if defined(CPU_COUNT) cpu_set_t CPUs; CHECK_EQ(sched_getaffinity(0, sizeof(cpu_set_t), &CPUs), 0); return CPU_COUNT(&CPUs); +#else + return 1; +#endif #endif }
On Fri, Nov 09, 2018 at 10:12:32AM +0100, Martin Liška wrote: > Ok, then I'm going to install following patch. Thanks. > 2018-11-09 Martin Liska <mliska@suse.cz> > > PR sanitizer/87892 > * sanitizer_common/sanitizer_linux_libcdep.cc (defined): Return > 1 when CPU_COUNT macro is not defined. > --- > libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc > index 32f335eaf23..28360f5656a 100644 > --- a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc > +++ b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc > @@ -694,9 +694,13 @@ u32 GetNumberOfCPUs() { > #elif SANITIZER_SOLARIS > return sysconf(_SC_NPROCESSORS_ONLN); > #else > +#if defined(CPU_COUNT) > cpu_set_t CPUs; > CHECK_EQ(sched_getaffinity(0, sizeof(cpu_set_t), &CPUs), 0); > return CPU_COUNT(&CPUs); > +#else > + return 1; > +#endif > #endif > } > > -- > 2.19.1 > Jakub
diff --git a/libsanitizer/sanitizer_common/sanitizer_common.cc b/libsanitizer/sanitizer_common/sanitizer_common.cc index 7f0f47c005d..29f35bb599a 100644 --- a/libsanitizer/sanitizer_common/sanitizer_common.cc +++ b/libsanitizer/sanitizer_common/sanitizer_common.cc @@ -23,7 +23,6 @@ const char *SanitizerToolName = "SanitizerTool"; atomic_uint32_t current_verbosity; uptr PageSizeCached; -u32 NumberOfCPUsCached; // PID of the tracer task in StopTheWorld. It shares the address space with the // main process, but has a different PID and thus requires special handling. diff --git a/libsanitizer/sanitizer_common/sanitizer_common.h b/libsanitizer/sanitizer_common/sanitizer_common.h index 603d922b969..d57434ae1dd 100644 --- a/libsanitizer/sanitizer_common/sanitizer_common.h +++ b/libsanitizer/sanitizer_common/sanitizer_common.h @@ -939,15 +939,6 @@ void CheckNoDeepBind(const char *filename, int flag); // be used to seed a PRNG. Defaults to blocking like the underlying syscall. bool GetRandom(void *buffer, uptr length, bool blocking = true); -// Returns the number of logical processors on the system. -u32 GetNumberOfCPUs(); -extern u32 NumberOfCPUsCached; -INLINE u32 GetNumberOfCPUsCached() { - if (!NumberOfCPUsCached) - NumberOfCPUsCached = GetNumberOfCPUs(); - return NumberOfCPUsCached; -} - } // namespace __sanitizer inline void *operator new(__sanitizer::operator_new_size_type size, diff --git a/libsanitizer/sanitizer_common/sanitizer_fuchsia.cc b/libsanitizer/sanitizer_common/sanitizer_fuchsia.cc index 6602f97b40b..d38151d2711 100644 --- a/libsanitizer/sanitizer_common/sanitizer_fuchsia.cc +++ b/libsanitizer/sanitizer_common/sanitizer_fuchsia.cc @@ -482,10 +482,6 @@ bool GetRandom(void *buffer, uptr length, bool blocking) { return true; } -u32 GetNumberOfCPUs() { - return zx_system_get_num_cpus(); -} - uptr GetRSS() { UNIMPLEMENTED(); } } // namespace __sanitizer diff --git a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc index 32f335eaf23..0cdc97111f4 100644 --- a/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc +++ b/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc @@ -35,17 +35,14 @@ #if SANITIZER_FREEBSD #include <pthread_np.h> #include <osreldate.h> -#include <sys/sysctl.h> #define pthread_getattr_np pthread_attr_get_np #endif #if SANITIZER_OPENBSD #include <pthread_np.h> -#include <sys/sysctl.h> #endif #if SANITIZER_NETBSD -#include <sys/sysctl.h> #include <sys/tls.h> #endif @@ -55,16 +52,6 @@ #if SANITIZER_ANDROID #include <android/api-level.h> -#if !defined(CPU_COUNT) && !defined(__aarch64__) -#include <dirent.h> -#include <fcntl.h> -struct __sanitizer::linux_dirent { - long d_ino; - off_t d_off; - unsigned short d_reclen; - char d_name[]; -}; -#endif #endif #if !SANITIZER_ANDROID @@ -644,62 +631,6 @@ uptr GetRSS() { return rss * GetPageSizeCached(); } -// sysconf(_SC_NPROCESSORS_{CONF,ONLN}) cannot be used on most platforms as -// they allocate memory. -u32 GetNumberOfCPUs() { -#if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_OPENBSD - u32 ncpu; - int req[2]; - uptr len = sizeof(ncpu); - req[0] = CTL_HW; - req[1] = HW_NCPU; - CHECK_EQ(internal_sysctl(req, 2, &ncpu, &len, NULL, 0), 0); - return ncpu; -#elif SANITIZER_ANDROID && !defined(CPU_COUNT) && !defined(__aarch64__) - // Fall back to /sys/devices/system/cpu on Android when cpu_set_t doesn't - // exist in sched.h. That is the case for toolchains generated with older - // NDKs. - // This code doesn't work on AArch64 because internal_getdents makes use of - // the 64bit getdents syscall, but cpu_set_t seems to always exist on AArch64. - uptr fd = internal_open("/sys/devices/system/cpu", O_RDONLY | O_DIRECTORY); - if (internal_iserror(fd)) - return 0; - InternalMmapVector<u8> buffer(4096); - uptr bytes_read = buffer.size(); - uptr n_cpus = 0; - u8 *d_type; - struct linux_dirent *entry = (struct linux_dirent *)&buffer[bytes_read]; - while (true) { - if ((u8 *)entry >= &buffer[bytes_read]) { - bytes_read = internal_getdents(fd, (struct linux_dirent *)buffer.data(), - buffer.size()); - if (internal_iserror(bytes_read) || !bytes_read) - break; - entry = (struct linux_dirent *)buffer.data(); - } - d_type = (u8 *)entry + entry->d_reclen - 1; - if (d_type >= &buffer[bytes_read] || - (u8 *)&entry->d_name[3] >= &buffer[bytes_read]) - break; - if (entry->d_ino != 0 && *d_type == DT_DIR) { - if (entry->d_name[0] == 'c' && entry->d_name[1] == 'p' && - entry->d_name[2] == 'u' && - entry->d_name[3] >= '0' && entry->d_name[3] <= '9') - n_cpus++; - } - entry = (struct linux_dirent *)(((u8 *)entry) + entry->d_reclen); - } - internal_close(fd); - return n_cpus; -#elif SANITIZER_SOLARIS - return sysconf(_SC_NPROCESSORS_ONLN); -#else - cpu_set_t CPUs; - CHECK_EQ(sched_getaffinity(0, sizeof(cpu_set_t), &CPUs), 0); - return CPU_COUNT(&CPUs); -#endif -} - #if SANITIZER_LINUX # if SANITIZER_ANDROID diff --git a/libsanitizer/sanitizer_common/sanitizer_mac.cc b/libsanitizer/sanitizer_common/sanitizer_mac.cc index 28b2906e226..bd4949c1b75 100644 --- a/libsanitizer/sanitizer_common/sanitizer_mac.cc +++ b/libsanitizer/sanitizer_common/sanitizer_mac.cc @@ -1082,10 +1082,6 @@ bool GetRandom(void *buffer, uptr length, bool blocking) { return true; } -u32 GetNumberOfCPUs() { - return (u32)sysconf(_SC_NPROCESSORS_ONLN); -} - } // namespace __sanitizer #endif // SANITIZER_MAC diff --git a/libsanitizer/sanitizer_common/sanitizer_win.cc b/libsanitizer/sanitizer_common/sanitizer_win.cc index ebc6c503036..74c04050a7a 100644 --- a/libsanitizer/sanitizer_common/sanitizer_win.cc +++ b/libsanitizer/sanitizer_common/sanitizer_win.cc @@ -1047,12 +1047,6 @@ bool GetRandom(void *buffer, uptr length, bool blocking) { UNIMPLEMENTED(); } -u32 GetNumberOfCPUs() { - SYSTEM_INFO sysinfo = {}; - GetNativeSystemInfo(&sysinfo); - return sysinfo.dwNumberOfProcessors; -} - } // namespace __sanitizer #endif // _WIN32