public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: libsanitizer merge from upstream r196090
@ 2013-12-02 15:49 Uros Bizjak
  2013-12-02 16:12 ` Konstantin Serebryany
  2014-01-10  9:24 ` Uros Bizjak
  0 siblings, 2 replies; 63+ messages in thread
From: Uros Bizjak @ 2013-12-02 15:49 UTC (permalink / raw)
  To: gcc-patches
  Cc: Jakub Jelinek, H.J. Lu, Konstantin Serebryany, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov

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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 15:49 libsanitizer merge from upstream r196090 Uros Bizjak
@ 2013-12-02 16:12 ` Konstantin Serebryany
  2013-12-02 16:26   ` Uros Bizjak
  2014-01-10  9:24 ` Uros Bizjak
  1 sibling, 1 reply; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-02 16:12 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: gcc-patches, Jakub Jelinek, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Mon, Dec 2, 2013 at 4:49 PM, Uros Bizjak <ubizjak@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 16:12 ` Konstantin Serebryany
@ 2013-12-02 16:26   ` Uros Bizjak
  2013-12-02 16:43     ` Konstantin Serebryany
  2013-12-02 16:45     ` Jakub Jelinek
  0 siblings, 2 replies; 63+ messages in thread
From: Uros Bizjak @ 2013-12-02 16:26 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: gcc-patches, Jakub Jelinek, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Mon, Dec 2, 2013 at 5:12 PM, Konstantin Serebryany
<konstantin.s.serebryany@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 16:26   ` Uros Bizjak
@ 2013-12-02 16:43     ` Konstantin Serebryany
  2013-12-02 16:54       ` Jakub Jelinek
  2013-12-03 11:50       ` Jakub Jelinek
  2013-12-02 16:45     ` Jakub Jelinek
  1 sibling, 2 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-02 16:43 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: gcc-patches, Jakub Jelinek, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Mon, Dec 2, 2013 at 5:26 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Dec 2, 2013 at 5:12 PM, Konstantin Serebryany
> <konstantin.s.serebryany@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 16:26   ` Uros Bizjak
  2013-12-02 16:43     ` Konstantin Serebryany
@ 2013-12-02 16:45     ` Jakub Jelinek
  2013-12-02 17:00       ` Konstantin Serebryany
  1 sibling, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-02 16:45 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: Konstantin Serebryany, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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 2>&1 | 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 16:43     ` Konstantin Serebryany
@ 2013-12-02 16:54       ` Jakub Jelinek
  2013-12-03 11:50       ` Jakub Jelinek
  1 sibling, 0 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-02 16:54 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 16:45     ` Jakub Jelinek
@ 2013-12-02 17:00       ` Konstantin Serebryany
  2013-12-02 17:05         ` Jakub Jelinek
  2013-12-02 22:33         ` Jakub Jelinek
  0 siblings, 2 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-02 17:00 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Mon, Dec 2, 2013 at 5:44 PM, Jakub Jelinek <jakub@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 2>&1 | 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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 17:00       ` Konstantin Serebryany
@ 2013-12-02 17:05         ` Jakub Jelinek
  2013-12-02 22:33         ` Jakub Jelinek
  1 sibling, 0 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-02 17:05 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Mon, Dec 02, 2013 at 05:59:53PM +0100, Konstantin Serebryany wrote:
> On Mon, Dec 2, 2013 at 5:44 PM, Jakub Jelinek <jakub@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 2>&1 | 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 2>&1 | 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 17:00       ` Konstantin Serebryany
  2013-12-02 17:05         ` Jakub Jelinek
@ 2013-12-02 22:33         ` Jakub Jelinek
  2013-12-03  5:53           ` Konstantin Serebryany
  2013-12-04  5:29           ` Konstantin Serebryany
  1 sibling, 2 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-02 22:33 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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.000000000 -0500
+++ sanitizer_platform_limits_linux.cc	2013-12-02 17:06:19.000000000 -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.000000000 -0500
+++ sanitizer_platform_limits_posix.cc	2013-12-02 17:11:00.000000000 -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 from /usr/include/arpa/inet.h:23:0,
                 from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
/usr/include/netinet/in.h:37:5: note: previous declaration ‘<anonymous enum> IPPROTO_ICMP’
     IPPROTO_ICMP = 1,    /* Internet Control Message Protocol.  */
     ^
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:28:18: error: redeclaration of ‘IPPROTO_IGMP’
   IPPROTO_IGMP = 2,  /* Internet Group Management Protocol */
                  ^
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:39:5: note: previous declaration ‘<anonymous enum> IPPROTO_IGMP’
     IPPROTO_IGMP = 2,    /* Internet Group Management Protocol. */
     ^
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:29:18: error: redeclaration of ‘IPPROTO_IPIP’
   IPPROTO_IPIP = 4,  /* IPIP tunnels (older KA9Q tunnels use 94) */
                  ^
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:41:5: note: previous declaration ‘<anonymous enum> IPPROTO_IPIP’
     IPPROTO_IPIP = 4,    /* IPIP tunnels (older KA9Q tunnels use 94).  */
     ^
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:30:17: error: redeclaration of ‘IPPROTO_TCP’
   IPPROTO_TCP = 6,  /* Transmission Control Protocol */
                 ^
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:43:5: note: previous declaration ‘<anonymous enum> IPPROTO_TCP’
     IPPROTO_TCP = 6,    /* Transmission Control Protocol.  */
     ^
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:31:17: error: redeclaration of ‘IPPROTO_EGP’
   IPPROTO_EGP = 8,  /* Exterior Gateway Protocol  */
                 ^
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:45:5: note: previous declaration ‘<anonymous enum> IPPROTO_EGP’
     IPPROTO_EGP = 8,    /* Exterior Gateway Protocol.  */
     ^
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:32:17: error: redeclaration of ‘IPPROTO_PUP’
   IPPROTO_PUP = 12,  /* PUP protocol    */
                 ^
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:47:5: note: previous declaration ‘<anonymous enum> IPPROTO_PUP’
     IPPROTO_PUP = 12,    /* PUP protocol.  */
     ^
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:33:17: error: redeclaration of ‘IPPROTO_UDP’
   IPPROTO_UDP = 17,  /* User Datagram Protocol  */
                 ^
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:49:5: note: previous declaration ‘<anonymous enum> IPPROTO_UDP’
     IPPROTO_UDP = 17,    /* User Datagram Protocol.  */
     ^
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:34:17: error: redeclaration of ‘IPPROTO_IDP’
   IPPROTO_IDP = 22,  /* XNS IDP protocol   */
                 ^
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:51:5: note: previous declaration ‘<anonymous enum> IPPROTO_IDP’
     IPPROTO_IDP = 22,    /* XNS IDP protocol.  */
     ^
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:36:18: error: redeclaration of ‘IPPROTO_RSVP’
   IPPROTO_RSVP = 46,  /* RSVP protocol   */
                  ^
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:61:5: note: previous declaration ‘<anonymous enum> IPPROTO_RSVP’
     IPPROTO_RSVP = 46,    /* Reservation Protocol.  */
     ^
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:37:17: error: redeclaration of ‘IPPROTO_GRE’
   IPPROTO_GRE = 47,  /* Cisco GRE tunnels (rfc 1701,1702) */
                 ^
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:63:5: note: previous declaration ‘<anonymous enum> IPPROTO_GRE’
     IPPROTO_GRE = 47,    /* General Routing Encapsulation.  */
     ^
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:39:19: error: redeclaration of ‘IPPROTO_IPV6’
   IPPROTO_IPV6  = 41,  /* IPv6-in-IPv4 tunnelling  */
                   ^
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:55:5: note: previous declaration ‘<anonymous enum> IPPROTO_IPV6’
     IPPROTO_IPV6 = 41,     /* IPv6 header.  */
     ^
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:41:17: error: redeclaration of ‘IPPROTO_ESP’
   IPPROTO_ESP = 50,            /* Encapsulation Security Payload protocol */
                 ^
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:65:5: note: previous declaration ‘<anonymous enum> IPPROTO_ESP’
     IPPROTO_ESP = 50,      /* encapsulating security payload.  */
     ^
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:42:16: error: redeclaration of ‘IPPROTO_AH’
   IPPROTO_AH = 51,             /* Authentication Header protocol       */
                ^
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:67:5: note: previous declaration ‘<anonymous enum> IPPROTO_AH’
     IPPROTO_AH = 51,       /* authentication header.  */
     ^
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:43:20: error: redeclaration of ‘IPPROTO_PIM’
   IPPROTO_PIM    = 103,  /* Protocol Independent Multicast */
                    ^
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:79:5: note: previous declaration ‘<anonymous enum> IPPROTO_PIM’
     IPPROTO_PIM = 103,    /* Protocol Independent Multicast.  */
     ^
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:45:20: error: redeclaration of ‘IPPROTO_COMP’
   IPPROTO_COMP   = 108,                /* Compression Header protocol */
                    ^
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:81:5: note: previous declaration ‘<anonymous enum> IPPROTO_COMP’
     IPPROTO_COMP = 108,    /* Compression Header Protocol.  */
     ^
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:46:20: error: redeclaration of ‘IPPROTO_SCTP’
   IPPROTO_SCTP   = 132,  /* Stream Control Transport Protocol */
                    ^
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:83:5: note: previous declaration ‘<anonymous enum> IPPROTO_SCTP’
     IPPROTO_SCTP = 132,    /* Stream Control Transmission Protocol.  */
     ^
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:48:18: error: redeclaration of ‘IPPROTO_RAW’
   IPPROTO_RAW  = 255,  /* Raw IP packets   */
                  ^
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:85:5: note: previous declaration ‘<anonymous enum> IPPROTO_RAW’
     IPPROTO_RAW = 255,    /* Raw IP packets.  */
     ^
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:49:3: error: redeclaration of ‘IPPROTO_MAX’
   IPPROTO_MAX
   ^
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:87:5: note: previous declaration ‘<anonymous enum> IPPROTO_MAX’
     IPPROTO_MAX
     ^
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:54:8: error: redefinition of ‘struct in_addr’
 struct in_addr {
        ^
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:137:8: error: previous definition of ‘struct in_addr’
 struct in_addr
        ^
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:113:8: error: redefinition of ‘struct ip_mreq’
 struct ip_mreq 
        ^
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:244:8: error: previous definition of ‘struct ip_mreq’
 struct ip_mreq
        ^
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:119:8: error: redefinition of ‘struct ip_mreqn’
 struct ip_mreqn
        ^
In file included from /usr/include/netinet/in.h:345:0,
                 from /usr/include/arpa/inet.h:23,
                 from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
/usr/include/bits/in.h:89:8: error: previous definition of ‘struct ip_mreqn’
 struct ip_mreqn
        ^
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:126:8: error: redefinition of ‘struct ip_mreq_source’
 struct ip_mreq_source {
        ^
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:253:8: error: previous definition of ‘struct ip_mreq_source’
 struct ip_mreq_source
        ^
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:132:8: error: redefinition of ‘struct ip_msfilter’
 struct ip_msfilter {
        ^
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:300:8: error: previous definition of ‘struct ip_msfilter’
 struct ip_msfilter
        ^
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:144:8: error: redefinition of ‘struct group_req’
 struct group_req
        ^
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:277:8: error: previous definition of ‘struct group_req’
 struct group_req
        ^
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:150:8: error: redefinition of ‘struct group_source_req’
 struct group_source_req
        ^
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:286:8: error: previous definition of ‘struct group_source_req’
 struct group_source_req
        ^
In file included from /usr/include/linux/mroute.h:5:0,
                 from ../../../../libsani    from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
/usr/include/bits/in.h:97:8: error: previous definition of ‘struct in_pktinfo’
 struct in_pktinfo
        ^
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:179:8: error: redefinition of ‘struct sockaddr_in’
 struct sockaddr_in {
        ^
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:219:8: error: previous definition of ‘struct sockaddr_in’
 struct sockaddr_in
        ^
make[1]: *** [sanitizer_platform_limits_posix.lo] Error 1
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
make[1]: *** [tsan_rtl_thread.lo] Error 1
../../../../libsanitizer/tsan/tsan_rtl.h: Assembler messages:
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
make[1]: *** [tsan_rtl_mutex.lo] Error 1
...
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
make[1]: *** [tsan_interface_atomic.lo] Error 1
../../../../libsanitizer/tsan/tsan_rtl.cc: Assembler messages:
../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
...
../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
make[1]: *** [tsan_rtl.lo] Error 1



	Jakub

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 22:33         ` Jakub Jelinek
@ 2013-12-03  5:53           ` Konstantin Serebryany
  2013-12-03  7:34             ` Uros Bizjak
  2013-12-04  5:29           ` Konstantin Serebryany
  1 sibling, 1 reply; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-03  5:53 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Tue, Dec 3, 2013 at 2:32 AM, Jakub Jelinek <jakub@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 asan&co 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.000000000 -0500
> +++ sanitizer_platform_limits_linux.cc  2013-12-02 17:06:19.000000000 -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.000000000 -0500
> +++ sanitizer_platform_limits_posix.cc  2013-12-02 17:11:00.000000000 -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 from /usr/include/arpa/inet.h:23:0,
>                  from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
> /usr/include/netinet/in.h:37:5: note: previous declaration ‘<anonymous enum> IPPROTO_ICMP’
>      IPPROTO_ICMP = 1,    /* Internet Control Message Protocol.  */
>      ^
> 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:28:18: error: redeclaration of ‘IPPROTO_IGMP’
>    IPPROTO_IGMP = 2,  /* Internet Group Management Protocol */
>                   ^
> 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:39:5: note: previous declaration ‘<anonymous enum> IPPROTO_IGMP’
>      IPPROTO_IGMP = 2,    /* Internet Group Management Protocol. */
>      ^
> 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:29:18: error: redeclaration of ‘IPPROTO_IPIP’
>    IPPROTO_IPIP = 4,  /* IPIP tunnels (older KA9Q tunnels use 94) */
>                   ^
> 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:41:5: note: previous declaration ‘<anonymous enum> IPPROTO_IPIP’
>      IPPROTO_IPIP = 4,    /* IPIP tunnels (older KA9Q tunnels use 94).  */
>      ^
> 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:30:17: error: redeclaration of ‘IPPROTO_TCP’
>    IPPROTO_TCP = 6,  /* Transmission Control Protocol */
>                  ^
> 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:43:5: note: previous declaration ‘<anonymous enum> IPPROTO_TCP’
>      IPPROTO_TCP = 6,    /* Transmission Control Protocol.  */
>      ^
> 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:31:17: error: redeclaration of ‘IPPROTO_EGP’
>    IPPROTO_EGP = 8,  /* Exterior Gateway Protocol  */
>                  ^
> 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:45:5: note: previous declaration ‘<anonymous enum> IPPROTO_EGP’
>      IPPROTO_EGP = 8,    /* Exterior Gateway Protocol.  */
>      ^
> 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:32:17: error: redeclaration of ‘IPPROTO_PUP’
>    IPPROTO_PUP = 12,  /* PUP protocol    */
>                  ^
> 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:47:5: note: previous declaration ‘<anonymous enum> IPPROTO_PUP’
>      IPPROTO_PUP = 12,    /* PUP protocol.  */
>      ^
> 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:33:17: error: redeclaration of ‘IPPROTO_UDP’
>    IPPROTO_UDP = 17,  /* User Datagram Protocol  */
>                  ^
> 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:49:5: note: previous declaration ‘<anonymous enum> IPPROTO_UDP’
>      IPPROTO_UDP = 17,    /* User Datagram Protocol.  */
>      ^
> 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:34:17: error: redeclaration of ‘IPPROTO_IDP’
>    IPPROTO_IDP = 22,  /* XNS IDP protocol   */
>                  ^
> 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:51:5: note: previous declaration ‘<anonymous enum> IPPROTO_IDP’
>      IPPROTO_IDP = 22,    /* XNS IDP protocol.  */
>      ^
> 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:36:18: error: redeclaration of ‘IPPROTO_RSVP’
>    IPPROTO_RSVP = 46,  /* RSVP protocol   */
>                   ^
> 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:61:5: note: previous declaration ‘<anonymous enum> IPPROTO_RSVP’
>      IPPROTO_RSVP = 46,    /* Reservation Protocol.  */
>      ^
> 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:37:17: error: redeclaration of ‘IPPROTO_GRE’
>    IPPROTO_GRE = 47,  /* Cisco GRE tunnels (rfc 1701,1702) */
>                  ^
> 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:63:5: note: previous declaration ‘<anonymous enum> IPPROTO_GRE’
>      IPPROTO_GRE = 47,    /* General Routing Encapsulation.  */
>      ^
> 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:39:19: error: redeclaration of ‘IPPROTO_IPV6’
>    IPPROTO_IPV6  = 41,  /* IPv6-in-IPv4 tunnelling  */
>                    ^
> 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:55:5: note: previous declaration ‘<anonymous enum> IPPROTO_IPV6’
>      IPPROTO_IPV6 = 41,     /* IPv6 header.  */
>      ^
> 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:41:17: error: redeclaration of ‘IPPROTO_ESP’
>    IPPROTO_ESP = 50,            /* Encapsulation Security Payload protocol */
>                  ^
> 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:65:5: note: previous declaration ‘<anonymous enum> IPPROTO_ESP’
>      IPPROTO_ESP = 50,      /* encapsulating security payload.  */
>      ^
> 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:42:16: error: redeclaration of ‘IPPROTO_AH’
>    IPPROTO_AH = 51,             /* Authentication Header protocol       */
>                 ^
> 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:67:5: note: previous declaration ‘<anonymous enum> IPPROTO_AH’
>      IPPROTO_AH = 51,       /* authentication header.  */
>      ^
> 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:43:20: error: redeclaration of ‘IPPROTO_PIM’
>    IPPROTO_PIM    = 103,  /* Protocol Independent Multicast */
>                     ^
> 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:79:5: note: previous declaration ‘<anonymous enum> IPPROTO_PIM’
>      IPPROTO_PIM = 103,    /* Protocol Independent Multicast.  */
>      ^
> 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:45:20: error: redeclaration of ‘IPPROTO_COMP’
>    IPPROTO_COMP   = 108,                /* Compression Header protocol */
>                     ^
> 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:81:5: note: previous declaration ‘<anonymous enum> IPPROTO_COMP’
>      IPPROTO_COMP = 108,    /* Compression Header Protocol.  */
>      ^
> 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:46:20: error: redeclaration of ‘IPPROTO_SCTP’
>    IPPROTO_SCTP   = 132,  /* Stream Control Transport Protocol */
>                     ^
> 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:83:5: note: previous declaration ‘<anonymous enum> IPPROTO_SCTP’
>      IPPROTO_SCTP = 132,    /* Stream Control Transmission Protocol.  */
>      ^
> 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:48:18: error: redeclaration of ‘IPPROTO_RAW’
>    IPPROTO_RAW  = 255,  /* Raw IP packets   */
>                   ^
> 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:85:5: note: previous declaration ‘<anonymous enum> IPPROTO_RAW’
>      IPPROTO_RAW = 255,    /* Raw IP packets.  */
>      ^
> 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:49:3: error: redeclaration of ‘IPPROTO_MAX’
>    IPPROTO_MAX
>    ^
> 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:87:5: note: previous declaration ‘<anonymous enum> IPPROTO_MAX’
>      IPPROTO_MAX
>      ^
> 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:54:8: error: redefinition of ‘struct in_addr’
>  struct in_addr {
>         ^
> 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:137:8: error: previous definition of ‘struct in_addr’
>  struct in_addr
>         ^
> 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:113:8: error: redefinition of ‘struct ip_mreq’
>  struct ip_mreq
>         ^
> 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:244:8: error: previous definition of ‘struct ip_mreq’
>  struct ip_mreq
>         ^
> 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:119:8: error: redefinition of ‘struct ip_mreqn’
>  struct ip_mreqn
>         ^
> In file included from /usr/include/netinet/in.h:345:0,
>                  from /usr/include/arpa/inet.h:23,
>                  from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
> /usr/include/bits/in.h:89:8: error: previous definition of ‘struct ip_mreqn’
>  struct ip_mreqn
>         ^
> 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:126:8: error: redefinition of ‘struct ip_mreq_source’
>  struct ip_mreq_source {
>         ^
> 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:253:8: error: previous definition of ‘struct ip_mreq_source’
>  struct ip_mreq_source
>         ^
> 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:132:8: error: redefinition of ‘struct ip_msfilter’
>  struct ip_msfilter {
>         ^
> 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:300:8: error: previous definition of ‘struct ip_msfilter’
>  struct ip_msfilter
>         ^
> 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:144:8: error: redefinition of ‘struct group_req’
>  struct group_req
>         ^
> 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:277:8: error: previous definition of ‘struct group_req’
>  struct group_req
>         ^
> 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:150:8: error: redefinition of ‘struct group_source_req’
>  struct group_source_req
>         ^
> 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:286:8: error: previous definition of ‘struct group_source_req’
>  struct group_source_req
>         ^
> In file included from /usr/include/linux/mroute.h:5:0,
>                  from ../../../../libsani    from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
> /usr/include/bits/in.h:97:8: error: previous definition of ‘struct in_pktinfo’
>  struct in_pktinfo
>         ^
> 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:179:8: error: redefinition of ‘struct sockaddr_in’
>  struct sockaddr_in {
>         ^
> 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:219:8: error: previous definition of ‘struct sockaddr_in’
>  struct sockaddr_in
>         ^
> make[1]: *** [sanitizer_platform_limits_posix.lo] Error 1
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_rtl_thread.lo] Error 1
> ../../../../libsanitizer/tsan/tsan_rtl.h: Assembler messages:
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_rtl_mutex.lo] Error 1
> ...
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_interface_atomic.lo] Error 1
> ../../../../libsanitizer/tsan/tsan_rtl.cc: Assembler messages:
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ...
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_rtl.lo] Error 1
>
>
>
>         Jakub

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03  5:53           ` Konstantin Serebryany
@ 2013-12-03  7:34             ` Uros Bizjak
  0 siblings, 0 replies; 63+ messages in thread
From: Uros Bizjak @ 2013-12-03  7:34 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Jakub Jelinek, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Tue, Dec 3, 2013 at 6:53 AM, Konstantin Serebryany
<konstantin.s.serebryany@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 asan&co 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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 16:43     ` Konstantin Serebryany
  2013-12-02 16:54       ` Jakub Jelinek
@ 2013-12-03 11:50       ` Jakub Jelinek
  2013-12-03 15:18         ` Konstantin Serebryany
  1 sibling, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-03 11:50 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

[-- Attachment #1: Type: text/plain, Size: 3557 bytes --]

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

[-- Attachment #2: Q --]
[-- Type: text/plain, Size: 1784 bytes --]

--- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.h.jj3	2013-12-03 03:33:20.000000000 -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 __sanitizer___kernel_gid_t;
+#endif
+#if defined(__x86_64__) && !defined(_LP64)
+  typedef long long __sanitizer___kernel_off_t;
+#else
   typedef long __sanitizer___kernel_off_t;
 #endif
 
-#if defined(__powerpc64__)
+#if defined(__powerpc__)
   typedef unsigned int __sanitizer___kernel_old_uid_t;
   typedef unsigned int __sanitizer___kernel_old_gid_t;
 #else

[-- Attachment #3: P --]
[-- Type: text/plain, Size: 12754 bytes --]

--- libsanitizer/sanitizer_common/sanitizer_linux.cc.jj	2013-12-03 11:47:47.399488381 +0100
+++ libsanitizer/sanitizer_common/sanitizer_linux.cc	2013-12-03 11:48:01.905411215 +0100
@@ -814,6 +814,9 @@ uptr internal_clone(int (*fn)(void *), v
                         *                %r8  = new_tls,
                         *                %r10 = child_tidptr)
                         */
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+                       ".cfi_endproc\n"
+#endif
                        "syscall\n"
 
                        /* if (%rax != 0)
@@ -826,6 +829,10 @@ uptr internal_clone(int (*fn)(void *), v
                        // XXX: We should also terminate the CFI unwind chain
                        // here. Unfortunately clang 3.2 doesn't support the
                        // necessary CFI directives, so we skip that part.
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+                       ".cfi_startproc\n"
+                       ".cfi_undefined %%rip;\n"
+#endif
                        "xorq   %%rbp,%%rbp\n"
 
                        /* Call "fn(arg)". */
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc.jj	2013-12-03 11:47:47.246489195 +0100
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc	2013-12-03 11:51:31.884291618 +0100
@@ -17,6 +17,18 @@
 #include "sanitizer_internal_defs.h"
 #include "sanitizer_platform_limits_posix.h"
 
+#if SANITIZER_LINUX && !SANITIZER_ANDROID
+#include <asm/types.h>
+#include <asm/posix_types.h>
+#define _LINUX_TYPES_H
+#define __le16 __u16
+#define __le32 __u32
+#define __le64 __u64
+#define __be16 __u16
+#define __be32 __u32
+#define __be64 __u64
+#endif
+
 #include <arpa/inet.h>
 #include <dirent.h>
 #include <errno.h>
@@ -86,8 +98,6 @@
 #include <linux/if_eql.h>
 #include <linux/if_plip.h>
 #include <linux/lp.h>
-#include <linux/mroute.h>
-#include <linux/mroute6.h>
 #include <linux/scc.h>
 #include <linux/serial.h>
 #include <sys/msg.h>
@@ -321,11 +331,6 @@ namespace __sanitizer {
   unsigned struct_unimapinit_sz = sizeof(struct unimapinit);
 #endif
 
-#if !SANITIZER_ANDROID && !SANITIZER_MAC
-  unsigned struct_sioc_sg_req_sz = sizeof(struct sioc_sg_req);
-  unsigned struct_sioc_vif_req_sz = sizeof(struct sioc_vif_req);
-#endif
-
   unsigned IOCTL_NOT_PRESENT = 0;
 
   unsigned IOCTL_FIOASYNC = FIOASYNC;
@@ -372,10 +377,6 @@ namespace __sanitizer {
   unsigned IOCTL_TIOCSPGRP = TIOCSPGRP;
   unsigned IOCTL_TIOCSTI = TIOCSTI;
   unsigned IOCTL_TIOCSWINSZ = TIOCSWINSZ;
-#if (SANITIZER_LINUX && !SANITIZER_ANDROID)
-  unsigned IOCTL_SIOCGETSGCNT = SIOCGETSGCNT;
-  unsigned IOCTL_SIOCGETVIFCNT = SIOCGETVIFCNT;
-#endif
 #if SANITIZER_LINUX
   unsigned IOCTL_EVIOCGABS = EVIOCGABS(0);
   unsigned IOCTL_EVIOCGBIT = EVIOCGBIT(0, 0);
--- libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc.jj	2013-12-03 11:47:47.449488116 +0100
+++ libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc	2013-12-03 11:54:34.800468807 +0100
@@ -26,6 +26,7 @@
 // With old kernels (and even new kernels on powerpc) asm/stat.h uses types that
 // are not defined anywhere in userspace headers. Fake them. This seems to work
 // fine with newer headers, too.
+#include <asm/types.h>
 #include <asm/posix_types.h>
 #define ino_t __kernel_ino_t
 #define mode_t __kernel_mode_t
@@ -42,6 +43,14 @@
 #undef gid_t
 #undef off_t
 
+#define _LINUX_TYPES_H
+#define __le16 __u16
+#define __le32 __u32
+#define __le64 __u64
+#define __be16 __u16
+#define __be32 __u32
+#define __be64 __u64
+
 #include <linux/aio_abi.h>
 
 #if SANITIZER_ANDROID
@@ -51,11 +60,32 @@
 #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
+#include <bits/sockaddr.h>
+#include <linux/in.h>
+#include <linux/mroute.h>
+// <linux/mroute6.h> has been added in 2.6.26
+#if LINUX_VERSION_CODE >= 132634
+#include <linux/in6.h>
+#include <linux/mroute6.h>
+#endif
+#endif
 
 namespace __sanitizer {
   unsigned struct_statfs64_sz = sizeof(struct statfs64);
+
+#if !SANITIZER_ANDROID
+  unsigned struct_sioc_sg_req_sz = sizeof(struct sioc_sg_req);
+  unsigned struct_sioc_vif_req_sz = sizeof(struct sioc_vif_req);
+
+  unsigned IOCTL_SIOCGETSGCNT = SIOCGETSGCNT;
+  unsigned IOCTL_SIOCGETVIFCNT = SIOCGETVIFCNT;
+#endif
+
 }  // namespace __sanitizer
 
 #if !defined(__powerpc64__)
@@ -75,15 +105,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
--- libsanitizer/tsan/tsan_rtl_amd64.S.jj	2012-11-23 00:14:41.336231221 +0100
+++ libsanitizer/tsan/tsan_rtl_amd64.S	2013-12-03 11:48:01.905411215 +0100
@@ -1,42 +1,58 @@
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+#define CFI_STARTPROC .cfi_startproc
+#define CFI_ENDPROC .cfi_endproc
+#define CFI_ADJUST_CFA_OFFSET(n) .cfi_adjust_cfa_offset n
+#define CFI_REL_OFFSET(reg,n) .cfi_rel_offset reg, n
+#define CFI_DEF_CFA_REGISTER(reg) .cfi_def_cfa_register reg
+#define CFI_RESTORE(reg) .cfi_restore reg
+#else
+#define CFI_STARTPROC
+#define CFI_ENDPROC
+#define CFI_ADJUST_CFA_OFFSET(n)
+#define CFI_REL_OFFSET(reg,n)
+#define CFI_DEF_CFA_REGISTER(reg)
+#define CFI_RESTORE(reg)
+#endif
+
 .section .text
 
 .globl __tsan_trace_switch_thunk
 __tsan_trace_switch_thunk:
-  .cfi_startproc
+  CFI_STARTPROC
   # Save scratch registers.
   push %rax
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rax, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rax, 0)
   push %rcx
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rcx, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rcx, 0)
   push %rdx
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rdx, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rdx, 0)
   push %rsi
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rsi, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rsi, 0)
   push %rdi
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rdi, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rdi, 0)
   push %r8
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r8, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r8, 0)
   push %r9
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r9, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r9, 0)
   push %r10
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r10, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r10, 0)
   push %r11
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r11, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r11, 0)
   # Align stack frame.
   push %rbx  # non-scratch
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rbx, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rbx, 0)
   mov %rsp, %rbx  # save current rsp
-  .cfi_def_cfa_register %rbx
+  CFI_DEF_CFA_REGISTER(%rbx)
   shr $4, %rsp  # clear 4 lsb, align to 16
   shl $4, %rsp
 
@@ -44,78 +60,78 @@ __tsan_trace_switch_thunk:
 
   # Unalign stack frame back.
   mov %rbx, %rsp  # restore the original rsp
-  .cfi_def_cfa_register %rsp
+  CFI_DEF_CFA_REGISTER(%rsp)
   pop %rbx
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   # Restore scratch registers.
   pop %r11
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %r10
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %r9
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %r8
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rdi
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rsi
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rdx
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rcx
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rax
-  .cfi_adjust_cfa_offset -8
-  .cfi_restore %rax
-  .cfi_restore %rbx
-  .cfi_restore %rcx
-  .cfi_restore %rdx
-  .cfi_restore %rsi
-  .cfi_restore %rdi
-  .cfi_restore %r8
-  .cfi_restore %r9
-  .cfi_restore %r10
-  .cfi_restore %r11
+  CFI_ADJUST_CFA_OFFSET(-8)
+  CFI_RESTORE(%rax)
+  CFI_RESTORE(%rbx)
+  CFI_RESTORE(%rcx)
+  CFI_RESTORE(%rdx)
+  CFI_RESTORE(%rsi)
+  CFI_RESTORE(%rdi)
+  CFI_RESTORE(%r8)
+  CFI_RESTORE(%r9)
+  CFI_RESTORE(%r10)
+  CFI_RESTORE(%r11)
   ret
-  .cfi_endproc
+  CFI_ENDPROC
 
 .globl __tsan_report_race_thunk
 __tsan_report_race_thunk:
-  .cfi_startproc
+  CFI_STARTPROC
   # Save scratch registers.
   push %rax
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rax, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rax, 0)
   push %rcx
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rcx, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rcx, 0)
   push %rdx
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rdx, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rdx, 0)
   push %rsi
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rsi, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rsi, 0)
   push %rdi
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rdi, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rdi, 0)
   push %r8
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r8, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r8, 0)
   push %r9
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r9, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r9, 0)
   push %r10
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r10, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r10, 0)
   push %r11
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %r11, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%r11, 0)
   # Align stack frame.
   push %rbx  # non-scratch
-  .cfi_adjust_cfa_offset 8
-  .cfi_rel_offset %rbx, 0
+  CFI_ADJUST_CFA_OFFSET(8)
+  CFI_REL_OFFSET(%rbx, 0)
   mov %rsp, %rbx  # save current rsp
-  .cfi_def_cfa_register %rbx
+  CFI_DEF_CFA_REGISTER(%rbx)
   shr $4, %rsp  # clear 4 lsb, align to 16
   shl $4, %rsp
 
@@ -123,40 +139,40 @@ __tsan_report_race_thunk:
 
   # Unalign stack frame back.
   mov %rbx, %rsp  # restore the original rsp
-  .cfi_def_cfa_register %rsp
+  CFI_DEF_CFA_REGISTER(%rsp)
   pop %rbx
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   # Restore scratch registers.
   pop %r11
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %r10
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %r9
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %r8
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rdi
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rsi
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rdx
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rcx
-  .cfi_adjust_cfa_offset -8
+  CFI_ADJUST_CFA_OFFSET(-8)
   pop %rax
-  .cfi_adjust_cfa_offset -8
-  .cfi_restore %rax
-  .cfi_restore %rbx
-  .cfi_restore %rcx
-  .cfi_restore %rdx
-  .cfi_restore %rsi
-  .cfi_restore %rdi
-  .cfi_restore %r8
-  .cfi_restore %r9
-  .cfi_restore %r10
-  .cfi_restore %r11
+  CFI_ADJUST_CFA_OFFSET(-8)
+  CFI_RESTORE(%rax)
+  CFI_RESTORE(%rbx)
+  CFI_RESTORE(%rcx)
+  CFI_RESTORE(%rdx)
+  CFI_RESTORE(%rsi)
+  CFI_RESTORE(%rdi)
+  CFI_RESTORE(%r8)
+  CFI_RESTORE(%r9)
+  CFI_RESTORE(%r10)
+  CFI_RESTORE(%r11)
   ret
-  .cfi_endproc
+  CFI_ENDPROC
 
 #ifdef __linux__
 /* We do not need executable stack.  */
--- libsanitizer/tsan/tsan_rtl.h.jj	2013-12-03 11:47:47.152489695 +0100
+++ libsanitizer/tsan/tsan_rtl.h	2013-12-03 11:48:01.888411307 +0100
@@ -732,13 +732,18 @@ void AcquireReleaseImpl(ThreadState *thr
 #if TSAN_DEBUG == 0
 // The caller may not create the stack frame for itself at all,
 // so we create a reserve stack frame for it (1024b must be enough).
+#ifdef __GCC_HAVE_DWARF2_CFI_ASM
+#define CFI_ADJUST_CFA_OFFSET(n) ".cfi_adjust_cfa_offset " #n ";"
+#else
+#define CFI_ADJUST_CFA_OFFSET(n)
+#endif
 #define HACKY_CALL(f) \
   __asm__ __volatile__("sub $1024, %%rsp;" \
-                       ".cfi_adjust_cfa_offset 1024;" \
+                       CFI_ADJUST_CFA_OFFSET(1024) \
                        ".hidden " #f "_thunk;" \
                        "call " #f "_thunk;" \
                        "add $1024, %%rsp;" \
-                       ".cfi_adjust_cfa_offset -1024;" \
+                       CFI_ADJUST_CFA_OFFSET(-1024) \
                        ::: "memory", "cc");
 #else
 #define HACKY_CALL(f) f()

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03 11:50       ` Jakub Jelinek
@ 2013-12-03 15:18         ` Konstantin Serebryany
  2013-12-03 15:47           ` Jakub Jelinek
  2013-12-03 21:49           ` Jakub Jelinek
  0 siblings, 2 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-03 15:18 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Tue, Dec 3, 2013 at 3:50 PM, Jakub Jelinek <jakub@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 asan&co 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, perhaps that is the only use model that llvm cares
> about, but for GCC that is definitely not acceptable.
>
>         Jakub

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03 15:18         ` Konstantin Serebryany
@ 2013-12-03 15:47           ` Jakub Jelinek
  2013-12-03 16:42             ` Jeff Law
  2013-12-03 21:49           ` Jakub Jelinek
  1 sibling, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-03 15:47 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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 asan&co 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03 15:47           ` Jakub Jelinek
@ 2013-12-03 16:42             ` Jeff Law
  2013-12-04  4:52               ` Konstantin Serebryany
  2013-12-04  5:08               ` Konstantin Serebryany
  0 siblings, 2 replies; 63+ messages in thread
From: Jeff Law @ 2013-12-03 16:42 UTC (permalink / raw)
  To: Jakub Jelinek, Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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 asan&co 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


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03 15:18         ` Konstantin Serebryany
  2013-12-03 15:47           ` Jakub Jelinek
@ 2013-12-03 21:49           ` Jakub Jelinek
  1 sibling, 0 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-03 21:49 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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.9	FIRST_32_SECOND_64(1136, 1712)
glibc 2.10.1			FIRST_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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03 16:42             ` Jeff Law
@ 2013-12-04  4:52               ` Konstantin Serebryany
  2013-12-04  5:08               ` Konstantin Serebryany
  1 sibling, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-04  4:52 UTC (permalink / raw)
  To: Jeff Law
  Cc: Jakub Jelinek, Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

>>>
>>> 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-03 16:42             ` Jeff Law
  2013-12-04  4:52               ` Konstantin Serebryany
@ 2013-12-04  5:08               ` Konstantin Serebryany
  2013-12-04 21:05                 ` Jeff Law
  1 sibling, 1 reply; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-04  5:08 UTC (permalink / raw)
  To: Jeff Law
  Cc: Jakub Jelinek, Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Tue, Dec 3, 2013 at 5:42 PM, Jeff Law <law@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 asan&co 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
>
>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 22:33         ` Jakub Jelinek
  2013-12-03  5:53           ` Konstantin Serebryany
@ 2013-12-04  5:29           ` Konstantin Serebryany
  2013-12-04  8:23             ` Jakub Jelinek
  2013-12-04 20:56             ` Jeff Law
  1 sibling, 2 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-04  5:29 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Mon, Dec 2, 2013 at 11:32 PM, Jakub Jelinek <jakub@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.000000000 -0500
> +++ sanitizer_platform_limits_linux.cc  2013-12-02 17:06:19.000000000 -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.000000000 -0500
> +++ sanitizer_platform_limits_posix.cc  2013-12-02 17:11:00.000000000 -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 from /usr/include/arpa/inet.h:23:0,
>                  from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
> /usr/include/netinet/in.h:37:5: note: previous declaration ‘<anonymous enum> IPPROTO_ICMP’
>      IPPROTO_ICMP = 1,    /* Internet Control Message Protocol.  */
>      ^
> 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:28:18: error: redeclaration of ‘IPPROTO_IGMP’
>    IPPROTO_IGMP = 2,  /* Internet Group Management Protocol */
>                   ^
> 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:39:5: note: previous declaration ‘<anonymous enum> IPPROTO_IGMP’
>      IPPROTO_IGMP = 2,    /* Internet Group Management Protocol. */
>      ^
> 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:29:18: error: redeclaration of ‘IPPROTO_IPIP’
>    IPPROTO_IPIP = 4,  /* IPIP tunnels (older KA9Q tunnels use 94) */
>                   ^
> 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:41:5: note: previous declaration ‘<anonymous enum> IPPROTO_IPIP’
>      IPPROTO_IPIP = 4,    /* IPIP tunnels (older KA9Q tunnels use 94).  */
>      ^
> 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:30:17: error: redeclaration of ‘IPPROTO_TCP’
>    IPPROTO_TCP = 6,  /* Transmission Control Protocol */
>                  ^
> 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:43:5: note: previous declaration ‘<anonymous enum> IPPROTO_TCP’
>      IPPROTO_TCP = 6,    /* Transmission Control Protocol.  */
>      ^
> 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:31:17: error: redeclaration of ‘IPPROTO_EGP’
>    IPPROTO_EGP = 8,  /* Exterior Gateway Protocol  */
>                  ^
> 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:45:5: note: previous declaration ‘<anonymous enum> IPPROTO_EGP’
>      IPPROTO_EGP = 8,    /* Exterior Gateway Protocol.  */
>      ^
> 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:32:17: error: redeclaration of ‘IPPROTO_PUP’
>    IPPROTO_PUP = 12,  /* PUP protocol    */
>                  ^
> 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:47:5: note: previous declaration ‘<anonymous enum> IPPROTO_PUP’
>      IPPROTO_PUP = 12,    /* PUP protocol.  */
>      ^
> 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:33:17: error: redeclaration of ‘IPPROTO_UDP’
>    IPPROTO_UDP = 17,  /* User Datagram Protocol  */
>                  ^
> 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:49:5: note: previous declaration ‘<anonymous enum> IPPROTO_UDP’
>      IPPROTO_UDP = 17,    /* User Datagram Protocol.  */
>      ^
> 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:34:17: error: redeclaration of ‘IPPROTO_IDP’
>    IPPROTO_IDP = 22,  /* XNS IDP protocol   */
>                  ^
> 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:51:5: note: previous declaration ‘<anonymous enum> IPPROTO_IDP’
>      IPPROTO_IDP = 22,    /* XNS IDP protocol.  */
>      ^
> 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:36:18: error: redeclaration of ‘IPPROTO_RSVP’
>    IPPROTO_RSVP = 46,  /* RSVP protocol   */
>                   ^
> 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:61:5: note: previous declaration ‘<anonymous enum> IPPROTO_RSVP’
>      IPPROTO_RSVP = 46,    /* Reservation Protocol.  */
>      ^
> 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:37:17: error: redeclaration of ‘IPPROTO_GRE’
>    IPPROTO_GRE = 47,  /* Cisco GRE tunnels (rfc 1701,1702) */
>                  ^
> 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:63:5: note: previous declaration ‘<anonymous enum> IPPROTO_GRE’
>      IPPROTO_GRE = 47,    /* General Routing Encapsulation.  */
>      ^
> 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:39:19: error: redeclaration of ‘IPPROTO_IPV6’
>    IPPROTO_IPV6  = 41,  /* IPv6-in-IPv4 tunnelling  */
>                    ^
> 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:55:5: note: previous declaration ‘<anonymous enum> IPPROTO_IPV6’
>      IPPROTO_IPV6 = 41,     /* IPv6 header.  */
>      ^
> 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:41:17: error: redeclaration of ‘IPPROTO_ESP’
>    IPPROTO_ESP = 50,            /* Encapsulation Security Payload protocol */
>                  ^
> 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:65:5: note: previous declaration ‘<anonymous enum> IPPROTO_ESP’
>      IPPROTO_ESP = 50,      /* encapsulating security payload.  */
>      ^
> 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:42:16: error: redeclaration of ‘IPPROTO_AH’
>    IPPROTO_AH = 51,             /* Authentication Header protocol       */
>                 ^
> 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:67:5: note: previous declaration ‘<anonymous enum> IPPROTO_AH’
>      IPPROTO_AH = 51,       /* authentication header.  */
>      ^
> 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:43:20: error: redeclaration of ‘IPPROTO_PIM’
>    IPPROTO_PIM    = 103,  /* Protocol Independent Multicast */
>                     ^
> 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:79:5: note: previous declaration ‘<anonymous enum> IPPROTO_PIM’
>      IPPROTO_PIM = 103,    /* Protocol Independent Multicast.  */
>      ^
> 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:45:20: error: redeclaration of ‘IPPROTO_COMP’
>    IPPROTO_COMP   = 108,                /* Compression Header protocol */
>                     ^
> 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:81:5: note: previous declaration ‘<anonymous enum> IPPROTO_COMP’
>      IPPROTO_COMP = 108,    /* Compression Header Protocol.  */
>      ^
> 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:46:20: error: redeclaration of ‘IPPROTO_SCTP’
>    IPPROTO_SCTP   = 132,  /* Stream Control Transport Protocol */
>                     ^
> 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:83:5: note: previous declaration ‘<anonymous enum> IPPROTO_SCTP’
>      IPPROTO_SCTP = 132,    /* Stream Control Transmission Protocol.  */
>      ^
> 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:48:18: error: redeclaration of ‘IPPROTO_RAW’
>    IPPROTO_RAW  = 255,  /* Raw IP packets   */
>                   ^
> 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:85:5: note: previous declaration ‘<anonymous enum> IPPROTO_RAW’
>      IPPROTO_RAW = 255,    /* Raw IP packets.  */
>      ^
> 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:49:3: error: redeclaration of ‘IPPROTO_MAX’
>    IPPROTO_MAX
>    ^
> 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:87:5: note: previous declaration ‘<anonymous enum> IPPROTO_MAX’
>      IPPROTO_MAX
>      ^
> 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:54:8: error: redefinition of ‘struct in_addr’
>  struct in_addr {
>         ^
> 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:137:8: error: previous definition of ‘struct in_addr’
>  struct in_addr
>         ^
> 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:113:8: error: redefinition of ‘struct ip_mreq’
>  struct ip_mreq
>         ^
> 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:244:8: error: previous definition of ‘struct ip_mreq’
>  struct ip_mreq
>         ^
> 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:119:8: error: redefinition of ‘struct ip_mreqn’
>  struct ip_mreqn
>         ^
> In file included from /usr/include/netinet/in.h:345:0,
>                  from /usr/include/arpa/inet.h:23,
>                  from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
> /usr/include/bits/in.h:89:8: error: previous definition of ‘struct ip_mreqn’
>  struct ip_mreqn
>         ^
> 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:126:8: error: redefinition of ‘struct ip_mreq_source’
>  struct ip_mreq_source {
>         ^
> 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:253:8: error: previous definition of ‘struct ip_mreq_source’
>  struct ip_mreq_source
>         ^
> 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:132:8: error: redefinition of ‘struct ip_msfilter’
>  struct ip_msfilter {
>         ^
> 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:300:8: error: previous definition of ‘struct ip_msfilter’
>  struct ip_msfilter
>         ^
> 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:144:8: error: redefinition of ‘struct group_req’
>  struct group_req
>         ^
> 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:277:8: error: previous definition of ‘struct group_req’
>  struct group_req
>         ^
> 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:150:8: error: redefinition of ‘struct group_source_req’
>  struct group_source_req
>         ^
> 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:286:8: error: previous definition of ‘struct group_source_req’
>  struct group_source_req
>         ^
> In file included from /usr/include/linux/mroute.h:5:0,
>                  from ../../../../libsani    from ../../../../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:20:
> /usr/include/bits/in.h:97:8: error: previous definition of ‘struct in_pktinfo’
>  struct in_pktinfo
>         ^
> 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:179:8: error: redefinition of ‘struct sockaddr_in’
>  struct sockaddr_in {
>         ^
> 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:219:8: error: previous definition of ‘struct sockaddr_in’
>  struct sockaddr_in
>         ^
> make[1]: *** [sanitizer_platform_limits_posix.lo] Error 1
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_rtl_thread.lo] Error 1
> ../../../../libsanitizer/tsan/tsan_rtl.h: Assembler messages:
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_rtl_mutex.lo] Error 1
> ...
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_interface_atomic.lo] Error 1
> ../../../../libsanitizer/tsan/tsan_rtl.cc: Assembler messages:
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.cc:400: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> ...
> ../../../../libsanitizer/tsan/tsan_rtl.h:763: Error: CFI instruction used without previous .cfi_startproc
> make[1]: *** [tsan_rtl.lo] Error 1
>
>
>
>         Jakub

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04  5:29           ` Konstantin Serebryany
@ 2013-12-04  8:23             ` Jakub Jelinek
  2013-12-04 12:49               ` Konstantin Serebryany
  2013-12-04 20:56             ` Jeff Law
  1 sibling, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-04  8:23 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04  8:23             ` Jakub Jelinek
@ 2013-12-04 12:49               ` Konstantin Serebryany
  2013-12-04 13:02                 ` Jakub Jelinek
  0 siblings, 1 reply; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-04 12:49 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

> 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 asan&co 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 asan&co 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 12:49               ` Konstantin Serebryany
@ 2013-12-04 13:02                 ` Jakub Jelinek
  2013-12-04 13:29                   ` Konstantin Serebryany
  0 siblings, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-04 13:02 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 13:02                 ` Jakub Jelinek
@ 2013-12-04 13:29                   ` Konstantin Serebryany
  2013-12-04 13:44                     ` Jakub Jelinek
  0 siblings, 1 reply; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-04 13:29 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Wed, Dec 4, 2013 at 5:02 PM, Jakub Jelinek <jakub@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 13:29                   ` Konstantin Serebryany
@ 2013-12-04 13:44                     ` Jakub Jelinek
  2013-12-04 14:17                       ` Konstantin Serebryany
  0 siblings, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-04 13:44 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 13:44                     ` Jakub Jelinek
@ 2013-12-04 14:17                       ` Konstantin Serebryany
  0 siblings, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-04 14:17 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Wed, Dec 4, 2013 at 5:44 PM, Jakub Jelinek <jakub@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04  5:29           ` Konstantin Serebryany
  2013-12-04  8:23             ` Jakub Jelinek
@ 2013-12-04 20:56             ` Jeff Law
  1 sibling, 0 replies; 63+ messages in thread
From: Jeff Law @ 2013-12-04 20:56 UTC (permalink / raw)
  To: Konstantin Serebryany, Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli, Marek Polacek,
	Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04  5:08               ` Konstantin Serebryany
@ 2013-12-04 21:05                 ` Jeff Law
  2013-12-05 12:23                   ` Konstantin Serebryany
  0 siblings, 1 reply; 63+ messages in thread
From: Jeff Law @ 2013-12-04 21:05 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: Jakub Jelinek, Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

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


^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 21:05                 ` Jeff Law
@ 2013-12-05 12:23                   ` Konstantin Serebryany
  2013-12-05 12:23                     ` Konstantin Serebryany
  0 siblings, 1 reply; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-05 12:23 UTC (permalink / raw)
  To: Jeff Law
  Cc: Jakub Jelinek, Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Thu, Dec 5, 2013 at 1:05 AM, Jeff Law <law@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
>
>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-05 12:23                   ` Konstantin Serebryany
@ 2013-12-05 12:23                     ` Konstantin Serebryany
  0 siblings, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-05 12:23 UTC (permalink / raw)
  To: Jeff Law
  Cc: Jakub Jelinek, Uros Bizjak, gcc-patches, H.J. Lu, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Evgeniy Stepanov, Alexey Samsonov

On Thu, Dec 5, 2013 at 4:22 PM, Konstantin Serebryany
<konstantin.s.serebryany@gmail.com> wrote:
> On Thu, Dec 5, 2013 at 1:05 AM, Jeff Law <law@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
>>
>>

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 15:49 libsanitizer merge from upstream r196090 Uros Bizjak
  2013-12-02 16:12 ` Konstantin Serebryany
@ 2014-01-10  9:24 ` Uros Bizjak
  2014-01-10  9:39   ` Jakub Jelinek
  2014-01-10 14:46   ` Uros Bizjak
  1 sibling, 2 replies; 63+ messages in thread
From: Uros Bizjak @ 2014-01-10  9:24 UTC (permalink / raw)
  To: gcc-patches
  Cc: Jakub Jelinek, H.J. Lu, Konstantin Serebryany, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov

On Mon, Dec 2, 2013 at 4:49 PM, Uros Bizjak <ubizjak@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.
0x00002aaaac0c5652 in malloc_consolidate () from /lib64/libc.so.6
(gdb) bt
#0  0x00002aaaac0c5652 in malloc_consolidate () from /lib64/libc.so.6
#1  0x00002aaaac0c6461 in mallopt () from /lib64/libc.so.6
#2  0x00002aaaaaf1a0e7 in __tsan::InitializeInterceptors () at
../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:2099
#3  0x00002aaaaaef8ab4 in __tsan::Initialize (thr=0x2aaaac3aa9e0
<main_arena>) at
../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_rtl.cc:223
#4  0x00002aaaaaf016a8 in ScopedInterceptor::ScopedInterceptor
(this=0x7fffffffda00, thr=0x2aaaacf0f580, fname=<optimized out>,
    pc=46912524510627) at
../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:173
#5  0x00002aaaaaf16d77 in __interceptor_memset (dst=0x7fffffffda68,
v=0, size=128)
    at ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:621
#6  0x00002aaaac5be5a3 in __pthread_initialize_minimal_internal ()
from /lib64/libpthread.so.0
#7  0x00002aaaac5bddd9 in _init () from /lib64/libpthread.so.0
#8  0x0000000000000000 in ?? ()

The 32 bit target executes tsan tests without problems.

[1] http://gcc.gnu.org/ml/gcc-testresults/2014-01/msg00689.html

Uros.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10  9:24 ` Uros Bizjak
@ 2014-01-10  9:39   ` Jakub Jelinek
  2014-01-10 11:27     ` Uros Bizjak
  2014-01-10 11:50     ` Maxim Ostapenko
  2014-01-10 14:46   ` Uros Bizjak
  1 sibling, 2 replies; 63+ messages in thread
From: Jakub Jelinek @ 2014-01-10  9:39 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: gcc-patches, H.J. Lu, Konstantin Serebryany, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10  9:39   ` Jakub Jelinek
@ 2014-01-10 11:27     ` Uros Bizjak
  2014-01-10 11:50     ` Maxim Ostapenko
  1 sibling, 0 replies; 63+ messages in thread
From: Uros Bizjak @ 2014-01-10 11:27 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: gcc-patches, H.J. Lu, Konstantin Serebryany, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov

On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinek <jakub@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(0x2abe3d840000, 4096, PROT_READ) = 0
mprotect(0x2abe3d638000, 4096, PROT_READ) = 0
mprotect(0x2abe3d420000, 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(0x7d0000000000, 1099511627776, PROT_NONE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS|MAP_NORESERVE, -1, 0) =
0x7d0000000000
mmap(0x7e0000000000, 12288, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7e0000000000

>> 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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10  9:39   ` Jakub Jelinek
  2014-01-10 11:27     ` Uros Bizjak
@ 2014-01-10 11:50     ` Maxim Ostapenko
  2014-01-10 15:20       ` Jakub Jelinek
  1 sibling, 1 reply; 63+ messages in thread
From: Maxim Ostapenko @ 2014-01-10 11:50 UTC (permalink / raw)
  To: Jakub Jelinek, Uros Bizjak
  Cc: gcc-patches, H.J. Lu, Konstantin Serebryany, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov, Yury Gribov, Slava Garbuzov


Hi all,

On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinek<jakub@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10  9:24 ` Uros Bizjak
  2014-01-10  9:39   ` Jakub Jelinek
@ 2014-01-10 14:46   ` Uros Bizjak
  1 sibling, 0 replies; 63+ messages in thread
From: Uros Bizjak @ 2014-01-10 14:46 UTC (permalink / raw)
  To: gcc-patches
  Cc: Jakub Jelinek, H.J. Lu, Konstantin Serebryany, Dodji Seketeli,
	Marek Polacek, Dmitry Vyukov

On Fri, Jan 10, 2014 at 10:23 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Dec 2, 2013 at 4:49 PM, Uros Bizjak <ubizjak@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.
> 0x00002aaaac0c5652 in malloc_consolidate () from /lib64/libc.so.6
> (gdb) bt
> #0  0x00002aaaac0c5652 in malloc_consolidate () from /lib64/libc.so.6
> #1  0x00002aaaac0c6461 in mallopt () from /lib64/libc.so.6
> #2  0x00002aaaaaf1a0e7 in __tsan::InitializeInterceptors () at
> ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:2099
> #3  0x00002aaaaaef8ab4 in __tsan::Initialize (thr=0x2aaaac3aa9e0
> <main_arena>) at
> ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_rtl.cc:223
> #4  0x00002aaaaaf016a8 in ScopedInterceptor::ScopedInterceptor
> (this=0x7fffffffda00, thr=0x2aaaacf0f580, fname=<optimized out>,
>     pc=46912524510627) at
> ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:173
> #5  0x00002aaaaaf16d77 in __interceptor_memset (dst=0x7fffffffda68,
> v=0, size=128)
>     at ../../../../gcc-svn/trunk/libsanitizer/tsan/tsan_interceptors.cc:621
> #6  0x00002aaaac5be5a3 in __pthread_initialize_minimal_internal ()
> from /lib64/libpthread.so.0
> #7  0x00002aaaac5bddd9 in _init () from /lib64/libpthread.so.0
> #8  0x0000000000000000 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 0x555555554000 < 0x7cf000000000)
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 0x555555554000 < 0x7cf000000000)
...

[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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10 11:50     ` Maxim Ostapenko
@ 2014-01-10 15:20       ` Jakub Jelinek
  2014-01-14  9:43         ` Konstantin Serebryany
  2014-02-04 14:44         ` Maxim Ostapenko
  0 siblings, 2 replies; 63+ messages in thread
From: Jakub Jelinek @ 2014-01-10 15:20 UTC (permalink / raw)
  To: Maxim Ostapenko
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Konstantin Serebryany,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov, Yury Gribov,
	Slava Garbuzov

On Fri, Jan 10, 2014 at 03:50:33PM +0400, Maxim Ostapenko wrote:
> On Fri, Jan 10, 2014 at 10:39 AM, Jakub Jelinek<jakub@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10 15:20       ` Jakub Jelinek
@ 2014-01-14  9:43         ` Konstantin Serebryany
  2014-02-04 14:44         ` Maxim Ostapenko
  1 sibling, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2014-01-14  9:43 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Maxim Ostapenko, Uros Bizjak, gcc-patches, H.J. Lu,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov, Yury Gribov,
	Slava Garbuzov

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 <jakub@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 Jelinek<jakub@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-01-10 15:20       ` Jakub Jelinek
  2014-01-14  9:43         ` Konstantin Serebryany
@ 2014-02-04 14:44         ` Maxim Ostapenko
  2014-02-04 14:54           ` Jakub Jelinek
  1 sibling, 1 reply; 63+ messages in thread
From: Maxim Ostapenko @ 2014-02-04 14:44 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Konstantin Serebryany,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov, Yury Gribov,
	Slava Garbuzov

[-- Attachment #1: Type: text/plain, Size: 332 bytes --]

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

[-- Attachment #2: default_options.diff --]
[-- Type: text/x-patch, Size: 438 bytes --]

2014-02-04  Max Ostapenko  <m.ostapenko@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>
 

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-02-04 14:44         ` Maxim Ostapenko
@ 2014-02-04 14:54           ` Jakub Jelinek
  2014-02-04 15:04             ` Maxim Ostapenko
  0 siblings, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2014-02-04 14:54 UTC (permalink / raw)
  To: Maxim Ostapenko
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Konstantin Serebryany,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov, Yury Gribov,
	Slava Garbuzov

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.ostapenko@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2014-02-04 14:54           ` Jakub Jelinek
@ 2014-02-04 15:04             ` Maxim Ostapenko
  0 siblings, 0 replies; 63+ messages in thread
From: Maxim Ostapenko @ 2014-02-04 15:04 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Uros Bizjak, gcc-patches, H.J. Lu, Konstantin Serebryany,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov, Yury Gribov,
	Slava Garbuzov

Commited in 207472.

-Maxim

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
@ 2014-01-14 10:10 Yuri Gribov
  0 siblings, 0 replies; 63+ messages in thread
From: Yuri Gribov @ 2014-01-14 10:10 UTC (permalink / raw)
  To: Uros Bizjak
  Cc: gcc-patches, Jakub Jelinek, H.J. Lu, Konstantin Serebryany,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
@ 2014-01-14 10:00 Yuri Gribov
  0 siblings, 0 replies; 63+ messages in thread
From: Yuri Gribov @ 2014-01-14 10:00 UTC (permalink / raw)
  To: Konstantin Serebryany, Jakub Jelinek
  Cc: Maxim Ostapenko, Uros Bizjak, gcc-patches, H.J. Lu,
	Dodji Seketeli, Marek Polacek, Dmitry Vyukov, Yury Gribov,
	Slava Garbuzov

> 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 17:01       ` H.J. Lu
  2013-12-04 17:28         ` Joseph S. Myers
@ 2013-12-05 12:40         ` Konstantin Serebryany
  1 sibling, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-05 12:40 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Jakub Jelinek, Ian Lance Taylor, FX, gcc-patches

> 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 11:52 Konstantin Serebryany
  2013-12-02 13:15 ` H.J. Lu
  2013-12-02 13:41 ` Marek Polacek
@ 2013-12-05  9:05 ` Jakub Jelinek
  2 siblings, 0 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-05  9:05 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: GCC Patches, Dodji Seketeli, Dmitry Vyukov, Marek Polacek

On Mon, Dec 02, 2013 at 03:52:09PM +0400, Konstantin Serebryany wrote:
> Expected ChangeLog entries:
> =========== libsanitizer/ChangeLog
> 
> 2013-12-0X  Kostya Serebryany  <kcc@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  <kcc@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:47   ` H.J. Lu
  2013-12-04 16:50     ` FX
  2013-12-04 16:58     ` Jakub Jelinek
@ 2013-12-05  8:58     ` Richard Biener
  2 siblings, 0 replies; 63+ messages in thread
From: Richard Biener @ 2013-12-05  8:58 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Ian Lance Taylor, FX, gcc-patches, Jakub Jelinek, Konstantin Serebryany

On Wed, Dec 4, 2013 at 5:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Dec 4, 2013 at 8:41 AM, Ian Lance Taylor <iant@google.com> wrote:
>> On Wed, Dec 4, 2013 at 8:04 AM, FX <fxcoudert@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:58     ` Jakub Jelinek
  2013-12-04 17:01       ` H.J. Lu
@ 2013-12-05  7:58       ` Konstantin Serebryany
  1 sibling, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-05  7:58 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: H.J. Lu, Ian Lance Taylor, FX, gcc-patches

On Wed, Dec 4, 2013 at 8:58 PM, Jakub Jelinek <jakub@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=196480&view=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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 17:28         ` Joseph S. Myers
@ 2013-12-04 21:48           ` H.J. Lu
  0 siblings, 0 replies; 63+ messages in thread
From: H.J. Lu @ 2013-12-04 21:48 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Jakub Jelinek, Ian Lance Taylor, FX, gcc-patches, Konstantin Serebryany

On Wed, Dec 4, 2013 at 9:28 AM, Joseph S. Myers <joseph@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 17:01       ` H.J. Lu
@ 2013-12-04 17:28         ` Joseph S. Myers
  2013-12-04 21:48           ` H.J. Lu
  2013-12-05 12:40         ` Konstantin Serebryany
  1 sibling, 1 reply; 63+ messages in thread
From: Joseph S. Myers @ 2013-12-04 17:28 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Jakub Jelinek, Ian Lance Taylor, FX, gcc-patches, Konstantin Serebryany

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
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:58     ` Jakub Jelinek
@ 2013-12-04 17:01       ` H.J. Lu
  2013-12-04 17:28         ` Joseph S. Myers
  2013-12-05 12:40         ` Konstantin Serebryany
  2013-12-05  7:58       ` Konstantin Serebryany
  1 sibling, 2 replies; 63+ messages in thread
From: H.J. Lu @ 2013-12-04 17:01 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ian Lance Taylor, FX, gcc-patches, Konstantin Serebryany

On Wed, Dec 4, 2013 at 8:58 AM, Jakub Jelinek <jakub@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:47   ` H.J. Lu
  2013-12-04 16:50     ` FX
@ 2013-12-04 16:58     ` Jakub Jelinek
  2013-12-04 17:01       ` H.J. Lu
  2013-12-05  7:58       ` Konstantin Serebryany
  2013-12-05  8:58     ` Richard Biener
  2 siblings, 2 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-04 16:58 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Ian Lance Taylor, FX, gcc-patches, Konstantin Serebryany

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:50     ` FX
@ 2013-12-04 16:54       ` H.J. Lu
  0 siblings, 0 replies; 63+ messages in thread
From: H.J. Lu @ 2013-12-04 16:54 UTC (permalink / raw)
  To: FX; +Cc: Ian Lance Taylor, gcc-patches, Jakub Jelinek, Konstantin Serebryany

On Wed, Dec 4, 2013 at 8:50 AM, FX <fxcoudert@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:47   ` H.J. Lu
@ 2013-12-04 16:50     ` FX
  2013-12-04 16:54       ` H.J. Lu
  2013-12-04 16:58     ` Jakub Jelinek
  2013-12-05  8:58     ` Richard Biener
  2 siblings, 1 reply; 63+ messages in thread
From: FX @ 2013-12-04 16:50 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Ian Lance Taylor, gcc-patches, Jakub Jelinek, Konstantin Serebryany

> 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:41 ` Ian Lance Taylor
  2013-12-04 16:47   ` FX
@ 2013-12-04 16:47   ` H.J. Lu
  2013-12-04 16:50     ` FX
                       ` (2 more replies)
  1 sibling, 3 replies; 63+ messages in thread
From: H.J. Lu @ 2013-12-04 16:47 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: FX, gcc-patches, Jakub Jelinek, Konstantin Serebryany

On Wed, Dec 4, 2013 at 8:41 AM, Ian Lance Taylor <iant@google.com> wrote:
> On Wed, Dec 4, 2013 at 8:04 AM, FX <fxcoudert@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:41 ` Ian Lance Taylor
@ 2013-12-04 16:47   ` FX
  2013-12-04 16:47   ` H.J. Lu
  1 sibling, 0 replies; 63+ messages in thread
From: FX @ 2013-12-04 16:47 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc-patches, Jakub Jelinek, Konstantin Serebryany

> 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-04 16:05 FX
@ 2013-12-04 16:41 ` Ian Lance Taylor
  2013-12-04 16:47   ` FX
  2013-12-04 16:47   ` H.J. Lu
  0 siblings, 2 replies; 63+ messages in thread
From: Ian Lance Taylor @ 2013-12-04 16:41 UTC (permalink / raw)
  To: FX; +Cc: gcc-patches, Jakub Jelinek, Konstantin Serebryany

On Wed, Dec 4, 2013 at 8:04 AM, FX <fxcoudert@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
@ 2013-12-04 16:05 FX
  2013-12-04 16:41 ` Ian Lance Taylor
  0 siblings, 1 reply; 63+ messages in thread
From: FX @ 2013-12-04 16:05 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek, Konstantin Serebryany

> > 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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 14:48     ` Jakub Jelinek
@ 2013-12-02 14:57       ` Marek Polacek
  0 siblings, 0 replies; 63+ messages in thread
From: Marek Polacek @ 2013-12-02 14:57 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Konstantin Serebryany, GCC Patches, Dodji Seketeli, Dmitry Vyukov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 14:41   ` Marek Polacek
  2013-12-02 14:47     ` Konstantin Serebryany
@ 2013-12-02 14:48     ` Jakub Jelinek
  2013-12-02 14:57       ` Marek Polacek
  1 sibling, 1 reply; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-02 14:48 UTC (permalink / raw)
  To: Marek Polacek
  Cc: Konstantin Serebryany, GCC Patches, Dodji Seketeli, Dmitry Vyukov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 14:41   ` Marek Polacek
@ 2013-12-02 14:47     ` Konstantin Serebryany
  2013-12-02 14:48     ` Jakub Jelinek
  1 sibling, 0 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-02 14:47 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches, Jakub Jelinek, Dodji Seketeli, Dmitry Vyukov

On Mon, Dec 2, 2013 at 6:41 PM, Marek Polacek <polacek@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 13:41 ` Marek Polacek
@ 2013-12-02 14:41   ` Marek Polacek
  2013-12-02 14:47     ` Konstantin Serebryany
  2013-12-02 14:48     ` Jakub Jelinek
  0 siblings, 2 replies; 63+ messages in thread
From: Marek Polacek @ 2013-12-02 14:41 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: GCC Patches, Jakub Jelinek, Dodji Seketeli, Dmitry Vyukov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 11:52 Konstantin Serebryany
  2013-12-02 13:15 ` H.J. Lu
@ 2013-12-02 13:41 ` Marek Polacek
  2013-12-02 14:41   ` Marek Polacek
  2013-12-05  9:05 ` Jakub Jelinek
  2 siblings, 1 reply; 63+ messages in thread
From: Marek Polacek @ 2013-12-02 13:41 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: GCC Patches, Jakub Jelinek, Dodji Seketeli, Dmitry Vyukov

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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 13:15 ` H.J. Lu
@ 2013-12-02 13:23   ` Jakub Jelinek
  0 siblings, 0 replies; 63+ messages in thread
From: Jakub Jelinek @ 2013-12-02 13:23 UTC (permalink / raw)
  To: H.J. Lu
  Cc: Konstantin Serebryany, GCC Patches, Dodji Seketeli,
	Dmitry Vyukov, Marek Polacek

On Mon, Dec 02, 2013 at 05:13:45AM -0800, H.J. Lu wrote:
> > =========== libsanitizer/ChangeLog
> >
> > 2013-12-0X  Kostya Serebryany  <kcc@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  <kcc@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

^ permalink raw reply	[flat|nested] 63+ messages in thread

* Re: libsanitizer merge from upstream r196090
  2013-12-02 11:52 Konstantin Serebryany
@ 2013-12-02 13:15 ` H.J. Lu
  2013-12-02 13:23   ` Jakub Jelinek
  2013-12-02 13:41 ` Marek Polacek
  2013-12-05  9:05 ` Jakub Jelinek
  2 siblings, 1 reply; 63+ messages in thread
From: H.J. Lu @ 2013-12-02 13:15 UTC (permalink / raw)
  To: Konstantin Serebryany
  Cc: GCC Patches, Jakub Jelinek, Dodji Seketeli, Dmitry Vyukov, Marek Polacek

On Mon, Dec 2, 2013 at 3:52 AM, Konstantin Serebryany
<konstantin.s.serebryany@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  <kcc@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  <kcc@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.

^ permalink raw reply	[flat|nested] 63+ messages in thread

* libsanitizer merge from upstream r196090
@ 2013-12-02 11:52 Konstantin Serebryany
  2013-12-02 13:15 ` H.J. Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 63+ messages in thread
From: Konstantin Serebryany @ 2013-12-02 11:52 UTC (permalink / raw)
  To: GCC Patches, Jakub Jelinek, Dodji Seketeli, Dmitry Vyukov, Marek Polacek

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

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  <kcc@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  <kcc@google.com>

        * c-c++-common/asan/null-deref-1.c: Update the test
        to match the fresh asan run-time.

--kcc

[-- Attachment #2: libsanitizer-196090.patch.gz --]
[-- Type: application/x-gzip, Size: 92093 bytes --]

^ permalink raw reply	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2014-02-04 15:04 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-02 15:49 libsanitizer merge from upstream r196090 Uros Bizjak
2013-12-02 16:12 ` Konstantin Serebryany
2013-12-02 16:26   ` Uros Bizjak
2013-12-02 16:43     ` Konstantin Serebryany
2013-12-02 16:54       ` Jakub Jelinek
2013-12-03 11:50       ` Jakub Jelinek
2013-12-03 15:18         ` Konstantin Serebryany
2013-12-03 15:47           ` Jakub Jelinek
2013-12-03 16:42             ` Jeff Law
2013-12-04  4:52               ` Konstantin Serebryany
2013-12-04  5:08               ` Konstantin Serebryany
2013-12-04 21:05                 ` Jeff Law
2013-12-05 12:23                   ` Konstantin Serebryany
2013-12-05 12:23                     ` Konstantin Serebryany
2013-12-03 21:49           ` Jakub Jelinek
2013-12-02 16:45     ` Jakub Jelinek
2013-12-02 17:00       ` Konstantin Serebryany
2013-12-02 17:05         ` Jakub Jelinek
2013-12-02 22:33         ` Jakub Jelinek
2013-12-03  5:53           ` Konstantin Serebryany
2013-12-03  7:34             ` Uros Bizjak
2013-12-04  5:29           ` Konstantin Serebryany
2013-12-04  8:23             ` Jakub Jelinek
2013-12-04 12:49               ` Konstantin Serebryany
2013-12-04 13:02                 ` Jakub Jelinek
2013-12-04 13:29                   ` Konstantin Serebryany
2013-12-04 13:44                     ` Jakub Jelinek
2013-12-04 14:17                       ` Konstantin Serebryany
2013-12-04 20:56             ` Jeff Law
2014-01-10  9:24 ` Uros Bizjak
2014-01-10  9:39   ` Jakub Jelinek
2014-01-10 11:27     ` Uros Bizjak
2014-01-10 11:50     ` Maxim Ostapenko
2014-01-10 15:20       ` Jakub Jelinek
2014-01-14  9:43         ` Konstantin Serebryany
2014-02-04 14:44         ` Maxim Ostapenko
2014-02-04 14:54           ` Jakub Jelinek
2014-02-04 15:04             ` Maxim Ostapenko
2014-01-10 14:46   ` Uros Bizjak
  -- strict thread matches above, loose matches on Subject: below --
2014-01-14 10:10 Yuri Gribov
2014-01-14 10:00 Yuri Gribov
2013-12-04 16:05 FX
2013-12-04 16:41 ` Ian Lance Taylor
2013-12-04 16:47   ` FX
2013-12-04 16:47   ` H.J. Lu
2013-12-04 16:50     ` FX
2013-12-04 16:54       ` H.J. Lu
2013-12-04 16:58     ` Jakub Jelinek
2013-12-04 17:01       ` H.J. Lu
2013-12-04 17:28         ` Joseph S. Myers
2013-12-04 21:48           ` H.J. Lu
2013-12-05 12:40         ` Konstantin Serebryany
2013-12-05  7:58       ` Konstantin Serebryany
2013-12-05  8:58     ` Richard Biener
2013-12-02 11:52 Konstantin Serebryany
2013-12-02 13:15 ` H.J. Lu
2013-12-02 13:23   ` Jakub Jelinek
2013-12-02 13:41 ` Marek Polacek
2013-12-02 14:41   ` Marek Polacek
2013-12-02 14:47     ` Konstantin Serebryany
2013-12-02 14:48     ` Jakub Jelinek
2013-12-02 14:57       ` Marek Polacek
2013-12-05  9:05 ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).