* [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892).
@ 2018-11-08 8:27 Martin Liška
2018-11-08 20:43 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2018-11-08 8:27 UTC (permalink / raw)
To: gcc-patches; +Cc: Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 755 bytes --]
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.
---
.../sanitizer_common/sanitizer_common.cc | 1 -
.../sanitizer_common/sanitizer_common.h | 9 ---
.../sanitizer_common/sanitizer_fuchsia.cc | 4 --
.../sanitizer_linux_libcdep.cc | 69 -------------------
.../sanitizer_common/sanitizer_mac.cc | 4 --
.../sanitizer_common/sanitizer_win.cc | 6 --
6 files changed, 93 deletions(-)
[-- Attachment #2: 0001-Revert-libsanitizer-r318802-as-we-don-t-use-Scudo-al.patch --]
[-- Type: text/x-patch, Size: 6197 bytes --]
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892).
2018-11-08 8:27 [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892) Martin Liška
@ 2018-11-08 20:43 ` Jeff Law
2018-11-08 20:48 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2018-11-08 20:43 UTC (permalink / raw)
To: Martin Liška, gcc-patches; +Cc: Jakub Jelinek
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892).
2018-11-08 20:43 ` Jeff Law
@ 2018-11-08 20:48 ` Jakub Jelinek
2018-11-08 20:50 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-11-08 20:48 UTC (permalink / raw)
To: Jeff Law; +Cc: Martin Liška, gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892).
2018-11-08 20:48 ` Jakub Jelinek
@ 2018-11-08 20:50 ` Jeff Law
2018-11-09 9:12 ` Martin Liška
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2018-11-08 20:50 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Martin Liška, gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892).
2018-11-08 20:50 ` Jeff Law
@ 2018-11-09 9:12 ` Martin Liška
2018-11-09 9:14 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Martin Liška @ 2018-11-09 9:12 UTC (permalink / raw)
To: Jeff Law, Jakub Jelinek; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]
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
[-- Attachment #2: 0001-Fallback-in-libsanitizer-for-scudo-sanitizer-PR-sani.patch --]
[-- Type: text/x-patch, Size: 1122 bytes --]
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
}
--
2.19.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892).
2018-11-09 9:12 ` Martin Liška
@ 2018-11-09 9:14 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2018-11-09 9:14 UTC (permalink / raw)
To: Martin Liška; +Cc: Jeff Law, gcc-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-09 9:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-08 8:27 [PATCH] Revert libsanitizer r318802 as we don't use Scudo allocator (PR sanitizer/87892) Martin Liška
2018-11-08 20:43 ` Jeff Law
2018-11-08 20:48 ` Jakub Jelinek
2018-11-08 20:50 ` Jeff Law
2018-11-09 9:12 ` Martin Liška
2018-11-09 9:14 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).