Re: libsanitizer merge from upstream r196090
Hi,
FAIL: g++.dg/tsan/default_options.C -O2 execution test
Both these tests don't report anything (well, default_options.C
prints DONE),
simply succeed (with dg-shouldfail that is a failure)
I've finally fixed g+.dg/tsan/default_options.C test (the check
dg-shouldfail should be removed).
OK to commit?
-Maxim
2014-02-04 Max Ostapenko m.ostape...@partner.samsung.com
* g++.dg/tsan/default_options.C: Invert check.
diff --git a/gcc/testsuite/g++.dg/tsan/default_options.C b/gcc/testsuite/g++.dg/tsan/default_options.C
index f0c0ece..13e1984 100644
--- a/gcc/testsuite/g++.dg/tsan/default_options.C
+++ b/gcc/testsuite/g++.dg/tsan/default_options.C
@@ -1,5 +1,3 @@
-/* { dg-shouldfail tsan } */
-
#include pthread.h
#include stdio.h
Re: libsanitizer merge from upstream r196090
On Tue, Feb 04, 2014 at 06:44:22PM +0400, Maxim Ostapenko wrote:
Hi,
FAIL: g++.dg/tsan/default_options.C -O2 execution test
Both these tests don't report anything (well, default_options.C
prints DONE),
simply succeed (with dg-shouldfail that is a failure)
I've finally fixed g+.dg/tsan/default_options.C test (the check
dg-shouldfail should be removed).
OK to commit?
Sure.
2014-02-04 Max Ostapenko m.ostape...@partner.samsung.com
* g++.dg/tsan/default_options.C: Invert check.
diff --git a/gcc/testsuite/g++.dg/tsan/default_options.C
b/gcc/testsuite/g++.dg/tsan/default_options.C
index f0c0ece..13e1984 100644
--- a/gcc/testsuite/g++.dg/tsan/default_options.C
+++ b/gcc/testsuite/g++.dg/tsan/default_options.C
@@ -1,5 +1,3 @@
-/* { dg-shouldfail tsan } */
-
#include pthread.h
#include stdio.h
Jakub
Re: libsanitizer merge from upstream r196090
Commited in 207472. -Maxim
Re: libsanitizer merge from upstream r196090
I've started a separate topic about flaky tsan tests here: https://groups.google.com/forum/#!topic/thread-sanitizer/KIok3F_b1oI --kcc On Fri, Jan 10, 2014 at 7:20 PM, Jakub Jelinek ja...@redhat.com wrote: On Fri, Jan 10, 2014 at 03:50:33PM +0400, Maxim Ostapenko wrote: On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinekja...@redhat.com wrote: Some of the tsan tests seems to FAIL randomly for quite a while (since they were added), didn't have time to look if it is just bugs in the test or some compiler issue or library issue. When I've commited these tsan tests, all of them were passed on my x86_64-unknown-linux-gnu 64bit system. Should I review them more carefully? That would be nice. The FAILs I'm seeing include e.g. FAIL: c-c++-common/tsan/simple_race.c -O2 execution test FAIL: c-c++-common/tsan/simple_race.c -O0 execution test FAIL: g++.dg/tsan/default_options.C -O2 execution test (and various others in the past, though not sure if any of the pattern related failures could have something with libbacktrace symbolization not being there yet (note, I do not have /usr/bin/llvm-symbolizer installed on the testing box)). Both these tests don't report anything (well, default_options.C prints DONE), simply succeed (with dg-shouldfail that is a failure). I don't see anything wrong in the compiler output, the Thread? routines just call __tsan_func_entry, then __tsan_write4 (Global); then __tsan_func_exit, so I don't see how it could be related to anything but the library. Note the box is sometimes quite loaded (two make -j48 regtests going on at the same time), but there is enough memory. Is the library perhaps timing sensitive, e.g. that it would track issues only if the two threads are actually concurrent rather than could be concurrent? Say if the first pthread_create creates thread immediately, second pthread_create returns but the clone started thread isn't up yet, then pthread_join on the first thread finishes and the first thread is destroyed, then the second thread actually starts? Jakub
Re: libsanitizer merge from upstream r196090
FAIL: g++.dg/tsan/default_options.C -O2 execution test This one looks plain wrong to me. It should be checked for success instead of failure. -Y
Re: libsanitizer merge from upstream r196090
Uros Bizjak wrote: The same tsan failures are reported in one of HJ's testers [2], with message: Can this be duplicate of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59410 ? -Y
Re: libsanitizer merge from upstream r196090
On Fri, Jan 10, 2014 at 10:23:59AM +0100, Uros Bizjak wrote: I was able to compile sanitizer with r206475 (after Jakub's fixes) and run the testsuite. However, on 64bit target, I got some failures in asan and several timeouts in tsan [1]. Some of the tsan tests seems to FAIL randomly for quite a while (since they were added), didn't have time to look if it is just bugs in the test or some compiler issue or library issue. The 32 bit target executes tsan tests without problems. [1] http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00689.html Obviously, tsan is only enabled for x86_64-linux 64-bit, so there are zero 32-bit tests ;) Jakub
Re: libsanitizer merge from upstream r196090
On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinek ja...@redhat.com wrote: On Fri, Jan 10, 2014 at 10:23:59AM +0100, Uros Bizjak wrote: I was able to compile sanitizer with r206475 (after Jakub's fixes) and run the testsuite. However, on 64bit target, I got some failures in asan and several timeouts in tsan [1]. Some of the tsan tests seems to FAIL randomly for quite a while (since they were added), didn't have time to look if it is just bugs in the test or some compiler issue or library issue. Just FYI, strace -f stops with: mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2abe3dd77000 mmap(NULL, 438272, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x2abe3dd78000 arch_prctl(ARCH_SET_FS, 0x2abe3dde1ac0) = 0 mprotect(0x2abe3db43000, 32768, PROT_READ) = 0 mprotect(0x2abe3d84, 4096, PROT_READ) = 0 mprotect(0x2abe3d638000, 4096, PROT_READ) = 0 mprotect(0x2abe3d42, 4096, PROT_READ) = 0 mprotect(0x2abe3d20e000, 16384, PROT_READ) = 0 mprotect(0x2abe3cebd000, 4096, PROT_READ) = 0 mprotect(0x2abe3bd32000, 4096, PROT_READ) = 0 munmap(0x2abe3cc1b000, 131456) = 0 set_tid_address(0x2abe3dde1b50) = 13757 set_robust_list(0x2abe3dde1b60, 0x18) = 0 futex(0x7fff2fe2f9bc, FUTEX_WAKE_PRIVATE, 1) = 0 mmap(0x7d00, 1099511627776, PROT_NONE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) = 0x7d00 mmap(0x7e00, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7e00 The 32 bit target executes tsan tests without problems. [1] http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00689.html Obviously, tsan is only enabled for x86_64-linux 64-bit, so there are zero 32-bit tests ;) Oh... no cookie here. :( Uros.
Re: libsanitizer merge from upstream r196090
Hi all, On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinekja...@redhat.com wrote: Some of the tsan tests seems to FAIL randomly for quite a while (since they were added), didn't have time to look if it is just bugs in the test or some compiler issue or library issue. When I've commited these tsan tests, all of them were passed on my x86_64-unknown-linux-gnu 64bit system. Should I review them more carefully? -Maxim.
Re: libsanitizer merge from upstream r196090
On Fri, Jan 10, 2014 at 10:23 AM, Uros Bizjak ubiz...@gmail.com wrote: On Mon, Dec 2, 2013 at 4:49 PM, Uros Bizjak ubiz...@gmail.com wrote: Does it support using libbacktrace in GCC? Not on it's own, but the support in the upstream maintained files is there, so hopefully it will be just a matter of follow-up patch with configury/Makefile etc. stuff, I'll work on it once the merge is committed. What is more important now is to test the patch Kostya posted on non-x86_64 targets and/or older kernel headers (say RHEL5, older SLES, etc.). Unfortunately, the build breaks on CentOS 5.10 (= RHEL5) with: libtool: compile: /home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc -B/home/uros/gcc-build-xxx/./gcc -nostdinc++ -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I. -I../../../../gcc-svn/trunk/libsanitizer/sanitizer_common -I ../../../../gcc-svn/trunk/libsanitizer/include -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -I../../libstdc++-v3/include -I../../libstdc++-v3/include/x86_64-unknown-linux-gnu -I../../../../gcc-svn/trunk/libsanitizer/../libstdc++-v3/libsupc++ -g -O2 -D_GNU_SOURCE -MT sanitizer_platform_limits_linux.lo -MD -MP -MF .deps/sanitizer_platform_limits_linux.Tpo -c ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc -fPIC -DPIC -o .libs/sanitizer_platform_limits_linux.o ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:54:30: fatal error: linux/perf_event.h: No such file or directory #include linux/perf_event.h ^ compilation terminated. gmake[4]: *** [sanitizer_platform_limits_linux.lo] Error 1 gmake[4]: *** Waiting for unfinished jobs I was able to compile sanitizer with r206475 (after Jakub's fixes) and run the testsuite. However, on 64bit target, I got some failures in asan and several timeouts in tsan [1]. Looking at: FAIL: c-c++-common/tsan/atomic_stack.c -O2 output pattern test, is , should match WARNING: ThreadSanitizer: data race.*( WARNING: program timed out. the execution hangs in: Program received signal SIGINT, Interrupt. 0x2c0c5652 in malloc_consolidate () from /lib64/libc.so.6 (gdb) bt #0 0x2c0c5652 in malloc_consolidate () from /lib64/libc.so.6 #1 0x2c0c6461 in mallopt () from /lib64/libc.so.6 #2 0x2af1a0e7 in __tsan::InitializeInterceptors () at ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:2099 #3 0x2aef8ab4 in __tsan::Initialize (thr=0x2c3aa9e0 main_arena) at ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_rtl.cc:223 #4 0x2af016a8 in ScopedInterceptor::ScopedInterceptor (this=0x7fffda00, thr=0x2cf0f580, fname=optimized out, pc=46912524510627) at ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:173 #5 0x2af16d77 in __interceptor_memset (dst=0x7fffda68, v=0, size=128) at ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:621 #6 0x2c5be5a3 in __pthread_initialize_minimal_internal () from /lib64/libpthread.so.0 #7 0x2c5bddd9 in _init () from /lib64/libpthread.so.0 #8 0x in ?? () The same tsan failures are reported in one of HJ's testers [2], with message: FAIL: c-c++-common/tsan/atomic_stack.c -O0 output pattern test, is FATAL: ThreadSanitizer can not mmap the shadow memory (something is mapped at 0x4000 0x7cf0) FAIL: c-c++-common/tsan/atomic_stack.c -O2 output pattern test, is FATAL: ThreadSanitizer can not mmap the shadow memory (something is mapped at 0x4000 0x7cf0) ... [1] http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00689.html [2] http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00705.html Uros.
Re: libsanitizer merge from upstream r196090
On Fri, Jan 10, 2014 at 03:50:33PM +0400, Maxim Ostapenko wrote: On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinekja...@redhat.com wrote: Some of the tsan tests seems to FAIL randomly for quite a while (since they were added), didn't have time to look if it is just bugs in the test or some compiler issue or library issue. When I've commited these tsan tests, all of them were passed on my x86_64-unknown-linux-gnu 64bit system. Should I review them more carefully? That would be nice. The FAILs I'm seeing include e.g. FAIL: c-c++-common/tsan/simple_race.c -O2 execution test FAIL: c-c++-common/tsan/simple_race.c -O0 execution test FAIL: g++.dg/tsan/default_options.C -O2 execution test (and various others in the past, though not sure if any of the pattern related failures could have something with libbacktrace symbolization not being there yet (note, I do not have /usr/bin/llvm-symbolizer installed on the testing box)). Both these tests don't report anything (well, default_options.C prints DONE), simply succeed (with dg-shouldfail that is a failure). I don't see anything wrong in the compiler output, the Thread? routines just call __tsan_func_entry, then __tsan_write4 (Global); then __tsan_func_exit, so I don't see how it could be related to anything but the library. Note the box is sometimes quite loaded (two make -j48 regtests going on at the same time), but there is enough memory. Is the library perhaps timing sensitive, e.g. that it would track issues only if the two threads are actually concurrent rather than could be concurrent? Say if the first pthread_create creates thread immediately, second pthread_create returns but the clone started thread isn't up yet, then pthread_join on the first thread finishes and the first thread is destroyed, then the second thread actually starts? Jakub
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 5:47 PM, H.J. Lu hjl.to...@gmail.com wrote: On Wed, Dec 4, 2013 at 8:41 AM, Ian Lance Taylor i...@google.com wrote: On Wed, Dec 4, 2013 at 8:04 AM, FX fxcoud...@gmail.com wrote: Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Well, you’re one of the maintainers of libsanitizer for GCC, so if you do not care about regressions in your code, it makes little sense for GCC (the whole project) to keep libsanitizer. I’ve posted this regression a month ago, it was not addressed. I’m not sure under what specific arrangement libsanitizer was added to GCC, but in general there is a responsibility of maintainers not to break bootstrap in their code. Yes, it’s a cost, and if you are not willing to do it, why did you contribute in the first place? Or is it a “hit and run” approach to maintainership? I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. I think libsanitizer should be disabled automatically if kernel or glibc are too old. BTW, fixincludes should fix the bad kernel header files from SuSE. Indeed - I'll give it a shot. Richard. -- H.J.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote: Expected ChangeLog entries: === libsanitizer/ChangeLog 2013-12-0X Kostya Serebryany k...@google.com * All source files: Merge from upstream r196090. * tsan/Makefile.am (tsan_files): Added new files. * tsan/Makefile.in: Regenerate. * sanitizer_common/Makefile.am (sanitizer_common_files): Added new fles. * sanitizer_common/Makefile.in: Regenerate. * lsan/Makefile.am (lsan_files): Added new files. * lsan/Makefile.in: Regenerate. === gcc/testsuite/ChangeLog 2013-12-0X Kostya Serebryany k...@google.com * c-c++-common/asan/null-deref-1.c: Update the test to match the fresh asan run-time. Ok. Please do another merge momentarily to bring in the ppc32 and CFI fixes and I'll then start working on some of the portability fixes outside of compiler-rt maintained files. Jakub
Re: libsanitizer merge from upstream r196090
On Thu, Dec 5, 2013 at 1:05 AM, Jeff Law l...@redhat.com wrote: On 12/03/13 22:08, Konstantin Serebryany wrote: We need a) patches that we can review and apply to the llvm repository (w/o breaking the modern systems, of course) b) a buildbot that would run 24/7 catching regressions. If we reach a green state for a platform X and have a buildbot for it, keeping it green will require relatively small effort. Every time we break it we will notice it in minutes and fix quickly while we still have the same context fresh. Fixing old systems once in few months during merge to gcc is costly because failures accumulate. I'm well overbooked already. However, if you have x86/x86_64 systems in your build farm that can be virtualized, Unfortunately we don't. In case anyone wants to provide their build machines and maintain a bot on them here are the instructions (Evgeniy and Alexey in CC may be able to help) --kcc I can help set up a suitable VM. CentOS 5.x is old enough to trigger lots of interesting problems, but is still in widespread use. Jeff
Re: libsanitizer merge from upstream r196090
On Thu, Dec 5, 2013 at 4:22 PM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Thu, Dec 5, 2013 at 1:05 AM, Jeff Law l...@redhat.com wrote: On 12/03/13 22:08, Konstantin Serebryany wrote: We need a) patches that we can review and apply to the llvm repository (w/o breaking the modern systems, of course) b) a buildbot that would run 24/7 catching regressions. If we reach a green state for a platform X and have a buildbot for it, keeping it green will require relatively small effort. Every time we break it we will notice it in minutes and fix quickly while we still have the same context fresh. Fixing old systems once in few months during merge to gcc is costly because failures accumulate. I'm well overbooked already. However, if you have x86/x86_64 systems in your build farm that can be virtualized, Unfortunately we don't. In case anyone wants to provide their build machines and maintain a bot on them here are the instructions Actual link: http://llvm.org/docs/HowToAddABuilder.html (Evgeniy and Alexey in CC may be able to help) --kcc I can help set up a suitable VM. CentOS 5.x is old enough to trigger lots of interesting problems, but is still in widespread use. Jeff
Re: libsanitizer merge from upstream r196090
The kernel and glibc check should be done at the toplevel. So what are the minimum kernel and glibc we want to support? For us, the versions we want to support are those that have green upstream buildbots and someone to keep them green. It's hard or impossible to set a more precise combination of kernel, glibc, binutils, arch, available RAM, or other environment requirements. libsanitizer is a very platform-dependent beast, much more so than most other code in GCC or LLVM. And it's a moving target since we keep adding more platform-dependent stuff, often quite unpleasant. Today we ourselves maintain the code (i.e. have green bots) for Ubuntu 12.04, fresh Mac OS, Windows, and Android (ARM), which makes the code more or less portable to other platforms. But we have no spare cycles to support anything else ourselves. --kcc
Re: libsanitizer merge from upstream r196090
On Wed, Dec 04, 2013 at 06:28:31AM +0100, Konstantin Serebryany wrote: We don't have any .cfi stuff in sanitizer_common (and I don't think we really need it in the internal_clone). Before fixing the tsan sources I'd like to see some indication that anyone cares about tsan working on older systems. Of course various parties care about that. But that doesn't necessarily imply they can or want to run buildbots for a different compiler they don't care about, there are e.g. security implications to that, resources etc. For GCC new ports etc. we never require people running some buildbots, people just report issues they encounter and those issues are fixed. tsan makes many assumptions about the system (address space, etc) which may not hold on old systems anyway. And tsan, even if it builds, will not work reliably. I really think that we need to disable libsanitizer on old systems until someone volunteers to set up a proper testing process upstream. If no one volunteers -- no one really needs it. The problem is that the definition of old systems is extremelly fuzzy, for you it is clearly != Ubuntu 12.04 or similar, but the unwritten assumptions libsanitizer makes are not related to one exact version of a single dependent component, it is a matter of compiler, compiler configuration, binutils, glibc, kernel, kernel headers, ... So, how do you disable libsanitizer on old systems when it is so fuzzy? There may be assumptions you just don't know about, and fixing bugreports by just adding reporter's toolchain combinations to kill lists is certainly not the way GCC is developed. Jakub
Re: libsanitizer merge from upstream r196090
Of course various parties care about that. But that doesn't necessarily imply they can or want to run buildbots for a different compiler they don't care about, there are e.g. security implications to that, resources etc. This brings us back to the initial issue: the way asanco are developed. The fact of life is that the development happens in the LLVM tree. The only strategy to keeping GCC's version in sync with upstream that works for us it to have verbatim copy of the sources in GCC and to have the merge process purely mechanical. It has not been purely mechanical with the last merge. Any other strategy would mean that someone else does the merges. This is totally fine as long as the sources do not diverge over time; but I afraid they will diverge. Testing asanco upstream does not necessary require testing the rest of the LLVM compiler. It would be really great to have someone test the entire LLVM tree on e.g. old Fedora, but we can limit such testing just to the compiler-rt project. Ideally for me, GCC would use sanitizer sources from compiler-rt completely verbatim, retaining the entire directory structure with all the tests and not adding any extra files inside that tree. Maybe even use svn external for greater simplicity. The GCC's build files (Makefile.am, etc) and GCC-specific tests would live outside of that source tree. This way it would be easy to set up a build bot that takes fresh compiler-rt, puts it into the GCC tree, and runs the GCC testing. The merges than will become purely mechanical: check the bots, put the new sources into gcc (or even update the svn external revision!). For GCC new ports etc. we never require people running some buildbots, people just report issues they encounter and those issues are fixed. tsan makes many assumptions about the system (address space, etc) which may not hold on old systems anyway. And tsan, even if it builds, will not work reliably. I really think that we need to disable libsanitizer on old systems until someone volunteers to set up a proper testing process upstream. If no one volunteers -- no one really needs it. The problem is that the definition of old systems is extremelly fuzzy, for you it is clearly != Ubuntu 12.04 or similar, but the unwritten assumptions libsanitizer makes are not related to one exact version of a single dependent component, it is a matter of compiler, compiler configuration, binutils, glibc, kernel, kernel headers, ... So, how do you disable libsanitizer on old systems when it is so fuzzy? I would start from kernel version and glibc version, this should cover the majority of use cases. --kcc There may be assumptions you just don't know about, and fixing bugreports by just adding reporter's toolchain combinations to kill lists is certainly not the way GCC is developed. Jakub
Re: libsanitizer merge from upstream r196090
On Wed, Dec 04, 2013 at 04:49:22PM +0400, Konstantin Serebryany wrote: I would start from kernel version and glibc version, this should cover the majority of use cases. Well, for the kernel headers what we perhaps can do is just add libsanitizer/include/linux/ tree that will be maintained by GCC and will contain (where needed) wrappers for kernel headers or their replacements to make sure things compile, if you don't care about it in the compiler-rt tree. But for the ppc32 stuff, we can't avoid modifying sanitizer_common (the first patch I've posted recently, btw, I wonder if it works on sparc*, we'll need to wait for somebody to test it), or e.g. for the .cfi_* stuff (if you just apply the the .cfi_* related part of the patch I've posted with say the macros __GCC_HAVE_* replaced by SANITIZER_USE_CFI_DIRECTIVES or similar, I guess we can provide the right definition for that outside of the compiler-rt maintained files. Regarding the TLS size, can you e.g. just only do it for glibc 2.13 and later, rather than having an (even for glibc 2.11/2.12 incorrect) values for older glibcs? Jakub
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 5:02 PM, Jakub Jelinek ja...@redhat.com wrote:
On Wed, Dec 04, 2013 at 04:49:22PM +0400, Konstantin Serebryany wrote:
I would start from kernel version and glibc version, this should cover
the majority of use cases.
Well, for the kernel headers what we perhaps can do is just add
libsanitizer/include/linux/ tree that will be maintained by GCC and will
if that works for you, no objections.
contain (where needed) wrappers for kernel headers or their replacements
to make sure things compile, if you don't care about it in the compiler-rt
tree. But for the ppc32 stuff, we can't avoid modifying sanitizer_common
(the first patch I've posted recently, btw, I wonder if it works on sparc*,
we'll need to wait for somebody to test it), or e.g. for the .cfi_* stuff
(if you just apply the the .cfi_* related part of the patch I've posted
with say the macros __GCC_HAVE_* replaced by SANITIZER_USE_CFI_DIRECTIVES
or similar, I guess we can provide the right definition for that outside of
the compiler-rt maintained files.
.cfi is used only in tsan sources now, and tsan is not supported
anywhere but x86_64
ppc32 never worked (last time I tried there were several different
issues so we disabled 32-bit build)
-- we should just disable it in GCC too. There is not value in
building code that does not run.
Regarding the TLS size, can you e.g. just only do it for glibc 2.13 and
later, rather than having an (even for glibc 2.11/2.12 incorrect) values for
older glibcs?
That would work for me, although it may bring some surprises later.
If we incorrectly compute the tls boundaries, lsan my produce false
positives or false negatives.
Having kThreadDescriptorSize=0 means that we include the stack
descriptor in the lsan's root set and thus
may miss a leak (with rather low probability). I can live with this.
Like this (tested only on my box)?
Index: sanitizer_linux_libcdep.cc
===
--- sanitizer_linux_libcdep.cc (revision 196375)
+++ sanitizer_linux_libcdep.cc (working copy)
@@ -207,12 +207,12 @@
#if defined(__x86_64__) || defined(__i386__)
// sizeof(struct thread) from glibc.
-// There has been a report of this being different on glibc 2.11 and 2.13. We
-// don't know when this change happened, so 2.14 is a conservative estimate.
-#if __GLIBC_PREREQ(2, 14)
+// This may change between glibc versions, we only support the versions we know
+// avout (= 2.13). For others we set kThreadDescriptorSize to 0.
+#if __GLIBC_PREREQ(2, 13)
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
#else
-const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
+const uptr kThreadDescriptorSize = 0; // Unknown.
#endif
uptr ThreadDescriptorSize() {
@@ -255,7 +255,7 @@
*stk_addr = stack_bottom;
*stk_size = stack_top - stack_bottom;
- if (!main) {
+ if (!main kThreadDescriptorSize) {
// If stack and tls intersect, make them non-intersecting.
if (*tls_addr *stk_addr *tls_addr *stk_addr + *stk_size) {
CHECK_GT(*tls_addr + *tls_size, *stk_addr);
Index: tests/sanitizer_linux_test.cc
===
--- tests/sanitizer_linux_test.cc (revision 196375)
+++ tests/sanitizer_linux_test.cc (working copy)
@@ -224,6 +224,7 @@
TEST(SanitizerLinux, ThreadDescriptorSize) {
pthread_t tid;
+ if (!ThreadDescriptorSize()) return;
void *result;
ASSERT_EQ(0, pthread_create(tid, 0, thread_descriptor_size_test_func, 0));
ASSERT_EQ(0, pthread_join(tid, result));
grumbling
If I had a buildbot with old Fedora, I would simply submit the
change and see if it broke/fixed it.
/grumbling
--kcc
Jakub
Re: libsanitizer merge from upstream r196090
On Wed, Dec 04, 2013 at 05:28:40PM +0400, Konstantin Serebryany wrote: Well, for the kernel headers what we perhaps can do is just add libsanitizer/include/linux/ tree that will be maintained by GCC and will if that works for you, no objections. I haven't tried to do that yet, so don't know how much work it will be, but at least from the second patch posted recently it it might work fine, at least for now. .cfi is used only in tsan sources now, and tsan is not supported anywhere but x86_64 But the .cfi_* issue is platform independent. Whether the compiler decides to emit them or not depends on how it was configured, on assembler and on compiler flags. I don't see how it can be a maintainance problem to just guard the few (right now two) .cfi_* occurrences in the C++ sources, or using CFI_* macros instead of .cfi_* directives directly in the assembly file. Other projects (e.g. glibc) manage to do that for years without any trouble. ppc32 never worked (last time I tried there were several different issues so we disabled 32-bit build) -- we should just disable it in GCC too. There is not value in building code that does not run. That doesn't mean it can't be made to work, and the patch I've posted is at least an (IMHO correct) step towards that. Note, I had just much bigger problems on ppc64 with the addr2line symbolization because of the ppc64 opd/plt stuff, though supposedly that might go away once I patch libsanitizer to use libbacktrace for symbolization. There is no inherent reason why ppc32 wouldn't work and ppc64 would, after all ppc64 with its weirdo function descriptor stuff is much harder to support. Regarding the TLS size, can you e.g. just only do it for glibc 2.13 and later, rather than having an (even for glibc 2.11/2.12 incorrect) values for older glibcs? That would work for me, although it may bring some surprises later. If we incorrectly compute the tls boundaries, lsan my produce false positives or false negatives. But is that solely for lsan and nothing else? Because, the assertion was failing in asan tests, without any asan options to request leak checking. And for non-i?86/x86_64 you ignore the tls boundaries too. Having kThreadDescriptorSize=0 means that we include the stack descriptor in the lsan's root set and thus may miss a leak (with rather low probability). I can live with this. Like this (tested only on my box)? --- sanitizer_linux_libcdep.cc (revision 196375) +++ sanitizer_linux_libcdep.cc (working copy) @@ -207,12 +207,12 @@ #if defined(__x86_64__) || defined(__i386__) // sizeof(struct thread) from glibc. -// There has been a report of this being different on glibc 2.11 and 2.13. We -// don't know when this change happened, so 2.14 is a conservative estimate. -#if __GLIBC_PREREQ(2, 14) +// This may change between glibc versions, we only support the versions we know +// avout (= 2.13). For others we set kThreadDescriptorSize to 0. +#if __GLIBC_PREREQ(2, 13) const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304); #else -const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304); +const uptr kThreadDescriptorSize = 0; // Unknown. Depends on (as I've asked earlier) on if you need the exact precise value or if say conservatively smaller value is fine. Then you could say for glibc = 2.5 pick the minimum of the values I've gathered. Jakub
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 5:44 PM, Jakub Jelinek ja...@redhat.com wrote: On Wed, Dec 04, 2013 at 05:28:40PM +0400, Konstantin Serebryany wrote: Well, for the kernel headers what we perhaps can do is just add libsanitizer/include/linux/ tree that will be maintained by GCC and will if that works for you, no objections. I haven't tried to do that yet, so don't know how much work it will be, but at least from the second patch posted recently it it might work fine, at least for now. .cfi is used only in tsan sources now, and tsan is not supported anywhere but x86_64 But the .cfi_* issue is platform independent. Whether the compiler decides to emit them or not depends on how it was configured, on assembler and on compiler flags. I don't see how it can be a maintainance problem to just guard the few (right now two) .cfi_* occurrences in the C++ sources, or using CFI_* macros instead of .cfi_* directives directly in the assembly file. Other projects (e.g. glibc) manage to do that for years without any trouble. replied separately. ppc32 never worked (last time I tried there were several different issues so we disabled 32-bit build) -- we should just disable it in GCC too. There is not value in building code that does not run. That doesn't mean it can't be made to work, and the patch I've posted is at least an (IMHO correct) step towards that. Sure it can. But all my previous grumbling about maintenance cost and our inability to test changes, etc applies here. Note, I had just much bigger problems on ppc64 with the addr2line symbolization because of the ppc64 opd/plt stuff, though supposedly that might go away once I patch libsanitizer to use libbacktrace for symbolization. There is no inherent reason why ppc32 wouldn't work and ppc64 would, after all ppc64 with its weirdo function descriptor stuff is much harder to support. Regarding the TLS size, can you e.g. just only do it for glibc 2.13 and later, rather than having an (even for glibc 2.11/2.12 incorrect) values for older glibcs? That would work for me, although it may bring some surprises later. If we incorrectly compute the tls boundaries, lsan my produce false positives or false negatives. But is that solely for lsan and nothing else? Mmm. I *think* yes, today this is lsan-only. Because, the assertion was failing in asan tests, without any asan options to request leak checking. And for non-i?86/x86_64 you ignore the tls boundaries too. My patch above should remove the assertion on 2.13 Having kThreadDescriptorSize=0 means that we include the stack descriptor in the lsan's root set and thus may miss a leak (with rather low probability). I can live with this. Like this (tested only on my box)? --- sanitizer_linux_libcdep.cc (revision 196375) +++ sanitizer_linux_libcdep.cc (working copy) @@ -207,12 +207,12 @@ #if defined(__x86_64__) || defined(__i386__) // sizeof(struct thread) from glibc. -// There has been a report of this being different on glibc 2.11 and 2.13. We -// don't know when this change happened, so 2.14 is a conservative estimate. -#if __GLIBC_PREREQ(2, 14) +// This may change between glibc versions, we only support the versions we know +// avout (= 2.13). For others we set kThreadDescriptorSize to 0. +#if __GLIBC_PREREQ(2, 13) const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304); #else -const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304); +const uptr kThreadDescriptorSize = 0; // Unknown. Depends on (as I've asked earlier) on if you need the exact precise value or if say conservatively smaller value is fine. Then you could say for glibc = 2.5 pick the minimum of the values I've gathered. precise is better, otherwise we may lose leaks. Jakub
Re: libsanitizer merge from upstream r196090
Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Well, you’re one of the maintainers of libsanitizer for GCC, so if you do not care about regressions in your code, it makes little sense for GCC (the whole project) to keep libsanitizer. I’ve posted this regression a month ago, it was not addressed. I’m not sure under what specific arrangement libsanitizer was added to GCC, but in general there is a responsibility of maintainers not to break bootstrap in their code. Yes, it’s a cost, and if you are not willing to do it, why did you contribute in the first place? Or is it a “hit and run” approach to maintainership? FX
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 8:04 AM, FX fxcoud...@gmail.com wrote: Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Well, you’re one of the maintainers of libsanitizer for GCC, so if you do not care about regressions in your code, it makes little sense for GCC (the whole project) to keep libsanitizer. I’ve posted this regression a month ago, it was not addressed. I’m not sure under what specific arrangement libsanitizer was added to GCC, but in general there is a responsibility of maintainers not to break bootstrap in their code. Yes, it’s a cost, and if you are not willing to do it, why did you contribute in the first place? Or is it a “hit and run” approach to maintainership? I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. Ian
Re: libsanitizer merge from upstream r196090
I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. You’re vastly better qualified than me to make this assessment, of course. My point is: unless someone (or multiple someones) is actually responsible for the thing, it cannot just work out of a sense of “someone should really do something about it”. The merge model of “we can break any target, except the single one we’re testing, every time we merge” seems poised for failure. FX
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 8:41 AM, Ian Lance Taylor i...@google.com wrote: On Wed, Dec 4, 2013 at 8:04 AM, FX fxcoud...@gmail.com wrote: Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Well, you’re one of the maintainers of libsanitizer for GCC, so if you do not care about regressions in your code, it makes little sense for GCC (the whole project) to keep libsanitizer. I’ve posted this regression a month ago, it was not addressed. I’m not sure under what specific arrangement libsanitizer was added to GCC, but in general there is a responsibility of maintainers not to break bootstrap in their code. Yes, it’s a cost, and if you are not willing to do it, why did you contribute in the first place? Or is it a “hit and run” approach to maintainership? I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. I think libsanitizer should be disabled automatically if kernel or glibc are too old. BTW, fixincludes should fix the bad kernel header files from SuSE. -- H.J.
Re: libsanitizer merge from upstream r196090
I think libsanitizer should be disabled automatically if kernel or glibc are too old. I think pretty much everyone agrees. But noone’s willing to put forward a patch, and so far the libsanitizer maintainers have failed to even document the requirements. (There are also binutils requirements, as I learnt the hard way.) FX
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 8:50 AM, FX fxcoud...@gmail.com wrote: I think libsanitizer should be disabled automatically if kernel or glibc are too old. I think pretty much everyone agrees. But noone’s willing to put forward a patch, What are the agreed-upon minimum kernel and glibc? I can give it a try. and so far the libsanitizer maintainers have failed to even document the requirements. (There are also binutils requirements, as I learnt the hard way.) What is the minimum binutils for libsanitizer? -- H.J.
Re: libsanitizer merge from upstream r196090
On Wed, Dec 04, 2013 at 08:47:41AM -0800, H.J. Lu wrote: I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. I think libsanitizer should be disabled automatically if kernel or glibc are too old. For very old I agree, I just strongly disagree with saying that anything older than a year and half is too old. So, as very old and unsupportable I'd probably consider e.g. Linux kernels without futex support, libsanitizer apparently uses those in various places and doesn't have a fallback. The question is how to do that though, because libraries are now disabled through lib*/configure.tgt UNSUPPORTED=1, and that is sourced in by toplevel configure, so any configure checks would need to be in toplevel configure. Or of course, we could in those cases configure the libsanitizer directory, but just decide not to build anything in there. Anyway, my preference right now would be if the ppc32 bits would be acceptable to Kostya (either by committing them upstream or just applying them as GCC local change for the time being), so that we don't break bootstrap on powerpc*-linux*, add those and commit the merge, then deal with the older kernel headers through include/linux subdirectory (I'll work on it), very old headers through configure, the CFI I hope Kostya would accept some macro, even if it is always enabled in the compiler-rt build and just GCC can disable .cfi_* addition if compiler doesn't use those, and then we can start fixing rest of portability issues. Jakub
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 8:58 AM, Jakub Jelinek ja...@redhat.com wrote: On Wed, Dec 04, 2013 at 08:47:41AM -0800, H.J. Lu wrote: I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. I think libsanitizer should be disabled automatically if kernel or glibc are too old. For very old I agree, I just strongly disagree with saying that anything older than a year and half is too old. So, as very old and unsupportable I'd probably consider e.g. Linux kernels without futex support, libsanitizer apparently uses those in various places and doesn't have a fallback. The question is how to do that though, because libraries are now disabled through lib*/configure.tgt UNSUPPORTED=1, and that is sourced in by toplevel configure, so any configure checks would need to be in toplevel configure. Or of course, we could in those cases configure the libsanitizer directory, but just decide not to build anything in there. The kernel and glibc check should be done at the toplevel. So what are the minimum kernel and glibc we want to support? -- H.J.
Re: libsanitizer merge from upstream r196090
On Wed, 4 Dec 2013, H.J. Lu wrote: The kernel and glibc check should be done at the toplevel. So what are the minimum kernel and glibc we want to support? Checking those at toplevel is tricky in general because you're checking something for the target rather than the host. You'd need to move the logic from gcc/configure.ac to compute target_header_dir and glibc_version_*, and GCC_GLIBC_VERSION_GTE_IFELSE from gcc/acinclude.m4, to something in toplevel config/ (and that logic depends on lots of other things in gcc/configure.ac). For binutils it's both easier to check (although the logic for binutils is also in gcc/acinclude.m4 at present) and more reasonable to require comparatively recent versions (for targets using binutils, which should cover everything supporting libsanitizer except Darwin) - I think there should be a minimum binutils version requirement generally when binutils is used with GCC, so we can reduce the need for conditionals on binutils features (unless of course the conditional code is still needed to support non-GNU assemblers and linkers for some target). It can be useful to build new tools for a target with old kernel and glibc in order to build binaries that will work on systems with a wide range of glibc versions. The oldest kernel and glibc versions I've used in that context with any post-4.3 GCC have been Linux 2.6.16 and glibc 2.4 (but the kernel headers were more recent than that, and this use case for old sysroots does *not* mean libsanitizer should necessarily be supported for them, simply that it's useful for the compiler and those libraries that may be used in production applications to be supported). If GCC were to desupport e.g. glibc before 2.4 you still get to deal with other libraries such as uClibc which pretends to be an older glibc (but again, you may well declare it unsupported for libsanitizer). -- Joseph S. Myers jos...@codesourcery.com
Re: libsanitizer merge from upstream r196090
On 12/03/13 22:28, Konstantin Serebryany wrote:
I really think that we need to disable libsanitizer on old systems
until someone volunteers to set up a proper testing process upstream.
If no one volunteers -- no one really needs it.
The right way to do this is to do feature tests rather than just
declaring something to be too old based on a version number. Using
version #s is a decent, but not great fall back (though it is often
easier to writing the appropriate feature tests).
Do you use autoconf upstream? If so, testing for things like the
existence of particular header files is trivial and avoids nonsense like
this entirely:
#if !SANITIZER_ANDROID
+#include linux/version.h
+// linux/perf_event.h has been added in 2.6.32
+#if LINUX_VERSION_CODE = 132640
#include linux/perf_event.h
#endif
+#endif
This kind of testing is so easy with autoconf that it's just silly.
namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
@@ -75,15 +79,18 @@ CHECK_SIZE_AND_OFFSET(io_event, res);
CHECK_SIZE_AND_OFFSET(io_event, res2);
#if !SANITIZER_ANDROID
+#if LINUX_VERSION_CODE = 132640
COMPILER_CHECK(sizeof(struct __sanitizer_perf_event_attr) =
sizeof(struct perf_event_attr));
CHECK_SIZE_AND_OFFSET(perf_event_attr, type);
CHECK_SIZE_AND_OFFSET(perf_event_attr, size);
#endif
+#endif
Couldn't this be done with a test as well. Given header files and the
ability to run the compiler, it should be fairly easy to test for this,
even in cross environments.
COMPILER_CHECK(iocb_cmd_pread == IOCB_CMD_PREAD);
COMPILER_CHECK(iocb_cmd_pwrite == IOCB_CMD_PWRITE);
-#if !SANITIZER_ANDROID
+#if !SANITIZER_ANDROID LINUX_VERSION_CODE = 132627
+// IOCB_CMD_PREADV/PWRITEV has been added in 2.6.19
COMPILER_CHECK(iocb_cmd_preadv == IOCB_CMD_PREADV);
COMPILER_CHECK(iocb_cmd_pwritev == IOCB_CMD_PWRITEV);
#endif
Also trivial to do with autoconf.
[ ... ]
But more generally, I'd look real closely at anything including linux/
headers directly and see if it can be reasonably avoided.The
ultimate result will actually be easier maintenance upstream over the
long term.
jeff
Re: libsanitizer merge from upstream r196090
On 12/03/13 22:08, Konstantin Serebryany wrote: We need a) patches that we can review and apply to the llvm repository (w/o breaking the modern systems, of course) b) a buildbot that would run 24/7 catching regressions. If we reach a green state for a platform X and have a buildbot for it, keeping it green will require relatively small effort. Every time we break it we will notice it in minutes and fix quickly while we still have the same context fresh. Fixing old systems once in few months during merge to gcc is costly because failures accumulate. I'm well overbooked already. However, if you have x86/x86_64 systems in your build farm that can be virtualized, I can help set up a suitable VM. CentOS 5.x is old enough to trigger lots of interesting problems, but is still in widespread use. Jeff
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 9:28 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 4 Dec 2013, H.J. Lu wrote: The kernel and glibc check should be done at the toplevel. So what are the minimum kernel and glibc we want to support? Checking those at toplevel is tricky in general because you're checking something for the target rather than the host. You'd need to move the logic from gcc/configure.ac to compute target_header_dir and glibc_version_*, and GCC_GLIBC_VERSION_GTE_IFELSE from gcc/acinclude.m4, to something in toplevel config/ (and that logic depends on lots of other things in gcc/configure.ac). I added config/gcc-setup.m4 and checked it into hjl/libsanitizer branch in git mirror. I moved logic from gcc/configure.ac to config/gcc-setup.m4. Toplevel configure can get the same info on glibc and kernel. We can set the minimum glibc and kernel. If they are too old, we can disable libsanitizer in toplevel configure. For binutils it's both easier to check (although the logic for binutils is also in gcc/acinclude.m4 at present) and more reasonable to require comparatively recent versions (for targets using binutils, which should cover everything supporting libsanitizer except Darwin) - I think there should be a minimum binutils version requirement generally when binutils is used with GCC, so we can reduce the need for conditionals on binutils features (unless of course the conditional code is still needed to support non-GNU assemblers and linkers for some target). We can move the binutils logic into config/gcc-setup.m4 and disable libsanitizer if binutils is too old. -- H.J.
Re: libsanitizer merge from upstream r196090
On Wed, Dec 4, 2013 at 8:58 PM, Jakub Jelinek ja...@redhat.com wrote: On Wed, Dec 04, 2013 at 08:47:41AM -0800, H.J. Lu wrote: I believe this is a case where the GCC project gets more benefit from libsanitizer than libsanitizer gets from being part of the GCC project. We should work with the libsanitizer developers to make this work, not just push everything back on them. I think libsanitizer should be disabled automatically if kernel or glibc are too old. For very old I agree, I just strongly disagree with saying that anything older than a year and half is too old. So, as very old and unsupportable I'd probably consider e.g. Linux kernels without futex support, libsanitizer apparently uses those in various places and doesn't have a fallback. The question is how to do that though, because libraries are now disabled through lib*/configure.tgt UNSUPPORTED=1, and that is sourced in by toplevel configure, so any configure checks would need to be in toplevel configure. Or of course, we could in those cases configure the libsanitizer directory, but just decide not to build anything in there. Anyway, my preference right now would be if the ppc32 bits would be acceptable to Kostya (either by committing them upstream or just applying them as GCC local change for the time being), Having GCC-local changes will make merges more painful in future, i.e. I will not be able to make them. I am ready to accept a ppc32 patch a) separately from other changes and b) such that it applies upstream. But long term we are not going to support platforms for which there are no public build bots upstream. so that we don't break bootstrap on powerpc*-linux*, add those and commit the merge, then deal with the older kernel headers through include/linux subdirectory (I'll work on it), very old headers through configure, the CFI I hope Kostya would accept Some kind of CFI support was just committed upstream, hopefully it works. http://llvm.org/viewvc/llvm-project?rev=196480view=rev --kcc some macro, even if it is always enabled in the compiler-rt build and just GCC can disable .cfi_* addition if compiler doesn't use those, and then we can start fixing rest of portability issues. Jakub
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 05:43:17PM +0100, Konstantin Serebryany wrote:
No, so your patch doesn't regress anything. I can configure with
--disable-libsanitizer to skip build of libsanitizer, although it
would be nice to support RHEL5 derived long-term distributions.
Ok, so this does not gate the merge.
Well, it regresses against 4.8, so it still is a P1 regression.
BTW, even the merge itself apparently regresses, powerpc* doesn't build any
longer and it did before this merge.
The first attached patch fixes the build on powerpc* (tested on RHEL6/ppc*)
and the second patch fixes the build on RHEL5/x86_64, beyond what I've
posted earlier the patch attempts to handle the .cfi* stuff (tried looking
if clang defines some usable macro, but couldn't find any, so no idea how
you can find out if you are compiled with clang -fno-dwarf2-cfi-asm
(when tsan will probably not build at all), or with the -fdwarf2-cfi-asm
default. Probably either you need to convince llvm folks to add something,
or add configure test, moved the linux/mroute* headers to the Linux specific
file so that linux/in.h stuff doesn't clash with netinet/in.h etc. and
finally hacks to avoid including linux/types.h at all costs, that header
historically has always been terrible portability minefield, as it defines
types that glibc headers define too.
With these two patches on top of your patch, I get libsanitizer to build
in multilib configurations on Fedora19/x86_64, RHEL6/ppc* and RHEL5/x86_64
(in all cases both 32-bit and 64-bit builds, but not the third x32 multilib).
Testsuite passes on Fedora19, on RHEL5/x86_64 tests that fail typically fail
on
==2738==AddressSanitizer CHECK failed:
../../../../libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:260
((*tls_addr + *tls_size)) = ((*stk_addr + *stk_size)) (0x2af8df1bc240,
0x2af8df1bc000)
which clearly is a bug in sanitizer_common,
#if defined(__x86_64__) || defined(__i386__)
// sizeof(struct thread) from glibc.
// There has been a report of this being different on glibc 2.11 and 2.13. We
// don't know when this change happened, so 2.14 is a conservative estimate.
#if __GLIBC_PREREQ(2, 14)
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
#else
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
#endif
uptr ThreadDescriptorSize() {
return kThreadDescriptorSize;
}
but also the InitTlsSize hack can't ever work reliably.
If you need this info, ask glibc folks for a proper supported API.
Hardcoding size of glibc private structure that glibc has changed multiple
times (note, RHEL5 has glibc 2.5 and clearly the size doesn't match there
even with the one you've computed on glibc 2.11) will most likely break any
time glibc adds further fields in there, not to mention that the above
means that if you e.g. build libsanitizer say on glibc 2.11 and run against
glibc 2.14, it will also not work. Plus calling _dl_get_tls_static_info,
which is a GLIBC_PRIVATE symbol, is also going to break whenever glibc
decides to change the ABI of that function.
I believe libthread_db.so.1 has some APIs to query sizeof (struct pthread),
but have never used those APIs so don't know how to query that.
My recommendation would be to just ifdef out this for now until you use
some sane reliable and supportable API. Otherwise, it may work if you are
lucky and always build libsanitizer against the exact same version of glibc
as you run it against, perhaps that is the only use model that llvm cares
about, but for GCC that is definitely not acceptable.
Jakub
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h.jj3
2013-12-03 03:33:20.0 -0500
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h
2013-12-03 05:26:41.864437661 -0500
@@ -140,23 +140,32 @@ namespace __sanitizer {
int gid;
int cuid;
int cgid;
-#ifdef __powerpc64__
+#ifdef __powerpc__
unsigned mode;
unsigned __seq;
+u64 __unused1;
+u64 __unused2;
#else
unsigned short mode;
unsigned short __pad1;
unsigned short __seq;
unsigned short __pad2;
+#if defined(__x86_64__) !defined(_LP64)
+u64 __unused1;
+u64 __unused2;
+#else
+unsigned long __unused1;
+unsigned long __unused2;
+#endif
#endif
-uptr __unused1;
-uptr __unused2;
};
struct __sanitizer_shmid_ds {
__sanitizer_ipc_perm shm_perm;
#ifndef __powerpc__
uptr shm_segsz;
+ #elif !defined(__powerpc64__)
+uptr __unused0;
#endif
uptr shm_atime;
#ifndef _LP64
@@ -288,17 +297,20 @@ namespace __sanitizer {
typedef long __sanitizer_clock_t;
#if SANITIZER_LINUX
-#if defined(_LP64) || defined(__x86_64__)
+#if defined(_LP64) || defined(__x86_64__) || defined(__powerpc__)
typedef unsigned __sanitizer___kernel_uid_t;
typedef unsigned __sanitizer___kernel_gid_t;
- typedef long long __sanitizer___kernel_off_t;
#else
typedef unsigned short __sanitizer___kernel_uid_t;
typedef unsigned short
Re: libsanitizer merge from upstream r196090
On Tue, Dec 3, 2013 at 3:50 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 02, 2013 at 05:43:17PM +0100, Konstantin Serebryany wrote:
No, so your patch doesn't regress anything. I can configure with
--disable-libsanitizer to skip build of libsanitizer, although it
would be nice to support RHEL5 derived long-term distributions.
Ok, so this does not gate the merge.
Well, it regresses against 4.8, so it still is a P1 regression.
Does anyone care?
Maintaining asanco on older platforms has a cost, and we are not
willing to pay it;
even reviewing patches like yours (still, thanks!) requires time.
The only long term strategy to support old systems is if someone works
closely with us in upstream
and we keep the code clean all the time.
If no one cares enough to do that, why should we?
I'll look at your second patch though.
BTW, even the merge itself apparently regresses, powerpc* doesn't build any
longer and it did before this merge.
The current llvm trunk builds on powerpc64 fine (just checked); we
build only the 64-bit variant.
I suppose that your patch fixes the 32-bit build.
It is broken beyond repair, I don't have any indication anyone ever used it.
Unless I hear otherwise I propose to disable 32-bit powerpc build.
64-bit variant is broken too (tests don't pass), but at least it builds.
If someone wants usable 32-bit powerpc they need to work with us upstream.
The first attached patch fixes the build on powerpc* (tested on RHEL6/ppc*)
and the second patch fixes the build on RHEL5/x86_64, beyond what I've
posted earlier the patch attempts to handle the .cfi* stuff (tried looking
if clang defines some usable macro, but couldn't find any, so no idea how
you can find out if you are compiled with clang -fno-dwarf2-cfi-asm
Yes, we will nee to find out some other macro to hide .CFI and such.
Maybe just something like SANITIZER_ALLOW_CFI_ASM or similar.
Again, we need to keep the code clean all the time in upstream,
otherwise gcc merges get too expensive.
(when tsan will probably not build at all), or with the -fdwarf2-cfi-asm
default. Probably either you need to convince llvm folks to add something,
or add configure test, moved the linux/mroute* headers to the Linux specific
file so that linux/in.h stuff doesn't clash with netinet/in.h etc. and
finally hacks to avoid including linux/types.h at all costs, that header
historically has always been terrible portability minefield, as it defines
types that glibc headers define too.
With these two patches on top of your patch, I get libsanitizer to build
in multilib configurations on Fedora19/x86_64, RHEL6/ppc* and RHEL5/x86_64
(in all cases both 32-bit and 64-bit builds, but not the third x32 multilib).
Testsuite passes on Fedora19, on RHEL5/x86_64 tests that fail typically fail
on
==2738==AddressSanitizer CHECK failed:
../../../../libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:260
((*tls_addr + *tls_size)) = ((*stk_addr + *stk_size)) (0x2af8df1bc240,
0x2af8df1bc000)
which clearly is a bug in sanitizer_common,
#if defined(__x86_64__) || defined(__i386__)
// sizeof(struct thread) from glibc.
// There has been a report of this being different on glibc 2.11 and 2.13. We
// don't know when this change happened, so 2.14 is a conservative estimate.
#if __GLIBC_PREREQ(2, 14)
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
#else
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
#endif
uptr ThreadDescriptorSize() {
return kThreadDescriptorSize;
}
but also the InitTlsSize hack can't ever work reliably.
If you need this info, ask glibc folks for a proper supported API.
This won't happen any time soon, right?
I'd like to ask glibc for many other things, not just this, but the latency
of the glibc changes propagating to users is too low, so we don't
bother (although we should)
E.g. we've been hit by the ugly
pthread_getattr_np/pthread_attr_getstack multiple times.
:(
Hardcoding size of glibc private structure that glibc has changed multiple
times (note, RHEL5 has glibc 2.5 and clearly the size doesn't match there
even with the one you've computed on glibc 2.11) will most likely break any
time glibc adds further fields in there, not to mention that the above
means that if you e.g. build libsanitizer say on glibc 2.11 and run against
glibc 2.14, it will also not work. Plus calling _dl_get_tls_static_info,
which is a GLIBC_PRIVATE symbol, is also going to break whenever glibc
decides to change the ABI of that function.
I believe libthread_db.so.1 has some APIs to query sizeof (struct pthread),
but have never used those APIs so don't know how to query that.
I think one of us tried that with not success.
My recommendation would be to just ifdef out this for now until you use
some sane reliable and supportable API. Otherwise, it may work if you are
lucky and always build libsanitizer against the exact same version of glibc
as you run it against,
Re: libsanitizer merge from upstream r196090
On Tue, Dec 03, 2013 at 07:18:14PM +0400, Konstantin Serebryany wrote:
No, so your patch doesn't regress anything. I can configure with
--disable-libsanitizer to skip build of libsanitizer, although it
would be nice to support RHEL5 derived long-term distributions.
Ok, so this does not gate the merge.
Well, it regresses against 4.8, so it still is a P1 regression.
Does anyone care?
Of course. First of all all users trying to compile gcc just to find out
it won't build out of the box. Also users that were using asan just fine
on older platforms in gcc 4.8 and now they'll find out that the support
has been dropped.
Maintaining asanco on older platforms has a cost, and we are not
willing to pay it;
Well, but then you just get approximately same cost if not higher in
maintaining configure bits for checking all the silent assumptions the code
makes and disabling libsanitizer in that case.
As you can see in the patches I've posted, most of the issues are in the
headers which you should be able to test the way I've posted yesterday, just
unpack a couple of .../usr/include trees from various distros. The only
ones not found by that are the .cfi_* stuff, you can test it by building
with -fno-dwarf2-cfi-asm and see whether it still builds, and
the tls size issue, which needs some solution in any case, because it is
just totally broken. Really ask the glibc folks for an API or talk to them
how to query it using libthread_db.so.1, struct pthread's size changes
frequently.
BTW, even the merge itself apparently regresses, powerpc* doesn't build any
longer and it did before this merge.
The current llvm trunk builds on powerpc64 fine (just checked); we
build only the 64-bit variant.
I suppose that your patch fixes the 32-bit build.
It is broken beyond repair, I don't have any indication anyone ever used it.
Unless I hear otherwise I propose to disable 32-bit powerpc build.
64-bit variant is broken too (tests don't pass), but at least it builds.
If someone wants usable 32-bit powerpc they need to work with us upstream.
Why do you think it would be broken beyond repair?
The first attached patch fixes the build on powerpc* (tested on RHEL6/ppc*)
and the second patch fixes the build on RHEL5/x86_64, beyond what I've
posted earlier the patch attempts to handle the .cfi* stuff (tried looking
if clang defines some usable macro, but couldn't find any, so no idea how
you can find out if you are compiled with clang -fno-dwarf2-cfi-asm
Yes, we will nee to find out some other macro to hide .CFI and such.
Maybe just something like SANITIZER_ALLOW_CFI_ASM or similar.
Again, we need to keep the code clean all the time in upstream,
otherwise gcc merges get too expensive.
Sure, just invent a macro for it and use it everywhere, for GCC that macro
can be defined based on whether __GCC_HAVE_DWARF2_CFI_ASM is defined, for
llvm/clang configure or whatever else.
#if defined(__x86_64__) || defined(__i386__)
// sizeof(struct thread) from glibc.
// There has been a report of this being different on glibc 2.11 and 2.13.
We
// don't know when this change happened, so 2.14 is a conservative estimate.
#if __GLIBC_PREREQ(2, 14)
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
#else
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
#endif
uptr ThreadDescriptorSize() {
return kThreadDescriptorSize;
}
but also the InitTlsSize hack can't ever work reliably.
If you need this info, ask glibc folks for a proper supported API.
This won't happen any time soon, right?
I'd like to ask glibc for many other things, not just this, but the latency
of the glibc changes propagating to users is too low, so we don't
bother (although we should)
E.g. we've been hit by the ugly
pthread_getattr_np/pthread_attr_getstack multiple times.
:(
If you never ask for it, it will never be done, it is that simple.
Anyway, does the code rely on exactly the right size of struct pthread, or
is a conservative minimum fine? Perhaps it is just a matter of adding
another case, if you tested say glibc 2.11, just don't do anything at all
for older glibcs (i.e. the same thing as for non-i?86/x86_64) - tls size 0.
Jakub
Re: libsanitizer merge from upstream r196090
On 12/03/13 08:47, Jakub Jelinek wrote: On Tue, Dec 03, 2013 at 07:18:14PM +0400, Konstantin Serebryany wrote: No, so your patch doesn't regress anything. I can configure with --disable-libsanitizer to skip build of libsanitizer, although it would be nice to support RHEL5 derived long-term distributions. Ok, so this does not gate the merge. Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Of course. First of all all users trying to compile gcc just to find out it won't build out of the box. Also users that were using asan just fine on older platforms in gcc 4.8 and now they'll find out that the support has been dropped. Right. We certainly care. Those old platforms are still in widespread use (for better or worse). Maintaining asanco on older platforms has a cost, and we are not willing to pay it; Well, but then you just get approximately same cost if not higher in maintaining configure bits for checking all the silent assumptions the code makes and disabling libsanitizer in that case. Right. Fixing this problem and fixing it correctly will reduce the long term pain for both projects. This won't happen any time soon, right? I'd like to ask glibc for many other things, not just this, but the latency of the glibc changes propagating to users is too low, so we don't bother (although we should) E.g. we've been hit by the ugly pthread_getattr_np/pthread_attr_getstack multiple times. :( If you never ask for it, it will never be done, it is that simple. glibc pushes out a release every six months which then gets put into the next Fedora release which usually follows a few months later. We're talking about a worst case lag time of roughly 9 months, often much less. It's probably similar for the SuSE community releases. I think everyone who has had a bad experience working with glibc's team in the past should try to put it behind them. The new team running glibc is much more reasonable to work with -- the biggest problem now is rebooting the project after years of mis-management. The progress they've made towards that over the last year has been tremendous. jeff
Re: libsanitizer merge from upstream r196090
On Tue, Dec 03, 2013 at 07:18:14PM +0400, Konstantin Serebryany wrote:
==2738==AddressSanitizer CHECK failed:
../../../../libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cc:260
((*tls_addr + *tls_size)) = ((*stk_addr + *stk_size)) (0x2af8df1bc240,
0x2af8df1bc000)
which clearly is a bug in sanitizer_common,
#if defined(__x86_64__) || defined(__i386__)
// sizeof(struct thread) from glibc.
// There has been a report of this being different on glibc 2.11 and 2.13.
We
// don't know when this change happened, so 2.14 is a conservative estimate.
#if __GLIBC_PREREQ(2, 14)
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1216, 2304);
#else
const uptr kThreadDescriptorSize = FIRST_32_SECOND_64(1168, 2304);
#endif
BTW, just to fill in some of the missing data from a couple of glibcs:
glibc 2.3.6 FIRST_32_SECOND_64(1104, 1696)
glibc 2.4 FIRST_32_SECOND_64(1120, 1728)
glibc 2.5 FIRST_32_SECOND_64(1136, 1728)
glibc 2.6, 2.7, 2.8, 2.9FIRST_32_SECOND_64(1136, 1712)
glibc 2.10.1FIRST_32_SECOND_64(1168, 1776)
glibc 2.11.1, 2.12 FIRST_32_SECOND_64(1168, 2288)
glibc 2.13, 2.14.1, 2.15, 2.17 FIRST_32_SECOND_64(1216, 2304)
script to extract the data was:
mkdir /tmp/aa; cd /tmp/aa; for i in /tmp/glibc-2.*; do echo $i; rm -rf
/tmp/aa/*; rpm2cpio $i | cpio -id; readelf -Ws lib*/libpthread-2.*.so | grep
'_thread_db_sizeof_pthread$' | awk '{print $2}'; j=`readelf -Ws
lib*/libpthread-2.*.so | grep '_thread_db_sizeof_pthread$' | awk '{print $2}' |
sed 's/[48c]$/0/;s/^00*//'`; objdump -s -j .rodata lib*/libpthread-2.*.so |
grep $j; done
So, as the data shows the numbers aren't even always monotonically increasing.
Jakub
Re: libsanitizer merge from upstream r196090
This won't happen any time soon, right? I'd like to ask glibc for many other things, not just this, but the latency of the glibc changes propagating to users is too low, so we don't bother (although we should) E.g. we've been hit by the ugly pthread_getattr_np/pthread_attr_getstack multiple times. :( If you never ask for it, it will never be done, it is that simple. glibc pushes out a release every six months which then gets put into the next Fedora release which usually follows a few months later. We're talking about a worst case lag time of roughly 9 months, often much less. It's probably similar for the SuSE community releases. I think everyone who has had a bad experience working with glibc's team in the past should try to put it behind them. The new team running glibc is much more reasonable to work with -- the biggest problem now is rebooting the project after years of mis-management. The progress they've made towards that over the last year has been tremendous. Submitted a feature request against glibc https://sourceware.org/bugzilla/show_bug.cgi?id=16291 Please comment there if you think I did not provide enough information. --kcc
Re: libsanitizer merge from upstream r196090
On Tue, Dec 3, 2013 at 5:42 PM, Jeff Law l...@redhat.com wrote: On 12/03/13 08:47, Jakub Jelinek wrote: On Tue, Dec 03, 2013 at 07:18:14PM +0400, Konstantin Serebryany wrote: No, so your patch doesn't regress anything. I can configure with --disable-libsanitizer to skip build of libsanitizer, although it would be nice to support RHEL5 derived long-term distributions. Ok, so this does not gate the merge. Well, it regresses against 4.8, so it still is a P1 regression. Does anyone care? Of course. First of all all users trying to compile gcc just to find out it won't build out of the box. Also users that were using asan just fine on older platforms in gcc 4.8 and now they'll find out that the support has been dropped. Right. We certainly care. Those old platforms are still in widespread use (for better or worse). So, would you be able to help us upstream? We need a) patches that we can review and apply to the llvm repository (w/o breaking the modern systems, of course) b) a buildbot that would run 24/7 catching regressions. If we reach a green state for a platform X and have a buildbot for it, keeping it green will require relatively small effort. Every time we break it we will notice it in minutes and fix quickly while we still have the same context fresh. Fixing old systems once in few months during merge to gcc is costly because failures accumulate. --kcc Maintaining asanco on older platforms has a cost, and we are not willing to pay it; Well, but then you just get approximately same cost if not higher in maintaining configure bits for checking all the silent assumptions the code makes and disabling libsanitizer in that case. Right. Fixing this problem and fixing it correctly will reduce the long term pain for both projects. This won't happen any time soon, right? I'd like to ask glibc for many other things, not just this, but the latency of the glibc changes propagating to users is too low, so we don't bother (although we should) E.g. we've been hit by the ugly pthread_getattr_np/pthread_attr_getstack multiple times. :( If you never ask for it, it will never be done, it is that simple. glibc pushes out a release every six months which then gets put into the next Fedora release which usually follows a few months later. We're talking about a worst case lag time of roughly 9 months, often much less. It's probably similar for the SuSE community releases. I think everyone who has had a bad experience working with glibc's team in the past should try to put it behind them. The new team running glibc is much more reasonable to work with -- the biggest problem now is rebooting the project after years of mis-management. The progress they've made towards that over the last year has been tremendous. jeff
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 11:32 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 02, 2013 at 05:59:53PM +0100, Konstantin Serebryany wrote:
with #if LINUX_VERSION_CODE = 132640
Good idea, let me try that.
Had a quick look at this on RHEL 5.
Following patch let me compile at least the first source file, but then
I run into tons of issues in sanitizer_platform_limits_posix.cc.
I think the main problem is that you are mixing standard glibc headers and
linux kernel headers in the same source file, that is a big no no.
Lots of the kernel headers declare the same things as glibc headers.
I'd strongly recommend splitting the files, so that you include absolute
minimum of
glibc headers when you include linux/* and/or asm/* headers and no kernel
headers
if you include tons of glibc headers.
And as the errors show up, there are also .cfi* directives that are used
unconditionally (you've set you've removed it from sanitizer_common or where
it
was used (IMHO a pitty, much better would be conditionalizing them on either
compiler
preprocessor macros or whatever clang provides as alternative for that when
not building
with gcc)), but they are used in tsan (in HACKY_CALL macro). Plus in *.S file
(either that could be again guarded by the same preprocessor macro, or
configure or
something else). Note that RHEL5 here has already gas that supports .cfi_*
directives
(just not .cfi_personality/.cfi_lsda I think), but if you go to even older
system
it will not be there. E.g. glibc assembler files solve that by defining
various
CFI_STARTPROC etc. macros that either expand to .cfi_startproc etc. if
assembler
supports the directives, or nothing otherwise.
We don't have any .cfi stuff in sanitizer_common (and I don't think we
really need it in the internal_clone).
Before fixing the tsan sources I'd like to see some indication that
anyone cares about tsan working on older systems.
tsan makes many assumptions about the system (address space, etc)
which may not hold on old systems anyway.
And tsan, even if it builds, will not work reliably.
I really think that we need to disable libsanitizer on old systems
until someone volunteers to set up a proper testing process upstream.
If no one volunteers -- no one really needs it.
--kcc
--- sanitizer_platform_limits_linux.cc.jj 2013-12-02 15:27:58.0
-0500
+++ sanitizer_platform_limits_linux.cc 2013-12-02 17:06:19.0 -0500
@@ -51,8 +51,12 @@
#endif
#if !SANITIZER_ANDROID
+#include linux/version.h
+// linux/perf_event.h has been added in 2.6.32
+#if LINUX_VERSION_CODE = 132640
#include linux/perf_event.h
#endif
+#endif
namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
@@ -75,15 +79,18 @@ CHECK_SIZE_AND_OFFSET(io_event, res);
CHECK_SIZE_AND_OFFSET(io_event, res2);
#if !SANITIZER_ANDROID
+#if LINUX_VERSION_CODE = 132640
COMPILER_CHECK(sizeof(struct __sanitizer_perf_event_attr) =
sizeof(struct perf_event_attr));
CHECK_SIZE_AND_OFFSET(perf_event_attr, type);
CHECK_SIZE_AND_OFFSET(perf_event_attr, size);
#endif
+#endif
COMPILER_CHECK(iocb_cmd_pread == IOCB_CMD_PREAD);
COMPILER_CHECK(iocb_cmd_pwrite == IOCB_CMD_PWRITE);
-#if !SANITIZER_ANDROID
+#if !SANITIZER_ANDROID LINUX_VERSION_CODE = 132627
+// IOCB_CMD_PREADV/PWRITEV has been added in 2.6.19
COMPILER_CHECK(iocb_cmd_preadv == IOCB_CMD_PREADV);
COMPILER_CHECK(iocb_cmd_pwritev == IOCB_CMD_PWRITEV);
#endif
--- sanitizer_platform_limits_posix.cc.jj 2013-12-02 15:27:58.0
-0500
+++ sanitizer_platform_limits_posix.cc 2013-12-02 17:11:00.0 -0500
@@ -82,12 +82,16 @@
#include sys/timex.h
#include sys/user.h
#include sys/ustat.h
+#include linux/version.h
#include linux/cyclades.h
#include linux/if_eql.h
#include linux/if_plip.h
#include linux/lp.h
#include linux/mroute.h
+// linux/mroute6.h has been added in 2.6.26
+#if LINUX_VERSION_CODE = 132634
#include linux/mroute6.h
+#endif
#include linux/scc.h
#include linux/serial.h
#include sys/msg.h
So the current errors are (from make -j64 -k to show more than one file):
In file included from /usr/include/sys/ustat.h:30:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:84:
/usr/include/bits/ustat.h:25:8: error: redefinition of ‘struct ustat’
struct ustat
^
In file included from /usr/include/linux/if_ether.h:24:0,
from /usr/include/netinet/if_ether.h:26,
from /usr/include/netinet/ether.h:26,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:47:
/usr/include/linux/types.h:156:8: error: previous definition of ‘struct ustat’
struct ustat {
^
In file included from /usr/include/linux/mroute.h:5:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:90:
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 3:52 AM, Konstantin Serebryany
konstantin.s.serebry...@gmail.com wrote:
Another libsanitizer merge from upstream, r196090
(needs attention on ubsan side)
This hopefully fixes various build failures on non-x86-linux platforms,
although I still tested it only on our x86_64 Linux Ubuntu 12.04 box:
rm -rf */{*/,}libsanitizer make -j 50 make -C gcc
check-g{cc,++} RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}
asan.exp'
This also fixed tsan; verified on a single small test.
This change breaks one ubsan test:
make check -C gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp'
FAIL: c-c++-common/ubsan/vla-1.c -O0 execution test
I am asking gcc-ubsan maintainers to help me decipher dejagnu
diagnostics and fix the test failure.
I am ready to make any reasonable additional testing that
can be done on x86_64 Linux Ubuntu 12.04.
The upstream bots on Mac and Linux (x86, x86_64, ARM) are green.
We also verified that the upstream source builds (but does not really
work) on PowerPC64
Expected ChangeLog entries:
=== libsanitizer/ChangeLog
2013-12-0X Kostya Serebryany k...@google.com
* All source files: Merge from upstream r196090.
* tsan/Makefile.am (tsan_files): Added new files.
* tsan/Makefile.in: Regenerate.
* sanitizer_common/Makefile.am (sanitizer_common_files): Added new
fles.
* sanitizer_common/Makefile.in: Regenerate.
* lsan/Makefile.am (lsan_files): Added new files.
* lsan/Makefile.in: Regenerate.
=== gcc/testsuite/ChangeLog
2013-12-0X Kostya Serebryany k...@google.com
* c-c++-common/asan/null-deref-1.c: Update the test
to match the fresh asan run-time.
--kcc
Does it support using libbacktrace in GCC?
--
H.J.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 05:13:45AM -0800, H.J. Lu wrote: === libsanitizer/ChangeLog 2013-12-0X Kostya Serebryany k...@google.com * All source files: Merge from upstream r196090. * tsan/Makefile.am (tsan_files): Added new files. * tsan/Makefile.in: Regenerate. * sanitizer_common/Makefile.am (sanitizer_common_files): Added new fles. * sanitizer_common/Makefile.in: Regenerate. * lsan/Makefile.am (lsan_files): Added new files. * lsan/Makefile.in: Regenerate. === gcc/testsuite/ChangeLog 2013-12-0X Kostya Serebryany k...@google.com * c-c++-common/asan/null-deref-1.c: Update the test to match the fresh asan run-time. --kcc Does it support using libbacktrace in GCC? Not on it's own, but the support in the upstream maintained files is there, so hopefully it will be just a matter of follow-up patch with configury/Makefile etc. stuff, I'll work on it once the merge is committed. What is more important now is to test the patch Kostya posted on non-x86_64 targets and/or older kernel headers (say RHEL5, older SLES, etc.). Jakub
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote:
This change breaks one ubsan test:
make check -C gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp'
FAIL: c-c++-common/ubsan/vla-1.c -O0 execution test
I am asking gcc-ubsan maintainers to help me decipher dejagnu
diagnostics and fix the test failure.
Ok, reproduced. I'll look into it.
Marek
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 02:41:05PM +0100, Marek Polacek wrote:
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote:
This change breaks one ubsan test:
make check -C gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp'
FAIL: c-c++-common/ubsan/vla-1.c -O0 execution test
I am asking gcc-ubsan maintainers to help me decipher dejagnu
diagnostics and fix the test failure.
Ok, reproduced. I'll look into it.
Well, this should help. The problem is that the testcase, when run,
SIGSEGVed, but since we're doing Ugly Things (VLAs with negative
size), it of course _can_ segfault, we're just relying that it
doesn't.
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
index 3e47bd3..1c5d14a 100644
--- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
+++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
@@ -13,7 +13,7 @@ main (void)
{
int x = -1;
double di = -3.2;
- V v = -666;
+ V v = -6;
int a[x];
int aa[x][x];
@@ -44,5 +44,5 @@ main (void)
/* { dg-output \[^\n\r]*variable length array bound evaluates to non-positive
value 0(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to non-positive
value -1(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to non-positive
value -1(\n|\r\n|\r) } */
-/* { dg-output \[^\n\r]*variable length array bound evaluates to non-positive
value -666(\n|\r\n|\r) } */
+/* { dg-output \[^\n\r]*variable length array bound evaluates to non-positive
value -6(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to non-positive
value -42(\n|\r\n|\r) } */
Marek
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 6:41 PM, Marek Polacek pola...@redhat.com wrote:
On Mon, Dec 02, 2013 at 02:41:05PM +0100, Marek Polacek wrote:
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote:
This change breaks one ubsan test:
make check -C gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp'
FAIL: c-c++-common/ubsan/vla-1.c -O0 execution test
I am asking gcc-ubsan maintainers to help me decipher dejagnu
diagnostics and fix the test failure.
Ok, reproduced. I'll look into it.
Well, this should help. The problem is that the testcase, when run,
SIGSEGVed, but since we're doing Ugly Things (VLAs with negative
size), it of course _can_ segfault, we're just relying that it
doesn't.
Thanks!
Shall I add this change to mine, or you want to commit it separately?
An alternative and more stable fix would be to rewrite the test to run
each case independently
and fail after the report, but that's up to you.
--kcc
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
index 3e47bd3..1c5d14a 100644
--- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
+++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
@@ -13,7 +13,7 @@ main (void)
{
int x = -1;
double di = -3.2;
- V v = -666;
+ V v = -6;
int a[x];
int aa[x][x];
@@ -44,5 +44,5 @@ main (void)
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value 0(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -1(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -1(\n|\r\n|\r) } */
-/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -666(\n|\r\n|\r) } */
+/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -6(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -42(\n|\r\n|\r) } */
Marek
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 03:41:18PM +0100, Marek Polacek wrote:
On Mon, Dec 02, 2013 at 02:41:05PM +0100, Marek Polacek wrote:
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote:
This change breaks one ubsan test:
make check -C gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}
ubsan.exp'
FAIL: c-c++-common/ubsan/vla-1.c -O0 execution test
I am asking gcc-ubsan maintainers to help me decipher dejagnu
diagnostics and fix the test failure.
Ok, reproduced. I'll look into it.
Well, this should help. The problem is that the testcase, when run,
SIGSEGVed, but since we're doing Ugly Things (VLAs with negative
size), it of course _can_ segfault, we're just relying that it
doesn't.
Suppossedly it might be better to split the main from the test into multiple
functions, with __attribute__((noinline)) and just one invalid VLA in each.
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
index 3e47bd3..1c5d14a 100644
--- a/gcc/testsuite/c-c++-common/ubsan/vla-1.c
+++ b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
@@ -13,7 +13,7 @@ main (void)
{
int x = -1;
double di = -3.2;
- V v = -666;
+ V v = -6;
int a[x];
int aa[x][x];
@@ -44,5 +44,5 @@ main (void)
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value 0(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -1(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -1(\n|\r\n|\r) } */
-/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -666(\n|\r\n|\r) } */
+/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -6(\n|\r\n|\r) } */
/* { dg-output \[^\n\r]*variable length array bound evaluates to
non-positive value -42(\n|\r\n|\r) } */
Jakub
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 03:47:18PM +0100, Jakub Jelinek wrote:
On Mon, Dec 02, 2013 at 03:41:18PM +0100, Marek Polacek wrote:
On Mon, Dec 02, 2013 at 02:41:05PM +0100, Marek Polacek wrote:
On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote:
This change breaks one ubsan test:
make check -C gcc RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}
ubsan.exp'
FAIL: c-c++-common/ubsan/vla-1.c -O0 execution test
I am asking gcc-ubsan maintainers to help me decipher dejagnu
diagnostics and fix the test failure.
Ok, reproduced. I'll look into it.
Well, this should help. The problem is that the testcase, when run,
SIGSEGVed, but since we're doing Ugly Things (VLAs with negative
size), it of course _can_ segfault, we're just relying that it
doesn't.
Suppossedly it might be better to split the main from the test into multiple
functions, with __attribute__((noinline)) and just one invalid VLA in each.
Okay, I'll do this separately.
So, Kostya, I guess just do the merge and I'll take care of that ubsan
fail.
Marek
Re: libsanitizer merge from upstream r196090
Hello! Does it support using libbacktrace in GCC? Not on it's own, but the support in the upstream maintained files is there, so hopefully it will be just a matter of follow-up patch with configury/Makefile etc. stuff, I'll work on it once the merge is committed. What is more important now is to test the patch Kostya posted on non-x86_64 targets and/or older kernel headers (say RHEL5, older SLES, etc.). Unfortunately, the build breaks on CentOS 5.10 (= RHEL5) with: libtool: compile: /home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc -B/home/uros/gcc-build-xxx/./gcc -nostdinc++ -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I. -I../../../../gcc-svn/trunk/libsanitizer/sanitizer_common -I ../../../../gcc-svn/trunk/libsanitizer/include -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -I../../libstdc++-v3/include -I../../libstdc++-v3/include/x86_64-unknown-linux-gnu -I../../../../gcc-svn/trunk/libsanitizer/../libstdc++-v3/libsupc++ -g -O2 -D_GNU_SOURCE -MT sanitizer_platform_limits_linux.lo -MD -MP -MF .deps/sanitizer_platform_limits_linux.Tpo -c ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc -fPIC -DPIC -o .libs/sanitizer_platform_limits_linux.o ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:54:30: fatal error: linux/perf_event.h: No such file or directory #include linux/perf_event.h ^ compilation terminated. gmake[4]: *** [sanitizer_platform_limits_linux.lo] Error 1 gmake[4]: *** Waiting for unfinished jobs Uros.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 4:49 PM, Uros Bizjak ubiz...@gmail.com wrote: Hello! Does it support using libbacktrace in GCC? Not on it's own, but the support in the upstream maintained files is there, so hopefully it will be just a matter of follow-up patch with configury/Makefile etc. stuff, I'll work on it once the merge is committed. What is more important now is to test the patch Kostya posted on non-x86_64 targets and/or older kernel headers (say RHEL5, older SLES, etc.). Unfortunately, the build breaks on CentOS 5.10 (= RHEL5) with: libtool: compile: /home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc -B/home/uros/gcc-build-xxx/./gcc -nostdinc++ -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I. -I../../../../gcc-svn/trunk/libsanitizer/sanitizer_common -I ../../../../gcc-svn/trunk/libsanitizer/include -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -I../../libstdc++-v3/include -I../../libstdc++-v3/include/x86_64-unknown-linux-gnu -I../../../../gcc-svn/trunk/libsanitizer/../libstdc++-v3/libsupc++ -g -O2 -D_GNU_SOURCE -MT sanitizer_platform_limits_linux.lo -MD -MP -MF .deps/sanitizer_platform_limits_linux.Tpo -c ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc -fPIC -DPIC -o .libs/sanitizer_platform_limits_linux.o ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:54:30: fatal error: linux/perf_event.h: No such file or directory #include linux/perf_event.h Sounds familiar. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59068 Do things work for you w/o my patch in fresh trunk? Is there a way to test gcc in such environment w/o setting up VMs (e.g. chroot, or some such)? --kcc ^ compilation terminated. gmake[4]: *** [sanitizer_platform_limits_linux.lo] Error 1 gmake[4]: *** Waiting for unfinished jobs Uros.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 5:12 PM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: Does it support using libbacktrace in GCC? Not on it's own, but the support in the upstream maintained files is there, so hopefully it will be just a matter of follow-up patch with configury/Makefile etc. stuff, I'll work on it once the merge is committed. What is more important now is to test the patch Kostya posted on non-x86_64 targets and/or older kernel headers (say RHEL5, older SLES, etc.). Unfortunately, the build breaks on CentOS 5.10 (= RHEL5) with: libtool: compile: /home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc -B/home/uros/gcc-build-xxx/./gcc -nostdinc++ -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I. -I../../../../gcc-svn/trunk/libsanitizer/sanitizer_common -I ../../../../gcc-svn/trunk/libsanitizer/include -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -I../../libstdc++-v3/include -I../../libstdc++-v3/include/x86_64-unknown-linux-gnu -I../../../../gcc-svn/trunk/libsanitizer/../libstdc++-v3/libsupc++ -g -O2 -D_GNU_SOURCE -MT sanitizer_platform_limits_linux.lo -MD -MP -MF .deps/sanitizer_platform_limits_linux.Tpo -c ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc -fPIC -DPIC -o .libs/sanitizer_platform_limits_linux.o ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:54:30: fatal error: linux/perf_event.h: No such file or directory #include linux/perf_event.h Sounds familiar. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59068 Do things work for you w/o my patch in fresh trunk? No, so your patch doesn't regress anything. I can configure with --disable-libsanitizer to skip build of libsanitizer, although it would be nice to support RHEL5 derived long-term distributions. Is there a way to test gcc in such environment w/o setting up VMs (e.g. chroot, or some such)? Maybe gcc compile farm has linux-2.6.18 machine available? Uros.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 5:26 PM, Uros Bizjak ubiz...@gmail.com wrote: On Mon, Dec 2, 2013 at 5:12 PM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: Does it support using libbacktrace in GCC? Not on it's own, but the support in the upstream maintained files is there, so hopefully it will be just a matter of follow-up patch with configury/Makefile etc. stuff, I'll work on it once the merge is committed. What is more important now is to test the patch Kostya posted on non-x86_64 targets and/or older kernel headers (say RHEL5, older SLES, etc.). Unfortunately, the build breaks on CentOS 5.10 (= RHEL5) with: libtool: compile: /home/uros/gcc-build-xxx/./gcc/xgcc -shared-libgcc -B/home/uros/gcc-build-xxx/./gcc -nostdinc++ -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/src/.libs -L/home/uros/gcc-build-xxx/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/.libs -B/usr/local/x86_64-unknown-linux-gnu/bin/ -B/usr/local/x86_64-unknown-linux-gnu/lib/ -isystem /usr/local/x86_64-unknown-linux-gnu/include -isystem /usr/local/x86_64-unknown-linux-gnu/sys-include -D_GNU_SOURCE -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I. -I../../../../gcc-svn/trunk/libsanitizer/sanitizer_common -I ../../../../gcc-svn/trunk/libsanitizer/include -Wall -W -Wno-unused-parameter -Wwrite-strings -pedantic -Wno-long-long -fPIC -fno-builtin -fno-exceptions -fno-rtti -fomit-frame-pointer -funwind-tables -fvisibility=hidden -Wno-variadic-macros -I../../libstdc++-v3/include -I../../libstdc++-v3/include/x86_64-unknown-linux-gnu -I../../../../gcc-svn/trunk/libsanitizer/../libstdc++-v3/libsupc++ -g -O2 -D_GNU_SOURCE -MT sanitizer_platform_limits_linux.lo -MD -MP -MF .deps/sanitizer_platform_limits_linux.Tpo -c ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc -fPIC -DPIC -o .libs/sanitizer_platform_limits_linux.o ../../../../gcc-svn/trunk/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:54:30: fatal error: linux/perf_event.h: No such file or directory #include linux/perf_event.h Sounds familiar. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59068 Do things work for you w/o my patch in fresh trunk? No, so your patch doesn't regress anything. I can configure with --disable-libsanitizer to skip build of libsanitizer, although it would be nice to support RHEL5 derived long-term distributions. Ok, so this does not gate the merge. We can fix this particular failure, but unless someone helps us test the code upstream (not just that it builds, but also that it works) asan has little chance to work on old systems anyway. --kcc Is there a way to test gcc in such environment w/o setting up VMs (e.g. chroot, or some such)? Maybe gcc compile farm has linux-2.6.18 machine available? Uros.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 05:43:17PM +0100, Konstantin Serebryany wrote: We can fix this particular failure, but unless someone helps us test the code upstream (not just that it builds, but also that it works) asan has little chance to work on old systems anyway. For these kernel headers that were added only lately and weren't existing in older kernels, perhaps you can #include linux/version.h and guard the include of such headers plus everything related to that with #if LINUX_VERSION_CODE = 132640 (at least from Kernel's git linux/perf_event.h header has been added in 2.6.32). Or alternatively use configure, but you'd need to use it in both compiler-rt buildsystem and gcc's libsanitizer configure. Jakub
Re: libsanitizer merge from upstream r196090
On Mon, Dec 2, 2013 at 5:44 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 02, 2013 at 05:26:45PM +0100, Uros Bizjak wrote:
No, so your patch doesn't regress anything. I can configure with
--disable-libsanitizer to skip build of libsanitizer, although it
would be nice to support RHEL5 derived long-term distributions.
Is there a way to test gcc in such environment w/o setting up VMs
(e.g. chroot, or some such)?
Maybe gcc compile farm has linux-2.6.18 machine available?
That or perhaps try say:
mkdir ~/centos5
cd ~/centos5
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/glibc-devel-2.5-118.x86_64.rpm
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/glibc-headers-2.5-118.x86_64.rpm
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/kernel-headers-2.6.18-371.el5.x86_64.rpm
for i in *.rpm; do
rpm2cpio $i | cpio -id
done
and then compile with
g++ -nostdinc `g++ -v -E -xc++ /dev/null 21 | sed -n '/^#include /,/^End
of/{/\/usr\/include/d;s/^ \//-isystem /p}'` -isystem ~/centos5/usr/include/
This command will use all standard C++ search paths except for /usr/include,
and will use ~/centos5/usr/include/ instead of that.
Doing this gives me:
../gcc/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:24:20:
fatal error: stddef.h: No such file or directory
because stddef.h is found in /usr/include/linux; I guess we need some
more gcc flags here.
with #if LINUX_VERSION_CODE = 132640
Good idea, let me try that.
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 05:59:53PM +0100, Konstantin Serebryany wrote:
On Mon, Dec 2, 2013 at 5:44 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 02, 2013 at 05:26:45PM +0100, Uros Bizjak wrote:
No, so your patch doesn't regress anything. I can configure with
--disable-libsanitizer to skip build of libsanitizer, although it
would be nice to support RHEL5 derived long-term distributions.
Is there a way to test gcc in such environment w/o setting up VMs
(e.g. chroot, or some such)?
Maybe gcc compile farm has linux-2.6.18 machine available?
That or perhaps try say:
mkdir ~/centos5
cd ~/centos5
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/glibc-devel-2.5-118.x86_64.rpm
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/glibc-headers-2.5-118.x86_64.rpm
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/kernel-headers-2.6.18-371.el5.x86_64.rpm
for i in *.rpm; do
rpm2cpio $i | cpio -id
done
and then compile with
g++ -nostdinc `g++ -v -E -xc++ /dev/null 21 | sed -n '/^#include /,/^End
of/{/\/usr\/include/d;s/^ \//-isystem /p}'` -isystem ~/centos5/usr/include/
This command will use all standard C++ search paths except for /usr/include,
and will use ~/centos5/usr/include/ instead of that.
Doing this gives me:
../gcc/libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc:24:20:
fatal error: stddef.h: No such file or directory
because stddef.h is found in /usr/include/linux; I guess we need some
more gcc flags here.
Oops, sorry, should have been:
g++ -nostdinc `g++ -v -E -xc++ /dev/null 21 | sed -n '/^#include /,/^End
of/{/\/usr\/include/d;s/^ \//-isystem \//p}'` -isystem ~/centos5/usr/include/
(forgot about \/ in there, so it resulted in -isystem usr/lib/... rather than
-isystem /usr/lib/...
Jakub
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 05:26:45PM +0100, Uros Bizjak wrote:
No, so your patch doesn't regress anything. I can configure with
--disable-libsanitizer to skip build of libsanitizer, although it
would be nice to support RHEL5 derived long-term distributions.
Is there a way to test gcc in such environment w/o setting up VMs
(e.g. chroot, or some such)?
Maybe gcc compile farm has linux-2.6.18 machine available?
That or perhaps try say:
mkdir ~/centos5
cd ~/centos5
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/glibc-devel-2.5-118.x86_64.rpm
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/glibc-headers-2.5-118.x86_64.rpm
wget
http://mirrors.kernel.org/centos/5/os/x86_64/CentOS/kernel-headers-2.6.18-371.el5.x86_64.rpm
for i in *.rpm; do
rpm2cpio $i | cpio -id
done
and then compile with
g++ -nostdinc `g++ -v -E -xc++ /dev/null 21 | sed -n '/^#include /,/^End
of/{/\/usr\/include/d;s/^ \//-isystem /p}'` -isystem ~/centos5/usr/include/
This command will use all standard C++ search paths except for /usr/include,
and will use ~/centos5/usr/include/ instead of that.
Similarly for various other distros.
Jakub
Re: libsanitizer merge from upstream r196090
On Mon, Dec 02, 2013 at 05:59:53PM +0100, Konstantin Serebryany wrote:
with #if LINUX_VERSION_CODE = 132640
Good idea, let me try that.
Had a quick look at this on RHEL 5.
Following patch let me compile at least the first source file, but then
I run into tons of issues in sanitizer_platform_limits_posix.cc.
I think the main problem is that you are mixing standard glibc headers and
linux kernel headers in the same source file, that is a big no no.
Lots of the kernel headers declare the same things as glibc headers.
I'd strongly recommend splitting the files, so that you include absolute
minimum of
glibc headers when you include linux/* and/or asm/* headers and no kernel
headers
if you include tons of glibc headers.
And as the errors show up, there are also .cfi* directives that are used
unconditionally (you've set you've removed it from sanitizer_common or where it
was used (IMHO a pitty, much better would be conditionalizing them on either
compiler
preprocessor macros or whatever clang provides as alternative for that when not
building
with gcc)), but they are used in tsan (in HACKY_CALL macro). Plus in *.S file
(either that could be again guarded by the same preprocessor macro, or
configure or
something else). Note that RHEL5 here has already gas that supports .cfi_*
directives
(just not .cfi_personality/.cfi_lsda I think), but if you go to even older
system
it will not be there. E.g. glibc assembler files solve that by defining various
CFI_STARTPROC etc. macros that either expand to .cfi_startproc etc. if assembler
supports the directives, or nothing otherwise.
--- sanitizer_platform_limits_linux.cc.jj 2013-12-02 15:27:58.0
-0500
+++ sanitizer_platform_limits_linux.cc 2013-12-02 17:06:19.0 -0500
@@ -51,8 +51,12 @@
#endif
#if !SANITIZER_ANDROID
+#include linux/version.h
+// linux/perf_event.h has been added in 2.6.32
+#if LINUX_VERSION_CODE = 132640
#include linux/perf_event.h
#endif
+#endif
namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
@@ -75,15 +79,18 @@ CHECK_SIZE_AND_OFFSET(io_event, res);
CHECK_SIZE_AND_OFFSET(io_event, res2);
#if !SANITIZER_ANDROID
+#if LINUX_VERSION_CODE = 132640
COMPILER_CHECK(sizeof(struct __sanitizer_perf_event_attr) =
sizeof(struct perf_event_attr));
CHECK_SIZE_AND_OFFSET(perf_event_attr, type);
CHECK_SIZE_AND_OFFSET(perf_event_attr, size);
#endif
+#endif
COMPILER_CHECK(iocb_cmd_pread == IOCB_CMD_PREAD);
COMPILER_CHECK(iocb_cmd_pwrite == IOCB_CMD_PWRITE);
-#if !SANITIZER_ANDROID
+#if !SANITIZER_ANDROID LINUX_VERSION_CODE = 132627
+// IOCB_CMD_PREADV/PWRITEV has been added in 2.6.19
COMPILER_CHECK(iocb_cmd_preadv == IOCB_CMD_PREADV);
COMPILER_CHECK(iocb_cmd_pwritev == IOCB_CMD_PWRITEV);
#endif
--- sanitizer_platform_limits_posix.cc.jj 2013-12-02 15:27:58.0
-0500
+++ sanitizer_platform_limits_posix.cc 2013-12-02 17:11:00.0 -0500
@@ -82,12 +82,16 @@
#include sys/timex.h
#include sys/user.h
#include sys/ustat.h
+#include linux/version.h
#include linux/cyclades.h
#include linux/if_eql.h
#include linux/if_plip.h
#include linux/lp.h
#include linux/mroute.h
+// linux/mroute6.h has been added in 2.6.26
+#if LINUX_VERSION_CODE = 132634
#include linux/mroute6.h
+#endif
#include linux/scc.h
#include linux/serial.h
#include sys/msg.h
So the current errors are (from make -j64 -k to show more than one file):
In file included from /usr/include/sys/ustat.h:30:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:84:
/usr/include/bits/ustat.h:25:8: error: redefinition of ‘struct ustat’
struct ustat
^
In file included from /usr/include/linux/if_ether.h:24:0,
from /usr/include/netinet/if_ether.h:26,
from /usr/include/netinet/ether.h:26,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:47:
/usr/include/linux/types.h:156:8: error: previous definition of ‘struct ustat’
struct ustat {
^
In file included from /usr/include/linux/mroute.h:5:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:90:
/usr/include/linux/in.h:26:16: error: redeclaration of ‘IPPROTO_IP’
IPPROTO_IP = 0, /* Dummy protocol for TCP */
^
In file included from /usr/include/arpa/inet.h:23:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
/usr/include/netinet/in.h:33:5: note: previous declaration ‘anonymous enum
IPPROTO_IP’
IPPROTO_IP = 0,/* Dummy protocol for TCP. */
^
In file included from /usr/include/linux/mroute.h:5:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:90:
/usr/include/linux/in.h:27:18: error: redeclaration of ‘IPPROTO_ICMP’
IPPROTO_ICMP = 1, /* Internet Control Message Protocol */
^
In file included
Re: libsanitizer merge from upstream r196090
On Tue, Dec 3, 2013 at 2:32 AM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 02, 2013 at 05:59:53PM +0100, Konstantin Serebryany wrote:
with #if LINUX_VERSION_CODE = 132640
Good idea, let me try that.
Had a quick look at this on RHEL 5.
Following patch let me compile at least the first source file, but then
I run into tons of issues in sanitizer_platform_limits_posix.cc.
That's what I am afraid of. Even if we manage to compile everything,
there is no guarantee that the code will work.
I suggest to simply disable libsanitizer build on the older systems
which is what happens de facto now.
If there is significant interest in maintaining asanco on older
systems (which I have not seen so far),
then those interested will need to help us in upstream repository (llvm) by
a) sending us patches using http://llvm.org/docs/Phabricator.html and
b) setting up a public buildbot (attached to the LLVM master bot) with
the system they care about.
If there is no one interested enough to do a) and b) I say we should
not spend time on this.
And this discussion does not affect the merge since nothing that works
today will get broken, right?
I think the main problem is that you are mixing standard glibc headers and
linux kernel headers in the same source file, that is a big no no.
Lots of the kernel headers declare the same things as glibc headers.
I'd strongly recommend splitting the files, so that you include absolute
minimum of
glibc headers when you include linux/* and/or asm/* headers and no kernel
headers
if you include tons of glibc headers.
And as the errors show up, there are also .cfi* directives that are used
unconditionally (you've set you've removed it from sanitizer_common or where
it
was used (IMHO a pitty, much better would be conditionalizing them on either
compiler
preprocessor macros or whatever clang provides as alternative for that when
not building
with gcc)), but they are used in tsan (in HACKY_CALL macro). Plus in *.S file
(either that could be again guarded by the same preprocessor macro, or
configure or
something else). Note that RHEL5 here has already gas that supports .cfi_*
directives
(just not .cfi_personality/.cfi_lsda I think), but if you go to even older
system
it will not be there. E.g. glibc assembler files solve that by defining
various
CFI_STARTPROC etc. macros that either expand to .cfi_startproc etc. if
assembler
supports the directives, or nothing otherwise.
--- sanitizer_platform_limits_linux.cc.jj 2013-12-02 15:27:58.0
-0500
+++ sanitizer_platform_limits_linux.cc 2013-12-02 17:06:19.0 -0500
@@ -51,8 +51,12 @@
#endif
#if !SANITIZER_ANDROID
+#include linux/version.h
+// linux/perf_event.h has been added in 2.6.32
+#if LINUX_VERSION_CODE = 132640
#include linux/perf_event.h
#endif
+#endif
namespace __sanitizer {
unsigned struct_statfs64_sz = sizeof(struct statfs64);
@@ -75,15 +79,18 @@ CHECK_SIZE_AND_OFFSET(io_event, res);
CHECK_SIZE_AND_OFFSET(io_event, res2);
#if !SANITIZER_ANDROID
+#if LINUX_VERSION_CODE = 132640
COMPILER_CHECK(sizeof(struct __sanitizer_perf_event_attr) =
sizeof(struct perf_event_attr));
CHECK_SIZE_AND_OFFSET(perf_event_attr, type);
CHECK_SIZE_AND_OFFSET(perf_event_attr, size);
#endif
+#endif
COMPILER_CHECK(iocb_cmd_pread == IOCB_CMD_PREAD);
COMPILER_CHECK(iocb_cmd_pwrite == IOCB_CMD_PWRITE);
-#if !SANITIZER_ANDROID
+#if !SANITIZER_ANDROID LINUX_VERSION_CODE = 132627
+// IOCB_CMD_PREADV/PWRITEV has been added in 2.6.19
COMPILER_CHECK(iocb_cmd_preadv == IOCB_CMD_PREADV);
COMPILER_CHECK(iocb_cmd_pwritev == IOCB_CMD_PWRITEV);
#endif
--- sanitizer_platform_limits_posix.cc.jj 2013-12-02 15:27:58.0
-0500
+++ sanitizer_platform_limits_posix.cc 2013-12-02 17:11:00.0 -0500
@@ -82,12 +82,16 @@
#include sys/timex.h
#include sys/user.h
#include sys/ustat.h
+#include linux/version.h
#include linux/cyclades.h
#include linux/if_eql.h
#include linux/if_plip.h
#include linux/lp.h
#include linux/mroute.h
+// linux/mroute6.h has been added in 2.6.26
+#if LINUX_VERSION_CODE = 132634
#include linux/mroute6.h
+#endif
#include linux/scc.h
#include linux/serial.h
#include sys/msg.h
So the current errors are (from make -j64 -k to show more than one file):
In file included from /usr/include/sys/ustat.h:30:0,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:84:
/usr/include/bits/ustat.h:25:8: error: redefinition of ‘struct ustat’
struct ustat
^
In file included from /usr/include/linux/if_ether.h:24:0,
from /usr/include/netinet/if_ether.h:26,
from /usr/include/netinet/ether.h:26,
from
../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:47:
/usr/include/linux/types.h:156:8: error: previous definition of ‘struct ustat’
struct ustat {
Re: libsanitizer merge from upstream r196090
On Tue, Dec 3, 2013 at 6:53 AM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: with #if LINUX_VERSION_CODE = 132640 Good idea, let me try that. Had a quick look at this on RHEL 5. Following patch let me compile at least the first source file, but then I run into tons of issues in sanitizer_platform_limits_posix.cc. That's what I am afraid of. Even if we manage to compile everything, there is no guarantee that the code will work. I suggest to simply disable libsanitizer build on the older systems which is what happens de facto now. If there is significant interest in maintaining asanco on older systems (which I have not seen so far), then those interested will need to help us in upstream repository (llvm) by a) sending us patches using http://llvm.org/docs/Phabricator.html and b) setting up a public buildbot (attached to the LLVM master bot) with the system they care about. If there is no one interested enough to do a) and b) I say we should not spend time on this. IMO, it is also OK for the configure to check for needed features and disable libsanitizer (perhaps with some informative message) if minimum requirements are not met. If someone adds workarounds for those missing features to support older systems, then chese checks can easily be adapted. The problem ATM is, that gcc won't build out-of-the-box on older distributions, although adding --disable-libsanitizer manually works OK. And this discussion does not affect the merge since nothing that works today will get broken, right? Yes, that's right. Uros.
