diff mbox

Revert libsanitizer patches or fix 59009

Message ID 20131112193015.GZ27813@tucnak.zalov.cz
State New
Headers show

Commit Message

Jakub Jelinek Nov. 12, 2013, 7:30 p.m. UTC
On Tue, Nov 12, 2013 at 10:59:12AM -0800, Kostya Serebryany wrote:
> This is all dead code in gcc repo. This is why I am asking for any
> quick #ifdef.
> in clang repo this code is used by MemorySanitizer (and will be used
> by asan/tsan later).

I can't find how it is used in msan/ in the llvm repo either, though
admittedly just using grep (was grepping for pre_impl or post_impl),
but sanitizer_common_syscalls.inc #undefs PRE_SYSCALL/POST_SYSCALL
at the end, so I'm curious how do you use it.  But not enough interest
to actually build the llvm stuff.

Anyway, the following #ifdefs out tons of dead code and still builds just
fine, the only difference is that those symbols nothing really uses from
libasan/libtsan are no longer exported, but nothing else changes.

I guess the #if 0 can be replaced by some #ifdef with some nice name or
something.

BTW, the 10MB .bss in libubsan is really insane, while perhaps users are
expecting to have huge overhead with libasan or libtsan, for libubsan it
looks way too big.  Why do you need it?



	Jakub

Comments

Kostya Serebryany Nov. 12, 2013, 7:34 p.m. UTC | #1
On Tue, Nov 12, 2013 at 11:30 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Nov 12, 2013 at 10:59:12AM -0800, Kostya Serebryany wrote:
>> This is all dead code in gcc repo. This is why I am asking for any
>> quick #ifdef.
>> in clang repo this code is used by MemorySanitizer (and will be used
>> by asan/tsan later).
>
> I can't find how it is used in msan/ in the llvm repo either, though
> admittedly just using grep (was grepping for pre_impl or post_impl),
> but sanitizer_common_syscalls.inc #undefs PRE_SYSCALL/POST_SYSCALL

we only have it in tests: msan/lit_tests/Linux/syscalls.cc

> at the end, so I'm curious how do you use it.  But not enough interest
> to actually build the llvm stuff.
>
> Anyway, the following #ifdefs out tons of dead code and still builds just

Thanks, that should work.
I am really sorry I couldn't do it myself before -- just got back from travel.

> fine, the only difference is that those symbols nothing really uses from
> libasan/libtsan are no longer exported, but nothing else changes.
>
> I guess the #if 0 can be replaced by some #ifdef with some nice name or
> something.
>
> BTW, the 10MB .bss in libubsan is really insane, while perhaps users are
> expecting to have huge overhead with libasan or libtsan, for libubsan it
> looks way too big.  Why do you need it?

libubsan is a bit too far away from me, and especially the gcc build of it.

>
> --- libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc.jj 2013-11-12 11:31:00.000000000 +0100
> +++ libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc    2013-11-12 20:04:49.981519143 +0100
> @@ -14,6 +14,7 @@
>  // userspace headers.
>  // Most "normal" includes go in sanitizer_platform_limits_posix.cc
>
> +#if 0
>  #include "sanitizer_platform.h"
>  #if SANITIZER_LINUX
>
> @@ -43,3 +44,4 @@ namespace __sanitizer {
>  }  // namespace __sanitizer
>
>  #endif  // SANITIZER_LINUX
> +#endif
> --- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc.jj 2013-11-12 11:31:00.000000000 +0100
> +++ libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc    2013-11-12 20:04:35.967587643 +0100
> @@ -10,7 +10,7 @@
>  // Sizes and layouts of platform-specific POSIX data structures.
>  //===----------------------------------------------------------------------===//
>
> -
> +#if 0
>  #include "sanitizer_platform.h"
>  #if SANITIZER_LINUX || SANITIZER_MAC
>
> @@ -881,3 +881,4 @@ CHECK_SIZE_AND_OFFSET(wordexp_t, we_offs
>  #endif
>
>  #endif  // SANITIZER_LINUX || SANITIZER_MAC
> +#endif
> --- libsanitizer/sanitizer_common/sanitizer_common_syscalls.inc.jj      2013-11-12 11:31:00.000000000 +0100
> +++ libsanitizer/sanitizer_common/sanitizer_common_syscalls.inc 2013-11-12 20:04:11.295715500 +0100
> @@ -58,6 +58,8 @@
>  # define COMMON_SYSCALL_POST_FORK(res)
>  #endif
>
> +#if 0
> +
>  // FIXME: do some kind of PRE_READ for all syscall arguments (int(s) and such).
>
>  extern "C" {
> @@ -2722,6 +2724,8 @@ POST_SYSCALL(vfork)(long res) {
>  }
>  }  // extern "C"
>
> +#endif
> +
>  #undef PRE_SYSCALL
>  #undef PRE_READ
>  #undef PRE_WRITE
>
>
>         Jakub
Jakub Jelinek Nov. 12, 2013, 7:40 p.m. UTC | #2
On Tue, Nov 12, 2013 at 11:34:48AM -0800, Kostya Serebryany wrote:
> > Anyway, the following #ifdefs out tons of dead code and still builds just
> 
> Thanks, that should work.
> I am really sorry I couldn't do it myself before -- just got back from travel.

So do you have some suggestion for #ifdef KERNEL_HEADER_WORKAROUND so it
won't confuse you during next merge?

> > fine, the only difference is that those symbols nothing really uses from
> > libasan/libtsan are no longer exported, but nothing else changes.
> >
> > I guess the #if 0 can be replaced by some #ifdef with some nice name or
> > something.
> >
> > BTW, the 10MB .bss in libubsan is really insane, while perhaps users are
> > expecting to have huge overhead with libasan or libtsan, for libubsan it
> > looks way too big.  Why do you need it?
> 
> libubsan is a bit too far away from me, and especially the gcc build of it.

It is sanitizer_allocator.cc's
static ALIGNED(64) char internal_alloc_placeholder[sizeof(InternalAllocator)];
Dunno why it is so huge, why libubsan needs an allocator at all, etc.

	Jakub
Michael Meissner Nov. 12, 2013, 7:47 p.m. UTC | #3
On Tue, Nov 12, 2013 at 11:34:48AM -0800, Kostya Serebryany wrote:
> On Tue, Nov 12, 2013 at 11:30 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Tue, Nov 12, 2013 at 10:59:12AM -0800, Kostya Serebryany wrote:
> >> This is all dead code in gcc repo. This is why I am asking for any
> >> quick #ifdef.
> >> in clang repo this code is used by MemorySanitizer (and will be used
> >> by asan/tsan later).
> >
> > I can't find how it is used in msan/ in the llvm repo either, though
> > admittedly just using grep (was grepping for pre_impl or post_impl),
> > but sanitizer_common_syscalls.inc #undefs PRE_SYSCALL/POST_SYSCALL
> 
> we only have it in tests: msan/lit_tests/Linux/syscalls.cc
> 
> > at the end, so I'm curious how do you use it.  But not enough interest
> > to actually build the llvm stuff.
> >
> > Anyway, the following #ifdefs out tons of dead code and still builds just
> 
> Thanks, that should work.
> I am really sorry I couldn't do it myself before -- just got back from travel.

In the future, don't commit patches like this just before travel, unless you
know you will have a good internet connection where you are going, and you will
have enough cycles to spare from your travels to fix any fall out.
Jakub Jelinek Nov. 12, 2013, 11:47 p.m. UTC | #4
On Tue, Nov 12, 2013 at 08:30:15PM +0100, Jakub Jelinek wrote:
> Anyway, the following #ifdefs out tons of dead code and still builds just
> fine, the only difference is that those symbols nothing really uses from
> libasan/libtsan are no longer exported, but nothing else changes.

Actually, ifdefing out the *limits_posix.cc file didn't work well, because
the ioctl wrapping isn't dead, so I'm afraid we are back to including
sys/vt.h again.  But, at least for now the dead syscall interception
is #ifdefed out.  Tested on x86_64-linux, committed to trunk.

> I guess the #if 0 can be replaced by some #ifdef with some nice name or
> something.
> 
> BTW, the 10MB .bss in libubsan is really insane, while perhaps users are
> expecting to have huge overhead with libasan or libtsan, for libubsan it
> looks way too big.  Why do you need it?

Seems the allocator is brought in through a chain of *.o files from
sanitizer_common, libubsan wants the backtrace stuff, but I'd hope if it
needs an allocator for that, it doesn't need one backed by 10MB .bss buffer,
some fairly minimal would be much better for that.

	Jakub
diff mbox

Patch

--- libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc.jj	2013-11-12 11:31:00.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc	2013-11-12 20:04:49.981519143 +0100
@@ -14,6 +14,7 @@ 
 // userspace headers.
 // Most "normal" includes go in sanitizer_platform_limits_posix.cc
 
+#if 0
 #include "sanitizer_platform.h"
 #if SANITIZER_LINUX
 
@@ -43,3 +44,4 @@  namespace __sanitizer {
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_LINUX
+#endif
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc.jj	2013-11-12 11:31:00.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc	2013-11-12 20:04:35.967587643 +0100
@@ -10,7 +10,7 @@ 
 // Sizes and layouts of platform-specific POSIX data structures.
 //===----------------------------------------------------------------------===//
 
-
+#if 0
 #include "sanitizer_platform.h"
 #if SANITIZER_LINUX || SANITIZER_MAC
 
@@ -881,3 +881,4 @@  CHECK_SIZE_AND_OFFSET(wordexp_t, we_offs
 #endif
 
 #endif  // SANITIZER_LINUX || SANITIZER_MAC
+#endif
--- libsanitizer/sanitizer_common/sanitizer_common_syscalls.inc.jj	2013-11-12 11:31:00.000000000 +0100
+++ libsanitizer/sanitizer_common/sanitizer_common_syscalls.inc	2013-11-12 20:04:11.295715500 +0100
@@ -58,6 +58,8 @@ 
 # define COMMON_SYSCALL_POST_FORK(res)
 #endif
 
+#if 0
+
 // FIXME: do some kind of PRE_READ for all syscall arguments (int(s) and such).
 
 extern "C" {
@@ -2722,6 +2724,8 @@  POST_SYSCALL(vfork)(long res) {
 }
 }  // extern "C"
 
+#endif
+
 #undef PRE_SYSCALL
 #undef PRE_READ
 #undef PRE_WRITE