Re: [PATCH] Replace const_vector with match_operand in sse.md

2012-11-13 Thread Uros Bizjak
On Tue, Nov 13, 2012 at 3:23 PM, Andrey Turetskiy
 wrote:
>> BTW: Probably, pmulhrsw insn patterns can be merged, too, but this can
>> be a follow-up patch.
>
> Please, have a look at patch which merge pmulhrsw patterns.
>
> Changelog:
>
> 2012-11-13  Andrey Turetskiy  
>
>* config/i386/sse.md (*_pmulhrsw3): Merge
>*avx2_pmulhrswv16hi3 and *ssse3_pmulhrswv8hi3 into one pattern.

OK for mainline SVN, if bootstrapped and regression tested appropriately.

Thanks,
Uros.


Re: Patch: PR target/55142: [4.8 Regression] internal compiler error: in plus_constant, at explow.c:88

2012-11-13 Thread Uros Bizjak
On Tue, Nov 13, 2012 at 7:23 PM, H.J. Lu  wrote:

  Since x32 runs in 64-bit mode, for address -0x4300(%rax), hardware
  sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
  But x32 wants 32-bit -0x4300, not 64-bit -0x4300.  This patch
  uses 32-bit registers instead of 64-bit registers when displacement
  < -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
  still generate -16(%rsp) instead of -16(%esp).
 
  Tested it on Linux/x32.  OK to install?
 >>>
 >>> This problem uncovers a bug in the middle-end, so I guess it would be
 >>> better to fix it there.
 >>>
 >>> Uros.
 >>
 >> The problem is it isn't safe to transform
 >>
 >> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
 >>
 >> to
 >>
 >> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
 >>
 >> when Y is negative and its absolute value is greater than FOO.  However,
 >> we have to do this transformation since other parts of GCC depend on
 >> it.  If we revert the fix for
 >>
 >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
 >>
 >> we will get
 >>
 >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (internal compiler 
 >> error)
 >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (test for excess errors)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
 >> -funroll-all-loo
 >> ps -finline-functions  (internal compiler error)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
 >> -funroll-all-loo
 >> ps -finline-functions  (test for excess errors)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
 >> -funroll-loops
 >> (internal compiler error)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
 >> -funroll-loops
 >> (test for excess errors)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  
 >> (internal compi
 >> ler error)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (test 
 >> for exces
 >> s errors)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (internal compiler error)
 >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (test for excess errors)
 >> FAIL: gcc.dg/Warray-bounds.c (internal compiler error)
 >> FAIL: gcc.dg/Warray-bounds.c (test for excess errors)
 >>
 >> since we generate pseudo registers to convert SImode to DImode
 >> after reload.  Fixing it requires significant changes.
 >>
 >> This is only a problem for 64-bit register address since the symbolic
 >> address is 32-bit.  Using 32-bit base/index registers will work around
 >> this issue.
 >
 > This address
 >
 > (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
 >
 > is OK for x32 as long as Y, which is encoded as 32-bit immediate,
 > is zero-extend from 32-bit to 64-bit.  SImode address does it.
 > My patch optimizes it a little bit by using SImode address only
 > for Y < -16*1024*1024.

 I was wondering, why we operate with constant -16*1024*1024? Should we
 use 0x7FF instead? Since the MSB is always zero, we are safe.

>>>
>>> We can check 0x7FF, i.e., disp < 0.  I use -16*1024*1024, which
>>> is also used to check legitimate address displacements for PIC, to
>>> reduce code sizes for small negative displacements.  Or we can always
>>> encode negative displacements with zero-extension, including -1(%rsp).
>>>
 Please add a fat ??? comment, why we paper-over this issue and repost
 the latest patch. I got lost in all the versions :(

>>>
>>> Here is the updated patch.
>>>
>>> gcc/
>>>
>>> 2012-11-13  Eric Botcazou  
>>> H.J. Lu  
>>>
>>> PR target/55142
>>> * config/i386/i386.c (legitimize_pic_address): Properly handle
>>> REG + CONST.
>>> (ix86_print_operand_address): For x32, zero-extend negative
>>> displacement if it < -16*1024*1024.
>>>
>>> gcc/testsuite/
>>>
>>> 2012-11-13  H.J. Lu  
>>>
>>> PR target/55142
>>> * gcc.target/i386/pr55142-1.c: New file.
>>> * gcc.target/i386/pr55142-2.c: Likewise.
>>
>> OK, for mainline SVN (with the reservation that middle-end fix was not
>> found to be a viable solution, so target fix is papering-over real
>> issue. Let's wait for the next victim... ).
>
> That is true.
>
>> Oh, and please fix a typo of mine in the one line above the patch
>> hunk; the modifier for SI addresses should be 'k', not 'l'.
>
> Will do.
>
>> BTW: Do we need this patch also for 4.7? x32 address-mode is long by
>> default there, but the problem doesn't trigger.
>>
>
> This regression is triggered by revision 188008:
>
> http://gcc.gnu.org/ml/gcc-cvs/2012-05/msg00038.html
>
> aka the un-sign-extension of sizetype constants.  We can
> backport it to 4.7 branch if we want to

[PING] PR 54805: __gthread_tsd* in vxlib-tls.c

2012-11-13 Thread rbmj

On 11/5/2012 12:57 PM, rbmj wrote:

This removes warnings about implicit declarations and fixes one of the
function calls in vxlib-tls.c for vxworks targets.

I got the old prototypes from
http://gcc.gnu.org/ml/gcc-patches/2005-08/msg01314.html

See bug for further details.

Someone please comment or commit :)



Ping: http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00406.html

Robert Mason



Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Hans-Peter Nilsson
> From: Richard Henderson 
> Date: Wed, 14 Nov 2012 02:34:32 +0100

> On 11/13/2012 05:20 PM, Hans-Peter Nilsson wrote:
> > Right.  And, I think it's worth to repeat ;) that IMHO best to
> > there simply check that -faddress-sanitizer can compile
> > error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on
> > the target).  No target lists needed.
> 
> We can't do that, since the compiler hasn't been built
> at top-level configure time.  Obviously.

Yah, I was referring to configure-time in libsanitizer ...but
that's obviously not what you meant; I now see you referred to
toplevel configure.ac including various subdir/configure.tgt at
(toplevel) configure-time.

Delaying bail-out to target-library configure-time would
probably require things like a dummy buildsubdir/Makefile.  I
don't see at a glance other libraries doing that so might not
be worth pursuing.  Bah.

brgds, H-P



Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Richard Henderson
On 11/13/2012 05:20 PM, Hans-Peter Nilsson wrote:
> Right.  And, I think it's worth to repeat ;) that IMHO best to
> there simply check that -faddress-sanitizer can compile
> error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on
> the target).  No target lists needed.

We can't do that, since the compiler hasn't been built
at top-level configure time.  Obviously.


r~


Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Hans-Peter Nilsson
> From: Richard Henderson 
> Date: Tue, 13 Nov 2012 21:38:40 +0100

> On 11/13/2012 05:24 AM, Jakub Jelinek wrote:
> > Yes.  And it shouldn't be just based on target CPU, but also based
> > on target OS, I don't think libsanitizer supports anything but linux (glibc
> > + maybe android) right now, with some smaller or bigger tweaks it could
> > support darwin (but see the reports that it doesn't build there right now)
> > or mingw/cygwin? (but there is a PR that it doesn't build there).
> > So IMHO it should be a whitelist of supported targets with *) case
> > adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than
> > blacklist of few unsupported ones.  Can you please prepare a patch?
> 
> See how libatomic and libitm are structured.
> 
> The logic for what targets are supported belongs
> inside the library directory, and not at top-level.

Right.  And, I think it's worth to repeat ;) that IMHO best to
there simply check that -faddress-sanitizer can compile
error-free (i.e. that TARGET_ASAN_SHADOW_OFFSET is defined on
the target).  No target lists needed.

brgds, H-P


Re: [tsan] ThreadSanitizer instrumentation part

2012-11-13 Thread Wei Mi
Thanks for catching this. I update the patch.

Regards,
Wei.

On Tue, Nov 13, 2012 at 4:54 PM, Richard Henderson  wrote:
> On 11/13/2012 04:08 PM, Wei Mi wrote:
>> +extern void tsan_finish_file (void);
>> +
>> +#endif /* TREE_TSAN */
>> +/* ThreadSanitizer, a data race detector.
>> +   Copyright (C) 2011 Free Software Foundation, Inc.
>> +   Contributed by Dmitry Vyukov 
>
> Careful, you've got double applied patches there.
>
>
> r~
Index: gcc/passes.c
===
--- gcc/passes.c(revision 193482)
+++ gcc/passes.c(working copy)
@@ -1457,6 +1457,7 @@ init_optimization_passes (void)
   NEXT_PASS (pass_pre);
   NEXT_PASS (pass_sink_code);
   NEXT_PASS (pass_asan);
+  NEXT_PASS (pass_tsan);
   NEXT_PASS (pass_tree_loop);
{
  struct opt_pass **p = &pass_tree_loop.pass.sub;
@@ -1563,6 +1564,7 @@ init_optimization_passes (void)
 }
   NEXT_PASS (pass_lower_complex_O0);
   NEXT_PASS (pass_asan_O0);
+  NEXT_PASS (pass_tsan_O0);
   NEXT_PASS (pass_cleanup_eh);
   NEXT_PASS (pass_lower_resx);
   NEXT_PASS (pass_nrv);
Index: gcc/doc/invoke.texi
===
--- gcc/doc/invoke.texi (revision 193490)
+++ gcc/doc/invoke.texi (working copy)
@@ -289,7 +289,8 @@ Objective-C and Objective-C++ Dialects}.
 @item Debugging Options
 @xref{Debugging Options,,Options for Debugging Your Program or GCC}.
 @gccoptlist{-d@var{letters}  -dumpspecs  -dumpmachine  -dumpversion @gol
--faddress-sanitizer -fdbg-cnt-list -fdbg-cnt=@var{counter-value-list} @gol
+-fsanitize=@var{style} @gol
+-fdbg-cnt-list -fdbg-cnt=@var{counter-value-list} @gol
 -fdisable-ipa-@var{pass_name} @gol
 -fdisable-rtl-@var{pass_name} @gol
 -fdisable-rtl-@var{pass-name}=@var{range-list} @gol
@@ -309,6 +310,7 @@ Objective-C and Objective-C++ Dialects}.
 -fdump-tree-ssa@r{[}-@var{n}@r{]} -fdump-tree-pre@r{[}-@var{n}@r{]} @gol
 -fdump-tree-ccp@r{[}-@var{n}@r{]} -fdump-tree-dce@r{[}-@var{n}@r{]} @gol
 -fdump-tree-gimple@r{[}-raw@r{]} -fdump-tree-mudflap@r{[}-@var{n}@r{]} @gol
+-fdump-tree-tsan@r{[}-@var{n}@r{]} @gol
 -fdump-tree-dom@r{[}-@var{n}@r{]} @gol
 -fdump-tree-dse@r{[}-@var{n}@r{]} @gol
 -fdump-tree-phiprop@r{[}-@var{n}@r{]} @gol
@@ -382,8 +384,8 @@ Objective-C and Objective-C++ Dialects}.
 -floop-parallelize-all -flto -flto-compression-level @gol
 -flto-partition=@var{alg} -flto-report -fmerge-all-constants @gol
 -fmerge-constants -fmodulo-sched -fmodulo-sched-allow-regmoves @gol
--fmove-loop-invariants fmudflap -fmudflapir -fmudflapth -fno-branch-count-reg 
@gol
--fno-default-inline @gol
+-fmove-loop-invariants -fmudflap -fmudflapir -fmudflapth -fno-branch-count-reg 
@gol
+-fthread-sanitizer-ignore -fno-default-inline @gol
 -fno-defer-pop -fno-function-cse -fno-guess-branch-probability @gol
 -fno-inline -fno-math-errno -fno-peephole -fno-peephole2 @gol
 -fno-sched-interblock -fno-sched-spec -fno-signed-zeros @gol
@@ -5983,6 +5985,11 @@ appending @file{.dce} to the source file
 Dump each function after adding mudflap instrumentation.  The file name is
 made by appending @file{.mudflap} to the source file name.
 
+@item tsan
+@opindex fdump-tree-tsan
+Dump each function after adding ThreadSanitizer instrumentation.  The file 
name is
+made by appending @file{.tsan} to the source file name.
+
 @item sra
 @opindex fdump-tree-sra
 Dump each function after performing scalar replacement of aggregates.  The
@@ -6849,11 +6856,14 @@ assumptions based on that.
 
 The default is @option{-fzero-initialized-in-bss}.
 
-@item -faddress-sanitizer
-Enable AddressSanitizer, a fast memory error detector.
-Memory access instructions will be instrumented to detect
-out-of-bounds and use-after-free bugs. So far only heap bugs will be detected.
-See @uref{http://code.google.com/p/address-sanitizer/} for more details.
+@item -fsanitize=[address|thread]
+Enable AddressSanitizer or ThreadSanitizer. AddressSanitizer is a fast 
+memory error detector. Memory access instructions will be instrumented 
+to detect out-of-bounds, use-after-free, stack overflow and global 
+overflow bugs. ThreadSanitizer is a fast data race detector.  
+See @uref{http://code.google.com/p/address-sanitizer/} and 
+@uref{http://code.google.com/p/data-race-test/wiki/ThreadSanitizer} 
+for more details.
 
 @item -fmudflap -fmudflapth -fmudflapir
 @opindex fmudflap
@@ -6881,6 +6891,11 @@ instrumentation (and therefore faster ex
 some protection against outright memory corrupting writes, but allows
 erroneously read data to propagate within a program.
 
+@item -fthread-sanitizer-ignore
+@opindex fthread-sanitizer-ignore
+Add ThreadSanitizer instrumentation. Use @option{-fthread-sanitizer-ignore} to 
specify
+an ignore file. Refer to http://go/tsan for details.
+
 @item -fthread-jumps
 @opindex fthread-jumps
 Perform optimizations that check to see if a jump branches to a
Index: gcc/tsan.c
==

Re: ASAN merge...

2012-11-13 Thread Ramana Radhakrishnan
On Tue, Nov 13, 2012 at 10:41 PM, Andrew Pinski  wrote:
> On Tue, Nov 13, 2012 at 2:31 PM, Ramana Radhakrishnan
>  wrote:
>>
>>
>> On 13 Nov 2012, at 21:18, Konstantin Serebryany 
>>  wrote:
>>
>>> On Tue, Nov 13, 2012 at 8:21 AM, Diego Novillo  wrote:
 On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:
>
> This has broken the build on every Linux target that hasn't added
> the necessary cpu specific code to asan_linux.cc

 This should be fixed by Dodji's recent patch.  ASAN is not currently
 ported to any target other than x86/linux, so it should just be
>>>
>>> asan run-time (and the LLVM part) works on Mac and on ARM/Linux.
>>
>> And when you say ARM / Linux, has this been tested on older versions of the 
>> architecture or just v7-a ?
>
> And this arm is really arm32 and not ARM64 :).

Being an incorrigible pedant  :)

Technically the 32 bit and the 64 bit execution states are AArch32 and
AArch64 as documented in the glossary on infocenter [1] . So if you
are inventing a term to distinguish between the two I would prefer
AArch32 rather than arm32 and arm64 because they don't come in any
documentation neither in the compiler source nor in the documentation
as it exists today - I do realize history and legacy have a part to
play but if you want a new term .

Regards,
Ramana

1. 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.aeg0014e/ABCDEFGH.html


Re: [tsan] ThreadSanitizer instrumentation part

2012-11-13 Thread Richard Henderson
On 11/13/2012 04:08 PM, Wei Mi wrote:
> +extern void tsan_finish_file (void);
> +
> +#endif /* TREE_TSAN */
> +/* ThreadSanitizer, a data race detector.
> +   Copyright (C) 2011 Free Software Foundation, Inc.
> +   Contributed by Dmitry Vyukov 

Careful, you've got double applied patches there.


r~


Re: [PATCH, generic] New RTL primitive: `define_subst'

2012-11-13 Thread Mike Stump
On Nov 13, 2012, at 1:03 PM, Richard Henderson  wrote:
> Looking at all this, I'm wondering if we shouldn't split out all of this
> macro/include processing to a separate pass.  Perform the preprocessing
> once, early, leaving the processed result in the build directory.  Then
> run the original/traditional rtl reader on that when running the other
> rtl manipulation passes.

Kenny and I have a built-in generator that works by generating .md files in the 
build tree and using those as source.  My experience is that occasionally it 
handy to have those .md files, and it offers a way to better understand what is 
going on.  That said, I have only a slight preference for this.  I don't think 
it is the end of the world if this isn't done.  Occasionally, I want the 
existing iterator stuff to have an preprocess output mode…  though, only very 
occasionally.  So, I think that puts me in the nice to have but I don't think I 
would mandate it camp.

I do have some fear of customized macro languages that are really poor because 
they aren't generic enough and can never be, by design.  I can't help but 
wonder if there is a nice design that can be easily extended, if something is 
needed, yet, isn't as gross as all the generic solutions I've seen.  Sigh.


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread Paolo Bonzini
Il 14/11/2012 00:43, H.J. Lu ha scritto:
> This works.

Ok, then please test remove install-sh and friends and do
autoconf/automake again.  If it works, commit the result (I don't care
if you make it one or two commits).

Then, wait 24 hours for breakages and commit the multilib patch.

Paolo


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 3:31 PM, Paolo Bonzini  wrote:
> Il 14/11/2012 00:27, H.J. Lu ha scritto:
>> On Tue, Nov 13, 2012 at 3:19 PM, Paolo Bonzini  wrote:
>>> Il 14/11/2012 00:16, H.J. Lu ha scritto:
>> What has to be fixed about it?  Anything except 
>> AC_PREREQ/AC_CONFIG_AUX_DIR?
>>
>> I really would prefer to do it in the order I mentioned above.
 We also need

 [hjl@gnu-tools-1 libsanitizer]$ cat acinclude.m4
 dnl --
 dnl This whole bit snagged from libgfortran.

 sinclude(../libtool.m4)
 dnl The lines below arrange for aclocal not to bring an installed
 dnl libtool.m4 into aclocal.m4, while still arranging for automake to
 dnl add a definition of LIBTOOL to Makefile.in.
 ifelse(,,,[AC_SUBST(LIBTOOL)
 AC_DEFUN([AM_PROG_LIBTOOL])
 ])
 [hjl@gnu-tools-1 libsanitizer]$

 Otherwise, autoconf won't work.
>>>
>>> Sure, that's fine to include too.
>>>
>>
>> We need all changes in:
>>
>>   * acinclude.m4: New file.
>>   * Makefile.am (ACLOCAL_AMFLAGS): New.
>>   * configure.ac (AC_PREREQ): Set to 2.64.
>>   (AC_CONFIG_AUX_DIR): Set to "..".
>>   (--enable-version-specific-runtime-libs): New option.
>>   (AC_CANONICAL_SYSTEM): New.
>>   (AM_ENABLE_MULTILIB): Moved right after AM_INIT_AUTOMAKE.
>>   (toolexecdir): Support multilib.
>>   (toolexeclibdir): Likewise.
>>
>> Missing one will cause a problem.
>
> I don't understand why removing files needs
> --enable-version-specific-runtime-libs or multilibs.
>

This works.

-- 
H.J.
--2012-11-13  H.J. Lu  

PR other/55304
* acinclude.m4: New file.
* Makefile.am (ACLOCAL_AMFLAGS): New.
* configure.ac (AC_PREREQ): Set to 2.64.
(AC_CONFIG_AUX_DIR): Set to "..".
* Makefile.in: Regenerated.
* aclocal.m4: Likewise.
* configure: Likewise.
* asan/Makefile.in: Likewise.
* interception/Makefile.in: Likewise.
* sanitizer_common/Makefile.in: Likewise.

diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index b28eb32..91e3434 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -1,3 +1,5 @@
+ACLOCAL_AMFLAGS = -I .. -I ../config
+
 SUBDIRS = interception sanitizer_common asan

 # Work around what appears to be a GNU make bug handling MAKEFLAGS
diff --git a/libsanitizer/acinclude.m4 b/libsanitizer/acinclude.m4
new file mode 100644
index 000..8e606e7
--- /dev/null
+++ b/libsanitizer/acinclude.m4
@@ -0,0 +1,10 @@
+dnl --
+dnl This whole bit snagged from libgfortran.
+
+sinclude(../libtool.m4)
+dnl The lines below arrange for aclocal not to bring an installed
+dnl libtool.m4 into aclocal.m4, while still arranging for automake to
+dnl add a definition of LIBTOOL to Makefile.in.
+ifelse(,,,[AC_SUBST(LIBTOOL)
+AC_DEFUN([AM_PROG_LIBTOOL])
+])
diff --git a/libsanitizer/configure.ac b/libsanitizer/configure.ac
index 3f186c2..cd57a69 100644
--- a/libsanitizer/configure.ac
+++ b/libsanitizer/configure.ac
@@ -1,10 +1,10 @@
 #   -*- Autoconf -*-
 # Process this file with autoconf to produce a configure script.

-AC_PREREQ([2.68])
+AC_PREREQ([2.64])
 AC_INIT(package-unused, version-unused, libsanitizer)
 AC_CONFIG_SRCDIR([include/sanitizer/common_interface_defs.h])
-AC_CONFIG_AUX_DIR(.)
+AC_CONFIG_AUX_DIR(..)
 AM_INIT_AUTOMAKE(foreign)

 # Checks for programs.
-


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread Paolo Bonzini
Il 14/11/2012 00:27, H.J. Lu ha scritto:
> On Tue, Nov 13, 2012 at 3:19 PM, Paolo Bonzini  wrote:
>> Il 14/11/2012 00:16, H.J. Lu ha scritto:
> What has to be fixed about it?  Anything except 
> AC_PREREQ/AC_CONFIG_AUX_DIR?
>
> I really would prefer to do it in the order I mentioned above.
>>> We also need
>>>
>>> [hjl@gnu-tools-1 libsanitizer]$ cat acinclude.m4
>>> dnl --
>>> dnl This whole bit snagged from libgfortran.
>>>
>>> sinclude(../libtool.m4)
>>> dnl The lines below arrange for aclocal not to bring an installed
>>> dnl libtool.m4 into aclocal.m4, while still arranging for automake to
>>> dnl add a definition of LIBTOOL to Makefile.in.
>>> ifelse(,,,[AC_SUBST(LIBTOOL)
>>> AC_DEFUN([AM_PROG_LIBTOOL])
>>> ])
>>> [hjl@gnu-tools-1 libsanitizer]$
>>>
>>> Otherwise, autoconf won't work.
>>
>> Sure, that's fine to include too.
>>
> 
> We need all changes in:
> 
>   * acinclude.m4: New file.
>   * Makefile.am (ACLOCAL_AMFLAGS): New.
>   * configure.ac (AC_PREREQ): Set to 2.64.
>   (AC_CONFIG_AUX_DIR): Set to "..".
>   (--enable-version-specific-runtime-libs): New option.
>   (AC_CANONICAL_SYSTEM): New.
>   (AM_ENABLE_MULTILIB): Moved right after AM_INIT_AUTOMAKE.
>   (toolexecdir): Support multilib.
>   (toolexeclibdir): Likewise.
> 
> Missing one will cause a problem.

I don't understand why removing files needs
--enable-version-specific-runtime-libs or multilibs.

Paolo



Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 3:19 PM, Paolo Bonzini  wrote:
> Il 14/11/2012 00:16, H.J. Lu ha scritto:
>>> > What has to be fixed about it?  Anything except 
>>> > AC_PREREQ/AC_CONFIG_AUX_DIR?
>>> >
>>> > I really would prefer to do it in the order I mentioned above.
>> We also need
>>
>> [hjl@gnu-tools-1 libsanitizer]$ cat acinclude.m4
>> dnl --
>> dnl This whole bit snagged from libgfortran.
>>
>> sinclude(../libtool.m4)
>> dnl The lines below arrange for aclocal not to bring an installed
>> dnl libtool.m4 into aclocal.m4, while still arranging for automake to
>> dnl add a definition of LIBTOOL to Makefile.in.
>> ifelse(,,,[AC_SUBST(LIBTOOL)
>> AC_DEFUN([AM_PROG_LIBTOOL])
>> ])
>> [hjl@gnu-tools-1 libsanitizer]$
>>
>> Otherwise, autoconf won't work.
>
> Sure, that's fine to include too.
>

We need all changes in:

* acinclude.m4: New file.
* Makefile.am (ACLOCAL_AMFLAGS): New.
* configure.ac (AC_PREREQ): Set to 2.64.
(AC_CONFIG_AUX_DIR): Set to "..".
(--enable-version-specific-runtime-libs): New option.
(AC_CANONICAL_SYSTEM): New.
(AM_ENABLE_MULTILIB): Moved right after AM_INIT_AUTOMAKE.
(toolexecdir): Support multilib.
(toolexeclibdir): Likewise.

Missing one will cause a problem.

-- 
H.J.
---
diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index b28eb32..91e3434 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -1,3 +1,5 @@
+ACLOCAL_AMFLAGS = -I .. -I ../config
+
 SUBDIRS = interception sanitizer_common asan

 # Work around what appears to be a GNU make bug handling MAKEFLAGS
diff --git a/libsanitizer/acinclude.m4 b/libsanitizer/acinclude.m4
new file mode 100644
index 000..8e606e7
--- /dev/null
+++ b/libsanitizer/acinclude.m4
@@ -0,0 +1,10 @@
+dnl --
+dnl This whole bit snagged from libgfortran.
+
+sinclude(../libtool.m4)
+dnl The lines below arrange for aclocal not to bring an installed
+dnl libtool.m4 into aclocal.m4, while still arranging for automake to
+dnl add a definition of LIBTOOL to Makefile.in.
+ifelse(,,,[AC_SUBST(LIBTOOL)
+AC_DEFUN([AM_PROG_LIBTOOL])
+])
diff --git a/libsanitizer/configure.ac b/libsanitizer/configure.ac
index 3f186c2..47a44d3 100644
--- a/libsanitizer/configure.ac
+++ b/libsanitizer/configure.ac
@@ -1,11 +1,60 @@
 #   -*- Autoconf -*-
 # Process this file with autoconf to produce a configure script.

-AC_PREREQ([2.68])
+AC_PREREQ([2.64])
 AC_INIT(package-unused, version-unused, libsanitizer)
 AC_CONFIG_SRCDIR([include/sanitizer/common_interface_defs.h])
-AC_CONFIG_AUX_DIR(.)
+AC_CONFIG_AUX_DIR(..)
+
+AC_MSG_CHECKING([for --enable-version-specific-runtime-libs])
+AC_ARG_ENABLE(version-specific-runtime-libs,
+[  --enable-version-specific-runtime-libsSpecify that runtime libraries sho
uld be installed in a compiler-specific directory ],
+[case "$enableval" in
+ yes) version_specific_libs=yes ;;
+ no)  version_specific_libs=no ;;
+ *)   AC_MSG_ERROR([Unknown argument to enable/disable version-specific libs]);
;
+ esac],
+[version_specific_libs=no])
+AC_MSG_RESULT($version_specific_libs)
+
+# Do not delete or change the following two lines.  For why, see
+# http://gcc.gnu.org/ml/libstdc++/2003-07/msg00451.html
+AC_CANONICAL_SYSTEM
+target_alias=${target_alias-$host_alias}
+AC_SUBST(target_alias)
+
 AM_INIT_AUTOMAKE(foreign)
+AM_ENABLE_MULTILIB(, ..)
+
+# Calculate toolexeclibdir
+# Also toolexecdir, though it's only used in toolexeclibdir
+case ${version_specific_libs} in
+  yes)
+# Need the gcc compiler version to know where to install libraries
+# and header files if --enable-version-specific-runtime-libs option
+# is selected.
+toolexecdir='$(libdir)/gcc/$(target_alias)'
+toolexeclibdir='$(toolexecdir)/$(gcc_version)$(MULTISUBDIR)'
+;;
+  no)
+if test -n "$with_cross_host" &&
+   test x"$with_cross_host" != x"no"; then
+  # Install a library built with a cross compiler in tooldir, not libdir.
+  toolexecdir='$(exec_prefix)/$(target_alias)'
+  toolexeclibdir='$(toolexecdir)/lib'
+else
+  toolexecdir='$(libdir)/gcc-lib/$(target_alias)'
+  toolexeclibdir='$(libdir)'
+fi
+multi_os_directory=`$CC -print-multi-os-directory`
+case $multi_os_directory in
+  .) ;; # Avoid trailing /.
+  *) toolexeclibdir=$toolexeclibdir/$multi_os_directory ;;
+esac
+;;
+esac
+AC_SUBST(toolexecdir)
+AC_SUBST(toolexeclibdir)

 # Checks for programs.
 AC_PROG_CC
@@ -18,15 +67,6 @@ AM_PROG_LIBTOOL
 AC_SUBST(enable_shared)
 AC_SUBST(enable_static)

-#AM_ENABLE_MULTILIB(, ..)
-target_alias=${target_alias-$host_alias}
-AC_SUBST(target_alias)
-
-toolexecdir='$(libdir)/gcc-lib/$(target_alias)'
-toolexeclibdir='$(libdir)'
-AC_SUBST(toolexecdir)
-AC_SUBST(toolexeclibdir)
-
 AC_CONFIG_FILES([Makefile])

 AC_CONFIG_FILES(AC_FOREACH([DIR], [i

Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread Paolo Bonzini
Il 14/11/2012 00:16, H.J. Lu ha scritto:
>> > What has to be fixed about it?  Anything except 
>> > AC_PREREQ/AC_CONFIG_AUX_DIR?
>> >
>> > I really would prefer to do it in the order I mentioned above.
> We also need
> 
> [hjl@gnu-tools-1 libsanitizer]$ cat acinclude.m4
> dnl --
> dnl This whole bit snagged from libgfortran.
> 
> sinclude(../libtool.m4)
> dnl The lines below arrange for aclocal not to bring an installed
> dnl libtool.m4 into aclocal.m4, while still arranging for automake to
> dnl add a definition of LIBTOOL to Makefile.in.
> ifelse(,,,[AC_SUBST(LIBTOOL)
> AC_DEFUN([AM_PROG_LIBTOOL])
> ])
> [hjl@gnu-tools-1 libsanitizer]$
> 
> Otherwise, autoconf won't work.

Sure, that's fine to include too.

Paolo


Re: ASAN merge...

2012-11-13 Thread Konstantin Serebryany
+euge...@google.com

>>>
>>> asan run-time (and the LLVM part) works on Mac and on ARM/Linux.
>>
>> And when you say ARM / Linux, has this been tested on older versions of the 
>> architecture or just v7-a ?

Evgeniy is better equipped to answer this.
At some point we've been testing asan on the actual ARM32/Linux, now
we only run it on fresh Android devices.
I don't expect any serious problems with older versions of ARM32.

>
> And this arm is really arm32 and not ARM64 :).  People need to start
> calling it arm32 to make sure they don't get confused between the two
> different architectures.

done

--kcc


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 3:03 PM, Paolo Bonzini  wrote:
> Il 13/11/2012 23:45, H.J. Lu ha scritto:
>>> >
>>> > Let's first remove the files that are duplicated between the toplevel
>>> > and libsanitizer/.  This is preapproved after a successful bootstrap,
>>> > but please remember to rerun autoconf/automake in the libsanitizer/
>>> > directory.
>> We can't remove duplicated files without fixing configure.ac
>> first.
>
> What has to be fixed about it?  Anything except AC_PREREQ/AC_CONFIG_AUX_DIR?
>
> I really would prefer to do it in the order I mentioned above.

We also need

[hjl@gnu-tools-1 libsanitizer]$ cat acinclude.m4
dnl --
dnl This whole bit snagged from libgfortran.

sinclude(../libtool.m4)
dnl The lines below arrange for aclocal not to bring an installed
dnl libtool.m4 into aclocal.m4, while still arranging for automake to
dnl add a definition of LIBTOOL to Makefile.in.
ifelse(,,,[AC_SUBST(LIBTOOL)
AC_DEFUN([AM_PROG_LIBTOOL])
])
[hjl@gnu-tools-1 libsanitizer]$

Otherwise, autoconf won't work.

-- 
H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 2:40 PM, H.J. Lu  wrote:
> On Tue, Nov 13, 2012 at 2:39 PM, H.J. Lu  wrote:
>> On Tue, Nov 13, 2012 at 2:26 PM, Konstantin Serebryany
>>  wrote:
>>> H.J.,
>>> Question about this patch.
>>> Will it work if we simply replace
>>>#if __WORDSIZE == 64
>>> with
>>>   #ifdef x86_64
>>> ?
>>>
>>> Today, x86_64 is the only 64-bit architecture supported by asan
>>> run-time on linux anyway.
>>>
>>>
>>
>> That works for me.
>>
>
> It should be
>
> #ifdef __x86_64__

Sure. Done, LLVM r167883.


>
> not
>
> #ifdef x86_64
>
> --
> H.J.


Re: [Bug libstdc++/54075] [4.7.1] unordered_map insert still slower than 4.6.2

2012-11-13 Thread Paolo Carlini

On 11/13/2012 11:53 PM, Paolo Carlini wrote:
To summarize my intuitions are (again, leaving out the final 
technicalities)


a- std::hash specializations for scalar types -> no cache
b- std::hash specialization for for std::string (or maybe 
everything else, for simplicity) -> cache

c- user defined functor -> cache or not basing on __is_noexcept_hash
Alternately, if we want to stress the consistency of our behavior, just 
a single rule: __is_noexcept_hash. That means I expect that b- above is 
normally penalized performance-wise, but we can document the behavior, 
the user can simply instantiate with a user defined hash functor which 
doesn't have noexcept on the call operator and just forwards to 
std::hash and switch the container to caching.


Paolo.


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread Paolo Bonzini
Il 13/11/2012 23:45, H.J. Lu ha scritto:
>> >
>> > Let's first remove the files that are duplicated between the toplevel
>> > and libsanitizer/.  This is preapproved after a successful bootstrap,
>> > but please remember to rerun autoconf/automake in the libsanitizer/
>> > directory.
> We can't remove duplicated files without fixing configure.ac
> first.

What has to be fixed about it?  Anything except AC_PREREQ/AC_CONFIG_AUX_DIR?

I really would prefer to do it in the order I mentioned above.

Paolo



Re: [Bug libstdc++/54075] [4.7.1] unordered_map insert still slower than 4.6.2

2012-11-13 Thread Paolo Carlini

Hi,

On 11/13/2012 10:40 PM, François Dumont wrote:
Here is the proposal to remove shrinking feature from hash policy. I 
have also considered your remark regarding usage of lower_bound so 
_M_bkt_for_elements doesn't call _M_next_bkt (calling lower_bound) 
anymore. For 2 of the 3 calls it was only a source of redundant 
lower_bound invocations, in the last case I call _M_next_bkt explicitly.


2012-11-13  François Dumont  

* include/bits/hashtable_policy.h (_Prime_rehash_policy): Remove
automatic shrink.
(_Prime_rehash_policy::_M_bkt_for_elements): Do not call
_M_next_bkt anymore.
(_Prime_rehash_policy::_M_next_bkt): Move usage of
_S_growth_factor ...
(_Prime_rehash_policy::_M_need_rehash): ... here.
* include/bits/hashtable.h (_Hashtable<>): Adapt.

Tested under linux x86_64, normal and debug modes.
Thanks. First blush the patch looks good but please give us a few days 
to analyze the details of it, we don't want to make mistakes for 4.8.
Regarding performance, I have done a small evolution of the 54075.cc 
test proposed last time. It is now checking performance with and 
without cache of hash code. Result is:


54075.cc std::unordered_set 30 Foo insertions 
without cache   9r9u0s 13765616mem0pf
54075.cc std::unordered_set 30 Foo insertions 
with cache  14r   13u0s 18562064mem0pf
54075.cc std::tr1::unordered_set 30 Foo 
insertions without cache   9r8u1s 13765616mem0pf
54075.cc std::tr1::unordered_set 30 Foo 
insertions with cache  14r   13u0s 18561952mem0pf


So the difference of performance in this case only seems to come 
from caching the hash code or not. In reported use case default 
behavior of std::unordered_set is to cache hash codes and 
std::tr1::unordered_set not to cache it. We should perhaps review 
default behavior regarding caching the hash code. Perhaps cache it if 
the hash functor can throw and not cache it otherwise, not easy to 
find out what's best to do.
Ah good. I think we finally have nailed the core performance issue. And, 
as it turns out, I'm a bit confused about the logic we have in place now 
for the defaults: can you please summarize what we are doing and which 
are the trade offs (leaving out the technicalities having to do with the 
final types)? I think the most interesting are three:


1- std::hash
2- std::hash
3- user_defined_hash which cannot throw

In the first we should normally not cache; in the second, from a 
performance point of view (from the exception safety point of view we 
could do both, because std::hash doesn't throw anyway) it 
would be better to cache; the third case is rather tricky, because, like 
the case of std::string, from the exception safety point of view we 
could do both, thus it's purely a performance issue. Do I understand 
correctly that currently we handle 2- and 3- above in the same way, thus 
we cache? It seems to me that whereas that kind of default makes a lot 
of sense for std::string, doesn't necessarily make sense for everything 
else, and it seems to me that such kind of default makes a suboptimal 
use of the knowledge we have via __is_noexcept_hash that the functor 
doesn't throw. That seems instead a sort of user-hint to not cache! 
Given the unfortunate situation that the user has no way to explicitly 
pick a behavior when instantiating the container, we can imagine that he 
can anyway provide a strong if indirect hint by decorating or not with 
noexcept the call operator. We could even document that as part of our 
implementation defined behavior. How does it sound? Do we have a way to 
figure out what other implementations are doing? Outside std::hash, it 
should be pretty easy to instantiate with a special functor which 
internally keeps a counter... if we have evidence that the other best 
implementations don't cache for 3- we should definitely do the same.


To summarize my intuitions are (again, leaving out the final technicalities)

a- std::hash specializations for scalar types -> no cache
b- std::hash specialization for for std::string (or maybe 
everything else, for simplicity) -> cache

c- user defined functor -> cache or not basing on __is_noexcept_hash

Jon?

Thanks!
Paolo.


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 2:44 PM, Wei Mi  wrote:
> Sure, I can add the support. To run -mx32 test, we need to add x32
> multilib support for libasan first, right?
>

Compiling with -mx32 is trivial. I am working on the small patch.

> Wei.
>
> On Tue, Nov 13, 2012 at 2:14 PM, H.J. Lu  wrote:
>> On Tue, Nov 13, 2012 at 1:49 PM, Wei Mi  wrote:
 Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?


 --kcc
>>>
>>> Yes, it runs in all modes. O0/O1/O2/O3 + -m32/-m64, which is specified
>>> in asan.exp.
>>>
>>
>> Does it support -mx32?
>>
>>
>> --
>> H.J.



-- 
H.J.


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Wei Mi
Sure, I can add the support. To run -mx32 test, we need to add x32
multilib support for libasan first, right?

Thanks,
Wei.

On Tue, Nov 13, 2012 at 2:14 PM, H.J. Lu  wrote:
> On Tue, Nov 13, 2012 at 1:49 PM, Wei Mi  wrote:
>>> Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?
>>>
>>>
>>> --kcc
>>
>> Yes, it runs in all modes. O0/O1/O2/O3 + -m32/-m64, which is specified
>> in asan.exp.
>>
>
> Does it support -mx32?
>
>
> --
> H.J.


Re: ASAN merge...

2012-11-13 Thread Andrew Pinski
On Tue, Nov 13, 2012 at 2:31 PM, Ramana Radhakrishnan
 wrote:
>
>
> On 13 Nov 2012, at 21:18, Konstantin Serebryany 
>  wrote:
>
>> On Tue, Nov 13, 2012 at 8:21 AM, Diego Novillo  wrote:
>>> On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:

 This has broken the build on every Linux target that hasn't added
 the necessary cpu specific code to asan_linux.cc
>>>
>>> This should be fixed by Dodji's recent patch.  ASAN is not currently
>>> ported to any target other than x86/linux, so it should just be
>>
>> asan run-time (and the LLVM part) works on Mac and on ARM/Linux.
>
> And when you say ARM / Linux, has this been tested on older versions of the 
> architecture or just v7-a ?

And this arm is really arm32 and not ARM64 :).  People need to start
calling it arm32 to make sure they don't get confused between the two
different architectures.

Thanks
Andrew


>
>
>> But I'd suggest not to worry about any platforms other than
>> x86_{32,64}/Linux until the whole thing
>> (including tests) is fully integrated.
>>
>
>
> Ramana
>
>> --kcc
>>
>>> completely disabled until the other ports start showing up.
>>>
>>> Dodji is your patch committed?
>>>
>>>
>>> Diego.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 2:39 PM, H.J. Lu  wrote:
> On Tue, Nov 13, 2012 at 2:26 PM, Konstantin Serebryany
>  wrote:
>> H.J.,
>> Question about this patch.
>> Will it work if we simply replace
>>#if __WORDSIZE == 64
>> with
>>   #ifdef x86_64
>> ?
>>
>> Today, x86_64 is the only 64-bit architecture supported by asan
>> run-time on linux anyway.
>>
>>
>
> That works for me.
>

It should be

#ifdef __x86_64__

not

#ifdef x86_64

-- 
H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 2:26 PM, Konstantin Serebryany
 wrote:
> H.J.,
> Question about this patch.
> Will it work if we simply replace
>#if __WORDSIZE == 64
> with
>   #ifdef x86_64
> ?
>
> Today, x86_64 is the only 64-bit architecture supported by asan
> run-time on linux anyway.
>
>

That works for me.

Thanks.


-- 
H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 2:29 PM, Andrew Pinski  wrote:
> On Tue, Nov 13, 2012 at 2:26 PM, Konstantin Serebryany
>  wrote:
>> H.J.,
>> Question about this patch.
>> Will it work if we simply replace
>>#if __WORDSIZE == 64
>> with
>>   #ifdef x86_64
>> ?
>>
>> Today, x86_64 is the only 64-bit architecture supported by asan
>> run-time on linux anyway.
>
> Because x86_64 is defined even for x32.

Sure, this is why I suggest to use #if defined x86_64 instead of
#if __WORDSIZE == 64 || defined __x86_64__

>> And it is the only one
> currently supported does not mean there will be more in the future.

I hope there will be more, this is why #if defined x86_64 might be
preferable --
the code will explicitly be x86_64-specific and we'll know what to fix.

??

--kcc

>
> Thanks,
> Andrew Pinski
>
>>
>>
>> On Tue, Nov 13, 2012 at 1:53 PM, H.J. Lu  wrote:
>>> On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
>>>  wrote:
 On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
>> On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
>> > Diego Novillo  a écrit:
>> >
>> >> Patches to libsanitizer should be sent upstream.  We should only
>> >> contain a copy of the master in the LLVM repository.  There should be
>> >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
>> >>  I can't check ATM).
>> >
>> > No there are not, for the moment.  README.gcc just says where the
>> > sources the 'upstream' project is.
>> >
>>
>> What is the plan to add GCC specific support:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304
>>
>> and
>>
>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>
> CCing Wei, I don't know the details about the import.  To me it looks like
> that most or all of the libsanitizer/ level files (and
> libsanitizer/*/Makefile.{am,in}) don't originate from
> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
> should be changed to support multilibs, use the same libtool/autoconf/etc.
> versions as rest of gcc etc.


 Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
 files is a purely GCC thing.

>
> For changes to the files actually imported from LLVM I guess it depends on
> if google is going to accept such changes in the LLVM upstream.

 Yes, we are willing to support any changes that make libasan support
 more targets.
 We would prefer all patches to go through LLVM first, and then ported
 to GCC by copying files verbatim
 This is the only way we can cope with the two versions.
 (Wei, we will need the exact details for doing this in the README file)

>>>
>>> Could someone please check this patch:
>>>
>>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00951.html
>>>
>>> into upstream?
>>>
>>> Thanks.
>>>
>>>
>>> --
>>> H.J.


Re: Patch RFA: Fix -fpie -fpic

2012-11-13 Thread Joseph S. Myers
On Tue, 13 Nov 2012, Ian Lance Taylor wrote:

> Right now if you run "gcc -fpie -fpic" you get, in effect, "gcc -fpie".
> I think you should get "gcc -fpic".  In general I think that of the
> options -fpic, -fPIC, -fpie, -fPIE, -fno-pic, -fno-PIC, -fno-pie,
> -fno-PIE the compiler should act as though only the last of those
> options were specified.  That follows the usual formulat in which the
> last option wins.
> 
> This patch implements that.  Bootstrapped and tested on
> x86_64-unknown-linux-gnu.  OK for mainline?

OK.

-- 
Joseph S. Myers
jos...@codesourcery.com


Merge from trunk to gccgo branch

2012-11-13 Thread Ian Lance Taylor
I have merged trunk revision 193484 to the gccgo branch.

Ian


Re: ASAN merge...

2012-11-13 Thread Ramana Radhakrishnan


On 13 Nov 2012, at 21:18, Konstantin Serebryany 
 wrote:

> On Tue, Nov 13, 2012 at 8:21 AM, Diego Novillo  wrote:
>> On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:
>>> 
>>> This has broken the build on every Linux target that hasn't added
>>> the necessary cpu specific code to asan_linux.cc
>> 
>> This should be fixed by Dodji's recent patch.  ASAN is not currently
>> ported to any target other than x86/linux, so it should just be
> 
> asan run-time (and the LLVM part) works on Mac and on ARM/Linux.

And when you say ARM / Linux, has this been tested on older versions of the 
architecture or just v7-a ? 


> But I'd suggest not to worry about any platforms other than
> x86_{32,64}/Linux until the whole thing
> (including tests) is fully integrated.
> 


Ramana

> --kcc
> 
>> completely disabled until the other ports start showing up.
>> 
>> Dodji is your patch committed?
>> 
>> 
>> Diego.


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread Paolo Bonzini
Il 13/11/2012 18:03, H.J. Lu ha scritto:
>> > libsanitizer/ChangeLog.asan should probably be just libsanitizer/ChangeLog.
>> >
>> > In any case, I'd prefer if configury maintainers could review that.
> Sure.

Let's first remove the files that are duplicated between the toplevel
and libsanitizer/.  This is preapproved after a successful bootstrap,
but please remember to rerun autoconf/automake in the libsanitizer/
directory.

Then, IIUC, H.J.'s first patch can be applied; I'll try to review it
tomorrow, otherwise please ping me again 1-2 days after removing the
duplicated files.

Paolo


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Andrew Pinski
On Tue, Nov 13, 2012 at 2:26 PM, Konstantin Serebryany
 wrote:
> H.J.,
> Question about this patch.
> Will it work if we simply replace
>#if __WORDSIZE == 64
> with
>   #ifdef x86_64
> ?
>
> Today, x86_64 is the only 64-bit architecture supported by asan
> run-time on linux anyway.

Because x86_64 is defined even for x32.  And it is the only one
currently supported does not mean there will be more in the future.

Thanks,
Andrew Pinski

>
>
> On Tue, Nov 13, 2012 at 1:53 PM, H.J. Lu  wrote:
>> On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
>>  wrote:
>>> On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
 On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
> On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
> > Diego Novillo  a écrit:
> >
> >> Patches to libsanitizer should be sent upstream.  We should only
> >> contain a copy of the master in the LLVM repository.  There should be
> >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
> >>  I can't check ATM).
> >
> > No there are not, for the moment.  README.gcc just says where the
> > sources the 'upstream' project is.
> >
>
> What is the plan to add GCC specific support:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304
>
> and
>
> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html

 CCing Wei, I don't know the details about the import.  To me it looks like
 that most or all of the libsanitizer/ level files (and
 libsanitizer/*/Makefile.{am,in}) don't originate from
 llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
 should be changed to support multilibs, use the same libtool/autoconf/etc.
 versions as rest of gcc etc.
>>>
>>>
>>> Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
>>> files is a purely GCC thing.
>>>

 For changes to the files actually imported from LLVM I guess it depends on
 if google is going to accept such changes in the LLVM upstream.
>>>
>>> Yes, we are willing to support any changes that make libasan support
>>> more targets.
>>> We would prefer all patches to go through LLVM first, and then ported
>>> to GCC by copying files verbatim
>>> This is the only way we can cope with the two versions.
>>> (Wei, we will need the exact details for doing this in the README file)
>>>
>>
>> Could someone please check this patch:
>>
>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00951.html
>>
>> into upstream?
>>
>> Thanks.
>>
>>
>> --
>> H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Konstantin Serebryany
H.J.,
Question about this patch.
Will it work if we simply replace
   #if __WORDSIZE == 64
with
  #ifdef x86_64
?

Today, x86_64 is the only 64-bit architecture supported by asan
run-time on linux anyway.


On Tue, Nov 13, 2012 at 1:53 PM, H.J. Lu  wrote:
> On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
>  wrote:
>> On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
>>> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
 On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
 > Diego Novillo  a écrit:
 >
 >> Patches to libsanitizer should be sent upstream.  We should only
 >> contain a copy of the master in the LLVM repository.  There should be
 >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
 >>  I can't check ATM).
 >
 > No there are not, for the moment.  README.gcc just says where the
 > sources the 'upstream' project is.
 >

 What is the plan to add GCC specific support:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304

 and

 http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>>>
>>> CCing Wei, I don't know the details about the import.  To me it looks like
>>> that most or all of the libsanitizer/ level files (and
>>> libsanitizer/*/Makefile.{am,in}) don't originate from
>>> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
>>> should be changed to support multilibs, use the same libtool/autoconf/etc.
>>> versions as rest of gcc etc.
>>
>>
>> Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
>> files is a purely GCC thing.
>>
>>>
>>> For changes to the files actually imported from LLVM I guess it depends on
>>> if google is going to accept such changes in the LLVM upstream.
>>
>> Yes, we are willing to support any changes that make libasan support
>> more targets.
>> We would prefer all patches to go through LLVM first, and then ported
>> to GCC by copying files verbatim
>> This is the only way we can cope with the two versions.
>> (Wei, we will need the exact details for doing this in the README file)
>>
>
> Could someone please check this patch:
>
> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00951.html
>
> into upstream?
>
> Thanks.
>
>
> --
> H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Xinliang David Li
On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
>> On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
>> > Diego Novillo  a écrit:
>> >
>> >> Patches to libsanitizer should be sent upstream.  We should only
>> >> contain a copy of the master in the LLVM repository.  There should be
>> >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
>> >>  I can't check ATM).
>> >
>> > No there are not, for the moment.  README.gcc just says where the
>> > sources the 'upstream' project is.
>> >
>>
>> What is the plan to add GCC specific support:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304
>>
>> and
>>
>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>
> CCing Wei, I don't know the details about the import.  To me it looks like
> that most or all of the libsanitizer/ level files (and
> libsanitizer/*/Makefile.{am,in}) don't originate from
> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
> should be changed to support multilibs, use the same libtool/autoconf/etc.
> versions as rest of gcc etc.
>

Agree.


> For changes to the files actually imported from LLVM I guess it depends on
> if google is going to accept such changes in the LLVM upstream.  For
> unsupported targets we want to add target-libsanitizer into noconfigdirs
> in toplevel configure.
>

These would be files under libsanitizer/asan, libsanitizer/tsan,
libsanitizer/sanitizer_common, libsanitizer/include directories.

For changes in those directories, why not sending the patch to Kosyta
and Dmitry, whom I assume will help review the patch and do the commit
properly?

> Note that just making libsanitizer to build on some architecture is not
> enough for full ASAN support, one needs to also add the target hook with
> mem>>3 to shadow offset, and I guess review all other spots where
> libsanitizer uses __i386__ or __x86_64__ macros.
>
> I'd also say that using sanitizer_atomic_clang.h for GCC is not a good
> idea, now that GCC 4.7+ has __atomic_* support that should be usable
> for most of the __sanitizer::atomic* stuff.

Right, but that can be changed.

thanks,

David


>
> Jakub


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 1:55 PM, Andrew Pinski  wrote:
> On Tue, Nov 13, 2012 at 1:24 PM, Konstantin Serebryany
>  wrote:
>>> I migrate a test in the third category. Please help to check whether
>>> it is ok. Then I will migrate the left. The files added are as follows
>>> and attached. (Please forgive I use -fasan in asan.exp because I use
>>> an old repository to try the migration)
>>>
>>> gcc/testsuite/lib/asan-dg.exp:
>>> copy from libmudflap/lib/mfdg.exp, it rewrites the proc
>>> dg-test, to enable dg-output check in xfail status.  Existing
>>> dg-output check in /usr/share/dejagnu/dg.exp only work in pass status
>>> but not in xfail status.
>>> gcc/testsuite/g++.dg/asan/asan.exp
>>> gcc/testsuite/g++.dg/asan/memcmp_test.cc
>>>
>>> A problem: llvm provides llvm-symbolizer and asan_symbolize.py to map
>>> virtual address to its parent func name, which is used in "CHECK:
>>> {{#0.*memcmp}}" in llvm test below.  I don't know whether gcc provides
>>> similar tools. How to deal with it?
>>
>> You can not use llvm-symbolizer, but maybe you can use asan_symbolize.py?
>> asan_symbolize.py is basically a wrapper for addr2line, a libbfd-based tool.
>> Or is python not allowed in gcc testing infrastructure?
>> Then you can probably write a simple script in perl that does the same.
>
> Why not just use TCL for this.  Since there is already a scripting
> language with dejagnu via TCL.

Any scripting language will work.

>
> Thanks,
> Andrew Pinski
>
>>
>>>
>>> memcmp_test.cc in llvm:
>>>
>>>   1 // RUN: %clangxx_asan -m64 -O0 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   2 // RUN: %clangxx_asan -m64 -O1 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   3 // RUN: %clangxx_asan -m64 -O2 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   4 // RUN: %clangxx_asan -m64 -O3 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   5 // RUN: %clangxx_asan -m32 -O0 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   6 // RUN: %clangxx_asan -m32 -O1 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   7 // RUN: %clangxx_asan -m32 -O2 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   8 // RUN: %clangxx_asan -m32 -O3 %s -o %t %lib && %t 2>&1 |
>>> %symbolize | FileCheck %s
>>>   9
>>>  10 #include 
>>>  11 int main(int argc, char **argv) {
>>>  12   char a1[] = {argc, 2, 3, 4};
>>>  13   char a2[] = {1, 2*argc, 3, 4};
>>>  14   int res = memcmp(a1, a2, 4 + argc);  // BOOM
>>>  15   // CHECK: AddressSanitizer stack-buffer-overflow
>>>  16   // CHECK: {{#0.*memcmp}}
>>>  17   // CHECK: {{#1.*main}}
>>>  18   return res;
>>>  19 }
>>>
>>> memcmp_test.cc planed for gcc:
>>>
>>>   1 #include 
>>>   2 int main(int argc, char **argv) {
>>>   3   char a1[] = {argc, 2, 3, 4};
>>>   4   char a2[] = {1, 2*argc, 3, 4};
>>>   5   int res = memcmp(a1, a2, 4 + argc);  // BOOM
>>>   6   return res;
>>>   7 }
>>>   8
>>>   9 /*  { dg-output "AddressSanitizer stack-buffer-overflow.*" } */
>>>  10 /*  { dg-do run { xfail *-*-* } } */
>>
>> Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?
>>
>>
>> --kcc
>>
>>>  11
>>>
>>> Thanks,
>>> Wei.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 1:53 PM, H.J. Lu  wrote:
> On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
>  wrote:
>> On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
>>> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
 On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
 > Diego Novillo  a écrit:
 >
 >> Patches to libsanitizer should be sent upstream.  We should only
 >> contain a copy of the master in the LLVM repository.  There should be
 >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
 >>  I can't check ATM).
 >
 > No there are not, for the moment.  README.gcc just says where the
 > sources the 'upstream' project is.
 >

 What is the plan to add GCC specific support:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304

 and

 http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>>>
>>> CCing Wei, I don't know the details about the import.  To me it looks like
>>> that most or all of the libsanitizer/ level files (and
>>> libsanitizer/*/Makefile.{am,in}) don't originate from
>>> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
>>> should be changed to support multilibs, use the same libtool/autoconf/etc.
>>> versions as rest of gcc etc.
>>
>>
>> Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
>> files is a purely GCC thing.
>>
>>>
>>> For changes to the files actually imported from LLVM I guess it depends on
>>> if google is going to accept such changes in the LLVM upstream.
>>
>> Yes, we are willing to support any changes that make libasan support
>> more targets.
>> We would prefer all patches to go through LLVM first, and then ported
>> to GCC by copying files verbatim
>> This is the only way we can cope with the two versions.
>> (Wei, we will need the exact details for doing this in the README file)
>>
>
> Could someone please check this patch:
>
> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00951.html
>
> into upstream?

Let me do this.

--kcc


>
> Thanks.
>
>
> --
> H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 1:46 PM, Andrew Pinski  wrote:
> On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
>  wrote:
>> On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
>>> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
 On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
 > Diego Novillo  a écrit:
 >
 >> Patches to libsanitizer should be sent upstream.  We should only
 >> contain a copy of the master in the LLVM repository.  There should be
 >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
 >>  I can't check ATM).
 >
 > No there are not, for the moment.  README.gcc just says where the
 > sources the 'upstream' project is.
 >

 What is the plan to add GCC specific support:

 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304

 and

 http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>>>
>>> CCing Wei, I don't know the details about the import.  To me it looks like
>>> that most or all of the libsanitizer/ level files (and
>>> libsanitizer/*/Makefile.{am,in}) don't originate from
>>> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
>>> should be changed to support multilibs, use the same libtool/autoconf/etc.
>>> versions as rest of gcc etc.
>>
>>
>> Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
>> files is a purely GCC thing.
>>
>>>
>>> For changes to the files actually imported from LLVM I guess it depends on
>>> if google is going to accept such changes in the LLVM upstream.
>>
>> Yes, we are willing to support any changes that make libasan support
>> more targets.
>> We would prefer all patches to go through LLVM first, and then ported
>> to GCC by copying files verbatim
>> This is the only way we can cope with the two versions.
>> (Wei, we will need the exact details for doing this in the README file)
>
> I rather have it the other way around; like how libffi is handled.
> Since GCC has many more targets and a different schedule than LLVM.

That may work too, but the very moment that the two versions get out of sync
we lose the ability to port the new version from LLVM to GCC with
reasonable effort.
So, whoever applies a change to the gcc version will need to make sure
the same change applies upstream.

>
> Thanks,
> Andrew
>
>>
>> --kcc
>>
>>
>>> For
>>> unsupported targets we want to add target-libsanitizer into noconfigdirs
>>> in toplevel configure.
>>>
>>> Note that just making libsanitizer to build on some architecture is not
>>> enough for full ASAN support, one needs to also add the target hook with
>>> mem>>3 to shadow offset, and I guess review all other spots where
>>> libsanitizer uses __i386__ or __x86_64__ macros.
>>
>>
>>
>>>
>>> I'd also say that using sanitizer_atomic_clang.h for GCC is not a good
>>> idea, now that GCC 4.7+ has __atomic_* support that should be usable
>>> for most of the __sanitizer::atomic* stuff.
>>>
>>> Jakub


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 1:49 PM, Wei Mi  wrote:
>> Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?
>>
>>
>> --kcc
>
> Yes, it runs in all modes. O0/O1/O2/O3 + -m32/-m64, which is specified
> in asan.exp.
>

Does it support -mx32?


-- 
H.J.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Xinliang David Li
On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
> Diego Novillo  a écrit:
>
>> Patches to libsanitizer should be sent upstream.  We should only
>> contain a copy of the master in the LLVM repository.  There should be
>> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
>>  I can't check ATM).
>
> No there are not, for the moment.  README.gcc just says where the
> sources the 'upstream' project is.
>

Right -- and as it shows, this is not a simple matter and requires
joint wisdom from the community  :).

David



> --
> Dodji


Re: [4/8] Add bit_field_mode_iterator

2012-11-13 Thread Eric Botcazou
> Now that we're in C++, I think we should be using iterators that are much
> more in the style of libstdc++.  I agree that the .next interface used here
> is a bit confusing.
> 
> I'd expect to see something like
> 
>   for (bit_field_mode_iterator i(x,y,z); i ; ++i)
> {
>   enum machine_mode mode = *i;
>   ...
> }

I pondered on that for half an hour. :-)  But the amount of stuff you need to 
write to make it work in this particular case will make the implementation 
convoluted and bloated for no obvious gains IMO.

-- 
Eric Botcazou


Re: [PATCH v3] Add support for sparc compare-and-branch

2012-11-13 Thread David Miller
From: Eric Botcazou 
Date: Tue, 13 Nov 2012 22:50:49 +0100

>> Thanks for finding this, that's definitely incorrect behavior.  I bet there
>> is some unintended override triggered by sparc4 selection, and I'll go and
>> fix that soon.
> 
> You're welcome.  That's the reason why I needed to go the ASM_ARCH way, the 
> straightforward approach would have put the -32/-64 first.

Right.  And meanwhile I found the problem, there is this code block in
the Sparc option parser of GAS that goes:

case OPTION_XARCH:
#ifdef OBJ_ELF
  if (strncmp (arg, "v9", 2) != 0)
md_parse_option (OPTION_32, NULL);
  else
md_parse_option (OPTION_64, NULL);
#endif
  /* Fall through.  */

And that's where the unexpected size override is happening.  That test
simply needs to be adjusted and I'll try to sort that out tonight.

> And I managed to miss a substitution in the previous patch, so please drop it 
> and use the attached one instead.

Thanks for this, I was just starting to work on integrating our work
together and debugging the result.


Re: User directed Function Multiversioning via Function Overloading (issue5752064)

2012-11-13 Thread Sriraman Tallam
Patch committed now after making the changes.

Thanks,
-Sri.

On Mon, Nov 12, 2012 at 6:39 PM, Jason Merrill  wrote:
> On 11/12/2012 08:11 PM, Sriraman Tallam wrote:
>>
>> +   && !targetm.target_option.function_versions (fn,
>> +TREE_PURPOSE (match)))
>
>
> The second argument should be lined up with the left paren if it's on a
> different line.  Perhaps formatting this as
>
> && !(targetm.target_option.function_versions
>  (fn, TREE_PURPOSE (match
>
> would be better.
>
>> +  error_at (input_location, "Use of multiversioned function "
>> +   "Multiversioning needs ifunc which is not supported "
>
>
> We don't capitalize the first letter of a diagnostic.
>
> OK with those changes.
>
> Jason
>


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Andrew Pinski
On Tue, Nov 13, 2012 at 1:24 PM, Konstantin Serebryany
 wrote:
>> I migrate a test in the third category. Please help to check whether
>> it is ok. Then I will migrate the left. The files added are as follows
>> and attached. (Please forgive I use -fasan in asan.exp because I use
>> an old repository to try the migration)
>>
>> gcc/testsuite/lib/asan-dg.exp:
>> copy from libmudflap/lib/mfdg.exp, it rewrites the proc
>> dg-test, to enable dg-output check in xfail status.  Existing
>> dg-output check in /usr/share/dejagnu/dg.exp only work in pass status
>> but not in xfail status.
>> gcc/testsuite/g++.dg/asan/asan.exp
>> gcc/testsuite/g++.dg/asan/memcmp_test.cc
>>
>> A problem: llvm provides llvm-symbolizer and asan_symbolize.py to map
>> virtual address to its parent func name, which is used in "CHECK:
>> {{#0.*memcmp}}" in llvm test below.  I don't know whether gcc provides
>> similar tools. How to deal with it?
>
> You can not use llvm-symbolizer, but maybe you can use asan_symbolize.py?
> asan_symbolize.py is basically a wrapper for addr2line, a libbfd-based tool.
> Or is python not allowed in gcc testing infrastructure?
> Then you can probably write a simple script in perl that does the same.

Why not just use TCL for this.  Since there is already a scripting
language with dejagnu via TCL.

Thanks,
Andrew Pinski

>
>>
>> memcmp_test.cc in llvm:
>>
>>   1 // RUN: %clangxx_asan -m64 -O0 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   2 // RUN: %clangxx_asan -m64 -O1 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   3 // RUN: %clangxx_asan -m64 -O2 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   4 // RUN: %clangxx_asan -m64 -O3 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   5 // RUN: %clangxx_asan -m32 -O0 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   6 // RUN: %clangxx_asan -m32 -O1 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   7 // RUN: %clangxx_asan -m32 -O2 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   8 // RUN: %clangxx_asan -m32 -O3 %s -o %t %lib && %t 2>&1 |
>> %symbolize | FileCheck %s
>>   9
>>  10 #include 
>>  11 int main(int argc, char **argv) {
>>  12   char a1[] = {argc, 2, 3, 4};
>>  13   char a2[] = {1, 2*argc, 3, 4};
>>  14   int res = memcmp(a1, a2, 4 + argc);  // BOOM
>>  15   // CHECK: AddressSanitizer stack-buffer-overflow
>>  16   // CHECK: {{#0.*memcmp}}
>>  17   // CHECK: {{#1.*main}}
>>  18   return res;
>>  19 }
>>
>> memcmp_test.cc planed for gcc:
>>
>>   1 #include 
>>   2 int main(int argc, char **argv) {
>>   3   char a1[] = {argc, 2, 3, 4};
>>   4   char a2[] = {1, 2*argc, 3, 4};
>>   5   int res = memcmp(a1, a2, 4 + argc);  // BOOM
>>   6   return res;
>>   7 }
>>   8
>>   9 /*  { dg-output "AddressSanitizer stack-buffer-overflow.*" } */
>>  10 /*  { dg-do run { xfail *-*-* } } */
>
> Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?
>
>
> --kcc
>
>>  11
>>
>> Thanks,
>> Wei.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
 wrote:
> On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
>> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
>>> On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
>>> > Diego Novillo  a écrit:
>>> >
>>> >> Patches to libsanitizer should be sent upstream.  We should only
>>> >> contain a copy of the master in the LLVM repository.  There should be
>>> >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
>>> >>  I can't check ATM).
>>> >
>>> > No there are not, for the moment.  README.gcc just says where the
>>> > sources the 'upstream' project is.
>>> >
>>>
>>> What is the plan to add GCC specific support:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304
>>>
>>> and
>>>
>>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>>
>> CCing Wei, I don't know the details about the import.  To me it looks like
>> that most or all of the libsanitizer/ level files (and
>> libsanitizer/*/Makefile.{am,in}) don't originate from
>> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
>> should be changed to support multilibs, use the same libtool/autoconf/etc.
>> versions as rest of gcc etc.
>
>
> Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
> files is a purely GCC thing.
>
>>
>> For changes to the files actually imported from LLVM I guess it depends on
>> if google is going to accept such changes in the LLVM upstream.
>
> Yes, we are willing to support any changes that make libasan support
> more targets.
> We would prefer all patches to go through LLVM first, and then ported
> to GCC by copying files verbatim
> This is the only way we can cope with the two versions.
> (Wei, we will need the exact details for doing this in the README file)
>

Could someone please check this patch:

http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00951.html

into upstream?

Thanks.


-- 
H.J.


Re: [PATCH v3] Add support for sparc compare-and-branch

2012-11-13 Thread Eric Botcazou
> Thanks for finding this, that's definitely incorrect behavior.  I bet there
> is some unintended override triggered by sparc4 selection, and I'll go and
> fix that soon.

You're welcome.  That's the reason why I needed to go the ASM_ARCH way, the 
straightforward approach would have put the -32/-64 first.

And I managed to miss a substitution in the previous patch, so please drop it 
and use the attached one instead.

-- 
Eric BotcazouIndex: config/sparc/sparc.h
===
--- config/sparc/sparc.h	(revision 193416)
+++ config/sparc/sparc.h	(working copy)
@@ -195,7 +195,7 @@ extern enum cmodel sparc_cmodel;
 #endif
 #if TARGET_CPU_DEFAULT == TARGET_CPU_niagara4
 #define CPP_CPU64_DEFAULT_SPEC "-D__sparc_v9__"
-#define ASM_CPU64_DEFAULT_SPEC "-Av9" AS_NIAGARA3_FLAG
+#define ASM_CPU64_DEFAULT_SPEC AS_NIAGARA4_FLAG
 #endif
 
 #else
@@ -337,7 +337,7 @@ extern enum cmodel sparc_cmodel;
 %{mcpu=niagara:%{!mv8plus:-Av9b}} \
 %{mcpu=niagara2:%{!mv8plus:-Av9b}} \
 %{mcpu=niagara3:%{!mv8plus:-Av9" AS_NIAGARA3_FLAG "}} \
-%{mcpu=niagara4:%{!mv8plus:-Av9" AS_NIAGARA3_FLAG "}} \
+%{mcpu=niagara4:%{!mv8plus:" AS_NIAGARA4_FLAG "}} \
 %{!mcpu*:%(asm_cpu_default)} \
 "
 
@@ -1746,6 +1747,12 @@ extern int sparc_indent_opcode;
 #define AS_NIAGARA3_FLAG "b"
 #endif
 
+#ifdef HAVE_AS_SPARC4
+#define AS_NIAGARA4_FLAG "-xarch=sparc4"
+#else
+#define AS_NIAGARA4_FLAG "-Av9" AS_NIAGARA3_FLAG
+#endif
+
 /* We use gcc _mcount for profiling.  */
 #define NO_PROFILE_COUNTERS 0
 
Index: config/sparc/sol2.h
===
--- config/sparc/sol2.h	(revision 193416)
+++ config/sparc/sol2.h	(working copy)
@@ -54,19 +54,56 @@ along with GCC; see the file COPYING3.
 
 /* Supposedly the same as vanilla sparc svr4, except for the stuff below: */
 
-/* This is here rather than in sparc.h because it's not known what
-   other assemblers will accept.  */
+/* If the assembler supports -xarch=sparc4, we switch to the explicit
+   word size selection mechanism available both in GNU as and Sun as,
+   for the Niagara4 and above configurations.  */
+#ifdef HAVE_AS_SPARC4
+
+#define AS_SPARC32_FLAG ""
+#define AS_SPARC64_FLAG ""
 
 #ifndef USE_GAS
-#define AS_SPARC64_FLAG	"-xarch=v9"
-#else
-#define AS_SPARC64_FLAG	"-TSO -64 -Av9"
+#undef ASM_ARCH32_SPEC
+#define ASM_ARCH32_SPEC "-m32"
+#undef ASM_ARCH64_SPEC
+#define ASM_ARCH64_SPEC "-m64"
 #endif
 
+/* Both Sun as and GNU as understand -K PIC.  */
+#undef ASM_SPEC
+#define ASM_SPEC ASM_SPEC_BASE " %(asm_arch)" ASM_PIC_SPEC
+
+#else /* HAVE_AS_SPARC4 */
+
+#define AS_SPARC32_FLAG "-xarch=v8plus"
+#define AS_SPARC64_FLAG "-xarch=v9"
+
+#undef AS_NIAGARA4_FLAG
+#define AS_NIAGARA4_FLAG AS_NIAGARA3_FLAG
+
+#undef ASM_ARCH32_SPEC
+#define ASM_ARCH32_SPEC ""
+
+#undef ASM_ARCH64_SPEC
+#define ASM_ARCH64_SPEC ""
+
+#undef ASM_ARCH_DEFAULT_SPEC
+#define ASM_ARCH_DEFAULT_SPEC ""
+
+#undef ASM_ARCH_SPEC
+#define ASM_ARCH_SPEC ""
+
+/* Both Sun as and GNU as understand -K PIC.  */
+#undef ASM_SPEC
+#define ASM_SPEC ASM_SPEC_BASE ASM_PIC_SPEC
+
+#endif /* HAVE_AS_SPARC4 */
+
+
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC	""
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC	AS_SPARC64_FLAG
+#define ASM_CPU64_DEFAULT_SPEC	"-xarch=v9"
 
 #if TARGET_CPU_DEFAULT == TARGET_CPU_v9
 #undef CPP_CPU64_DEFAULT_SPEC
@@ -83,7 +120,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusa"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "a"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9a"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -94,7 +131,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusb"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "b"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9b"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -105,7 +142,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusb"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "b"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9b"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -116,7 +153,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusb"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "b"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9b"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -127,7 +164,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plus" AS_NIAGARA3_FLAG
 #undef ASM_CPU64_D

[COMMITTED] Move libsanitizer configure logic to subdirectory

2012-11-13 Thread Richard Henderson
As discussed elsewhere.  Tested on x86_64-linux.


r~
---
 ChangeLog   |  5 +
 configure   | 28 +++-
 configure.ac| 25 -
 libsanitizer/ChangeLog.asan |  4 
 libsanitizer/configure.tgt  | 28 
 5 files changed, 72 insertions(+), 18 deletions(-)
 create mode 100644 libsanitizer/configure.tgt

diff --git a/ChangeLog b/ChangeLog
index bb42398..d1302b3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2012-11-13  Richard Henderson  
+
+   * configure.ac: Move libsanitizer logic to subdirectory.
+   * configure: Regenerate.
+
 2012-11-13  Dodji Seketeli  
 
* configure.ac: Enable libsanitizer just on x86 linux for now.
diff --git a/configure b/configure
index 39df09f..45e521e 100755
--- a/configure
+++ b/configure
@@ -3196,6 +3196,25 @@ $as_echo "yes" >&6; }
 fi
 fi
 
+# Disable libsanitizer on unsupported systems.
+if test -d ${srcdir}/libsanitizer; then
+if test x$enable_libsanitizer = x; then
+   { $as_echo "$as_me:${as_lineno-$LINENO}: checking for libsanitizer 
support" >&5
+$as_echo_n "checking for libsanitizer support... " >&6; }
+   if (srcdir=${srcdir}/libsanitizer; \
+   . ${srcdir}/configure.tgt; \
+   test -n "$UNSUPPORTED")
+   then
+   { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+   noconfigdirs="$noconfigdirs target-libsanitizer"
+   else
+   { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+   fi
+fi
+fi
+
 # Disable libquadmath for some systems.
 case "${target}" in
   avr-*-*)
@@ -3208,15 +3227,6 @@ case "${target}" in
 ;;
 esac
 
-# Disable libsanitizer on all systems but x86 linux for now.
-case "${target}" in
-  x86_64-*-linux-* | i?86-*-linux-*)
-;;
-  *)
-noconfigdirs="$noconfigdirs target-libsanitizer"
-;;
-esac
-
 # Disable libssp for some systems.
 case "${target}" in
   avr-*-*)
diff --git a/configure.ac b/configure.ac
index 6c1b008..91ccb3d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -538,6 +538,22 @@ if test -d ${srcdir}/libitm; then
 fi
 fi
 
+# Disable libsanitizer on unsupported systems.
+if test -d ${srcdir}/libsanitizer; then
+if test x$enable_libsanitizer = x; then
+   AC_MSG_CHECKING([for libsanitizer support])
+   if (srcdir=${srcdir}/libsanitizer; \
+   . ${srcdir}/configure.tgt; \
+   test -n "$UNSUPPORTED")
+   then
+   AC_MSG_RESULT([no])
+   noconfigdirs="$noconfigdirs target-libsanitizer"
+   else
+   AC_MSG_RESULT([yes])
+   fi
+fi
+fi
+
 # Disable libquadmath for some systems.
 case "${target}" in
   avr-*-*)
@@ -550,15 +566,6 @@ case "${target}" in
 ;;
 esac
 
-# Disable libsanitizer on all systems but x86 linux for now.
-case "${target}" in
-  x86_64-*-linux-* | i?86-*-linux-*)
-;;
-  *)
-noconfigdirs="$noconfigdirs target-libsanitizer"
-;;
-esac
-
 # Disable libssp for some systems.
 case "${target}" in
   avr-*-*)
diff --git a/libsanitizer/ChangeLog.asan b/libsanitizer/ChangeLog.asan
index 5592092..a12e908 100644
--- a/libsanitizer/ChangeLog.asan
+++ b/libsanitizer/ChangeLog.asan
@@ -1,3 +1,7 @@
+2012-11-13  Richard Henderson  
+
+   * configure.tgt: New file.
+
 2012-11-12  David S. Miller  
 
* asan/asan_linux.cc (GetPcSpBp): Add sparc support.
diff --git a/libsanitizer/configure.tgt b/libsanitizer/configure.tgt
new file mode 100644
index 000..ca7ac1f
--- /dev/null
+++ b/libsanitizer/configure.tgt
@@ -0,0 +1,28 @@
+# -*- shell-script -*-
+#   Copyright (C) 2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not see .
+
+# This is the target specific configuration file.  This is invoked by the
+# autoconf generated configure script.  Putting it in a separate shell file
+# lets us skip running autoconf when modifying target specific information.
+
+# Filter out unsupported systems.
+case "${target}" in
+  x86_64-*-linux* | i?86-*-linux*)
+   ;;
+  *)
+   UNSUPPORTED=1
+   ;;
+esac
-- 
1.7.11.7



Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Wei Mi
> Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?
>
>
> --kcc

Yes, it runs in all modes. O0/O1/O2/O3 + -m32/-m64, which is specified
in asan.exp.

Thanks,
Wei.


Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Andrew Pinski
On Tue, Nov 13, 2012 at 1:40 PM, Konstantin Serebryany
 wrote:
> On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
>> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
>>> On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
>>> > Diego Novillo  a écrit:
>>> >
>>> >> Patches to libsanitizer should be sent upstream.  We should only
>>> >> contain a copy of the master in the LLVM repository.  There should be
>>> >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
>>> >>  I can't check ATM).
>>> >
>>> > No there are not, for the moment.  README.gcc just says where the
>>> > sources the 'upstream' project is.
>>> >
>>>
>>> What is the plan to add GCC specific support:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304
>>>
>>> and
>>>
>>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>>
>> CCing Wei, I don't know the details about the import.  To me it looks like
>> that most or all of the libsanitizer/ level files (and
>> libsanitizer/*/Makefile.{am,in}) don't originate from
>> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
>> should be changed to support multilibs, use the same libtool/autoconf/etc.
>> versions as rest of gcc etc.
>
>
> Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
> files is a purely GCC thing.
>
>>
>> For changes to the files actually imported from LLVM I guess it depends on
>> if google is going to accept such changes in the LLVM upstream.
>
> Yes, we are willing to support any changes that make libasan support
> more targets.
> We would prefer all patches to go through LLVM first, and then ported
> to GCC by copying files verbatim
> This is the only way we can cope with the two versions.
> (Wei, we will need the exact details for doing this in the README file)

I rather have it the other way around; like how libffi is handled.
Since GCC has many more targets and a different schedule than LLVM.

Thanks,
Andrew

>
> --kcc
>
>
>> For
>> unsupported targets we want to add target-libsanitizer into noconfigdirs
>> in toplevel configure.
>>
>> Note that just making libsanitizer to build on some architecture is not
>> enough for full ASAN support, one needs to also add the target hook with
>> mem>>3 to shadow offset, and I guess review all other spots where
>> libsanitizer uses __i386__ or __x86_64__ macros.
>
>
>
>>
>> I'd also say that using sanitizer_atomic_clang.h for GCC is not a good
>> idea, now that GCC 4.7+ has __atomic_* support that should be usable
>> for most of the __sanitizer::atomic* stuff.
>>
>> Jakub


Re: [4/8] Add bit_field_mode_iterator

2012-11-13 Thread Richard Henderson
On 11/13/2012 04:41 AM, Eric Botcazou wrote:
> I find the interface a bit awkward though.  Can't we model it on the existing 
> iterators in basic-block.h or tree-flow.h?  get_best_mode would be written:
> 
>   FOR_EACH_BITFIELD_MODE (mode, iter, bitsize, bitpos,
> bitregion_start, bitregion_end,
> align, volatilep)

Now that we're in C++, I think we should be using iterators that are much
more in the style of libstdc++.  I agree that the .next interface used here
is a bit confusing.

I'd expect to see something like

  for (bit_field_mode_iterator i(x,y,z); i ; ++i)
{
  enum machine_mode mode = *i;
  ...
}

For 4.9, I expect that someone can slowly purge our FOR_EACH_* macros...


r~


Re: [Bug libstdc++/54075] [4.7.1] unordered_map insert still slower than 4.6.2

2012-11-13 Thread François Dumont
Here is the proposal to remove shrinking feature from hash policy. 
I have also considered your remark regarding usage of lower_bound so 
_M_bkt_for_elements doesn't call _M_next_bkt (calling lower_bound) 
anymore. For 2 of the 3 calls it was only a source of redundant 
lower_bound invocations, in the last case I call _M_next_bkt explicitly.


2012-11-13  François Dumont  

* include/bits/hashtable_policy.h (_Prime_rehash_policy): Remove
automatic shrink.
(_Prime_rehash_policy::_M_bkt_for_elements): Do not call
_M_next_bkt anymore.
(_Prime_rehash_policy::_M_next_bkt): Move usage of
_S_growth_factor ...
(_Prime_rehash_policy::_M_need_rehash): ... here.
* include/bits/hashtable.h (_Hashtable<>): Adapt.

Tested under linux x86_64, normal and debug modes.

Regarding performance, I have done a small evolution of the 54075.cc 
test proposed last time. It is now checking performance with and without 
cache of hash code. Result is:


54075.cc std::unordered_set 30 Foo insertions 
without cache   9r9u0s 13765616mem0pf
54075.cc std::unordered_set 30 Foo insertions 
with cache  14r   13u0s 18562064mem0pf
54075.cc std::tr1::unordered_set 30 Foo 
insertions without cache   9r8u1s 13765616mem0pf
54075.cc std::tr1::unordered_set 30 Foo 
insertions with cache  14r   13u0s 18561952mem0pf


So the difference of performance in this case only seems to come 
from caching the hash code or not. In reported use case default behavior 
of std::unordered_set is to cache hash codes and std::tr1::unordered_set 
not to cache it. We should perhaps review default behavior regarding 
caching the hash code. Perhaps cache it if the hash functor can throw 
and not cache it otherwise, not easy to find out what's best to do.


François


On 11/09/2012 11:50 AM, Paolo Carlini wrote:

Hi again,

+// To get previous bound we use _S_growth * 2 to avoid 
ocillations in the

+// number of buckets when looping on insertion/removal of elements.
 const unsigned long* __prev_bkt
-  = std::lower_bound(__prime_list + 1, __next_bkt, __n / 
_S_growth_factor);

+  = std::lower_bound(__prime_list + 1, __next_bkt,
+ __n / _S_growth_factor / _S_growth_factor);

Looks like, here you are dividing by _S_Growth ^ 2? Is it intended? 
But anyway, in my opinion the very my _M_prev_resize idea (thus rehash 
shrinking, right?) is proving quite fragile and we also got negative 
comments from the users about shrinking, which is new. Before pursuing 
it further, I think we should double check what the other 
implementations do, are they also shrinking? Because otherwise we 
could, at least for the time being, remove the related bits and save 
ourselves many headaches...


Paolo.



Index: include/bits/hashtable.h
===
--- include/bits/hashtable.h	(revision 193484)
+++ include/bits/hashtable.h	(working copy)
@@ -806,11 +806,6 @@
   _M_rehash_policy()
 {
   _M_bucket_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
-
-  // We don't want the rehash policy to ask for the hashtable to
-  // shrink on the first insertion so we need to reset its
-  // previous resize level.
-  _M_rehash_policy._M_prev_resize = 0;
   _M_buckets = _M_allocate_buckets(_M_bucket_count);
 }
 
@@ -834,16 +829,12 @@
 	_M_element_count(0),
 	_M_rehash_policy()
   {
+	auto __nb_elems = __detail::__distance_fw(__f, __l);
 	_M_bucket_count =
-	  _M_rehash_policy._M_bkt_for_elements(__detail::__distance_fw(__f,
-   __l));
-	if (_M_bucket_count <= __bucket_hint)
-	  _M_bucket_count = _M_rehash_policy._M_next_bkt(__bucket_hint);
+	  _M_rehash_policy._M_next_bkt(
+	std::max(_M_rehash_policy._M_bkt_for_elements(__nb_elems),
+		 __bucket_hint));
 
-	// We don't want the rehash policy to ask for the hashtable to
-	// shrink on the first insertion so we need to reset its
-	// previous resize level.
-	_M_rehash_policy._M_prev_resize = 0;
 	_M_buckets = _M_allocate_buckets(_M_bucket_count);
 	__try
 	  {
@@ -990,6 +981,7 @@
 __rehash_policy(const _RehashPolicy& __pol)
 {
   size_type __n_bkt = __pol._M_bkt_for_elements(_M_element_count);
+  __n_bkt = __pol._M_next_bkt(__n_bkt);
   if (__n_bkt != _M_bucket_count)
 	_M_rehash(__n_bkt, _M_rehash_policy._M_state());
   _M_rehash_policy = __pol;
@@ -1641,19 +1633,12 @@
 {
   const __rehash_state& __saved_state = _M_rehash_policy._M_state();
   std::size_t __buckets
-	= _M_rehash_policy._M_bkt_for_elements(_M_element_count + 1);
-  if (__buckets <= __n)
-	__buckets = _M_rehash_policy._M_next_bkt(__n);
+	= std::max(_M_rehash_policy._M_bkt_for_elements(_M_element_count + 1),
+		   __n);
+  __buckets = _M_rehash_policy._M_next_bkt(__buckets);
 
   if (__buckets != _M_bucket_count)
-	{

Re: PATCH: PR other/55292: libsanitizer doesn't support x32

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 3:42 AM, Jakub Jelinek  wrote:
> On Tue, Nov 13, 2012 at 03:31:21AM -0800, H.J. Lu wrote:
>> On Tue, Nov 13, 2012 at 3:20 AM, Dodji Seketeli  wrote:
>> > Diego Novillo  a écrit:
>> >
>> >> Patches to libsanitizer should be sent upstream.  We should only
>> >> contain a copy of the master in the LLVM repository.  There should be
>> >> instructions in libsanitizer/README.gcc (Jakub, Dodji, are they there?
>> >>  I can't check ATM).
>> >
>> > No there are not, for the moment.  README.gcc just says where the
>> > sources the 'upstream' project is.
>> >
>>
>> What is the plan to add GCC specific support:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55291
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55292
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55304
>>
>> and
>>
>> http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00967.html
>
> CCing Wei, I don't know the details about the import.  To me it looks like
> that most or all of the libsanitizer/ level files (and
> libsanitizer/*/Makefile.{am,in}) don't originate from
> llvm/projects/compiler-rt/lib , so they should be owned by GCC and thus
> should be changed to support multilibs, use the same libtool/autoconf/etc.
> versions as rest of gcc etc.


Correct. Whatever happens to Makefile, configure and other non-.{cc,h}
files is a purely GCC thing.

>
> For changes to the files actually imported from LLVM I guess it depends on
> if google is going to accept such changes in the LLVM upstream.

Yes, we are willing to support any changes that make libasan support
more targets.
We would prefer all patches to go through LLVM first, and then ported
to GCC by copying files verbatim
This is the only way we can cope with the two versions.
(Wei, we will need the exact details for doing this in the README file)

--kcc


> For
> unsupported targets we want to add target-libsanitizer into noconfigdirs
> in toplevel configure.
>
> Note that just making libsanitizer to build on some architecture is not
> enough for full ASAN support, one needs to also add the target hook with
> mem>>3 to shadow offset, and I guess review all other spots where
> libsanitizer uses __i386__ or __x86_64__ macros.



>
> I'd also say that using sanitizer_atomic_clang.h for GCC is not a good
> idea, now that GCC 4.7+ has __atomic_* support that should be usable
> for most of the __sanitizer::atomic* stuff.
>
> Jakub


libbacktrace patch committed: Only add -Werror for target library

2012-11-13 Thread Ian Lance Taylor
PR 55312 points out that libbacktrace is adding -Werror incorrectly.  We
should not be adding it when built as a host library, because we don't
know the characteristics of the host compiler.  We should only add it
when built as a target library.  This patch implements that.
Bootstrapped and ran libbacktrace testsuite on
x86_64-unknown-linux-gnu.  Committed to mainline.

Ian


2012-11-13  Ian Lance Taylor  

PR other/55312
* configure.ac: Only add -Werror if building a target library.
* configure: Rebuild.


Index: configure.ac
===
--- configure.ac	(revision 193484)
+++ configure.ac	(working copy)
@@ -120,7 +120,7 @@ ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wwri
 			  -Wmissing-format-attribute -Wcast-qual],
 			  [WARN_FLAGS])
 
-if test "x$GCC" = "xyes"; then
+if test -n "${with_target_subdir}"; then
   WARN_FLAGS="$WARN_FLAGS -Werror"
 fi
 


Re: ASAN merge...

2012-11-13 Thread Konstantin Serebryany
On Tue, Nov 13, 2012 at 8:21 AM, Diego Novillo  wrote:
> On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:
>>
>> This has broken the build on every Linux target that hasn't added
>> the necessary cpu specific code to asan_linux.cc
>
> This should be fixed by Dodji's recent patch.  ASAN is not currently
> ported to any target other than x86/linux, so it should just be

asan run-time (and the LLVM part) works on Mac and on ARM/Linux.
But I'd suggest not to worry about any platforms other than
x86_{32,64}/Linux until the whole thing
(including tests) is fully integrated.

--kcc

> completely disabled until the other ports start showing up.
>
> Dodji is your patch committed?
>
>
> Diego.


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Konstantin Serebryany
> I migrate a test in the third category. Please help to check whether
> it is ok. Then I will migrate the left. The files added are as follows
> and attached. (Please forgive I use -fasan in asan.exp because I use
> an old repository to try the migration)
>
> gcc/testsuite/lib/asan-dg.exp:
> copy from libmudflap/lib/mfdg.exp, it rewrites the proc
> dg-test, to enable dg-output check in xfail status.  Existing
> dg-output check in /usr/share/dejagnu/dg.exp only work in pass status
> but not in xfail status.
> gcc/testsuite/g++.dg/asan/asan.exp
> gcc/testsuite/g++.dg/asan/memcmp_test.cc
>
> A problem: llvm provides llvm-symbolizer and asan_symbolize.py to map
> virtual address to its parent func name, which is used in "CHECK:
> {{#0.*memcmp}}" in llvm test below.  I don't know whether gcc provides
> similar tools. How to deal with it?

You can not use llvm-symbolizer, but maybe you can use asan_symbolize.py?
asan_symbolize.py is basically a wrapper for addr2line, a libbfd-based tool.
Or is python not allowed in gcc testing infrastructure?
Then you can probably write a simple script in perl that does the same.

>
> memcmp_test.cc in llvm:
>
>   1 // RUN: %clangxx_asan -m64 -O0 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   2 // RUN: %clangxx_asan -m64 -O1 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   3 // RUN: %clangxx_asan -m64 -O2 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   4 // RUN: %clangxx_asan -m64 -O3 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   5 // RUN: %clangxx_asan -m32 -O0 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   6 // RUN: %clangxx_asan -m32 -O1 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   7 // RUN: %clangxx_asan -m32 -O2 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   8 // RUN: %clangxx_asan -m32 -O3 %s -o %t %lib && %t 2>&1 |
> %symbolize | FileCheck %s
>   9
>  10 #include 
>  11 int main(int argc, char **argv) {
>  12   char a1[] = {argc, 2, 3, 4};
>  13   char a2[] = {1, 2*argc, 3, 4};
>  14   int res = memcmp(a1, a2, 4 + argc);  // BOOM
>  15   // CHECK: AddressSanitizer stack-buffer-overflow
>  16   // CHECK: {{#0.*memcmp}}
>  17   // CHECK: {{#1.*main}}
>  18   return res;
>  19 }
>
> memcmp_test.cc planed for gcc:
>
>   1 #include 
>   2 int main(int argc, char **argv) {
>   3   char a1[] = {argc, 2, 3, 4};
>   4   char a2[] = {1, 2*argc, 3, 4};
>   5   int res = memcmp(a1, a2, 4 + argc);  // BOOM
>   6   return res;
>   7 }
>   8
>   9 /*  { dg-output "AddressSanitizer stack-buffer-overflow.*" } */
>  10 /*  { dg-do run { xfail *-*-* } } */

Will this run the test in all mode (O0 vs O2 and -m32 vs -m64)?


--kcc

>  11
>
> Thanks,
> Wei.


Re: [PATCH, PR 55253] Propagate aggs_contain_variable flag in aggregater IPA-CP

2012-11-13 Thread Ian Lance Taylor
On Tue, Nov 13, 2012 at 9:03 AM, Martin Jambor  wrote:

> Index: src/gcc/ipa-cp.c
> ===
> --- src.orig/gcc/ipa-cp.c
> +++ src/gcc/ipa-cp.c
> @@ -1276,6 +1276,8 @@ merge_aggregate_lattices (struct cgraph_
>  return true;
>if (src_plats->aggs_bottom)
>  return set_agg_lats_contain_variable (dest_plats);
> +  if (src_plats->aggs_contain_variable)
> +ret |= set_agg_lats_contain_variable (dest_plats);
>dst_aglat = &dest_plats->aggs;

Out of curiousity, why are the lines just above the ones you added not

if (src_plats->aggs_bottom)
  return set_agg_lats_to_bottom (dest_plats);

?

Ian


Re: [PATCH, generic] New RTL primitive: `define_subst'

2012-11-13 Thread Richard Henderson
> +  for (elem = other_queue; elem ; elem = elem->next)
> +  {

Note that your indentation is off in places.

> +  /* We are parsing DEFINE_SUBST_ATTR, which could cause generation
> +  of DEFINE_ATTR for introduced DEFINE_SUBST.  It doesn't happen
> +  if such DEFINE_ATTR has already been introduced.
> +  If this did happen, we need to return TRUE to process newly
> +  introduced DEFINE_ATTR.  */

This comment is less than clear.  Unclear enough that I don't even
know how to suggest re-wording it.  We're returning true so that the
new define_subst_attr is processed, or that some define_subst is processed?

> +   while ((start = strchr (end, '<')) && (end  = strchr (end, '>')))
> + {
> +   if (start && end
> +   && (end - start - 1 > 0)
> +   && (end - start - 1 < (int)sizeof (tmpstr)))

That doesn't look right.
(1) end search should be from start, not from the previous end.
(2) you know start and end are non-null inside the loop; why are
you checking for it again.

The bulk of the patch is looking pretty decent now.  We may find problems
with it when we start to use it, but at the moment it's fairly hard to
evaluate completely.

---

Looking at all this, I'm wondering if we shouldn't split out all of this
macro/include processing to a separate pass.  Perform the preprocessing
once, early, leaving the processed result in the build directory.  Then
run the original/traditional rtl reader on that when running the other
rtl manipulation passes.

The reason being that it's going to become increasingly hard to figure
out if the reason for an error is in the macro processing or in the source
md file.  Being able to see the macro expansion is going to be important.

Of course, another way to get this macro expansion is to leave the macro
processing where it is and have another gen program that merely dumps the
processed rtl.  It wouldn't be run normally, but a makefile target used
for debugging would be sufficient.

---

Which brings us to the question of what to do with the patch for 4.8.
It's true that you made the deadline for stage1 closure.  But there will
be no users of this feature, so it begs the question of why we should
apply it now.  Have you a convincing reason?

RM's do you have an opinion here?


r~


Re: Patch RFA: Fix -fpie -fpic

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 12:45 PM, Ian Lance Taylor  wrote:
> Right now if you run "gcc -fpie -fpic" you get, in effect, "gcc -fpie".
> I think you should get "gcc -fpic".  In general I think that of the
> options -fpic, -fPIC, -fpie, -fPIE, -fno-pic, -fno-PIC, -fno-pie,
> -fno-PIE the compiler should act as though only the last of those
> options were specified.  That follows the usual formulat in which the
> last option wins.
>
> This patch implements that.  Bootstrapped and tested on
> x86_64-unknown-linux-gnu.  OK for mainline?
>
> Ian
>
>
> 2012-11-13  Ian Lance Taylor  
>
> * common.opt (fPIC, fPIE, fpic, fpie): Create a Negative loop such
> that any of these options disables the others.
>
>

Although I can't approve it, this is how it works when I introduced
the Negative feature.

-- 
H.J.


Patch RFA: Fix -fpie -fpic

2012-11-13 Thread Ian Lance Taylor
Right now if you run "gcc -fpie -fpic" you get, in effect, "gcc -fpie".
I think you should get "gcc -fpic".  In general I think that of the
options -fpic, -fPIC, -fpie, -fPIE, -fno-pic, -fno-PIC, -fno-pie,
-fno-PIE the compiler should act as though only the last of those
options were specified.  That follows the usual formulat in which the
last option wins.

This patch implements that.  Bootstrapped and tested on
x86_64-unknown-linux-gnu.  OK for mainline?

Ian


2012-11-13  Ian Lance Taylor  

* common.opt (fPIC, fPIE, fpic, fpie): Create a Negative loop such
that any of these options disables the others.


Index: common.opt
===
--- common.opt	(revision 193484)
+++ common.opt	(working copy)
@@ -1583,19 +1583,19 @@ Common Report Var(flag_peephole2) Optimi
 Enable an RTL peephole pass before sched2
 
 fPIC
-Common Report Var(flag_pic,2)
+Common Report Var(flag_pic,2) Negative(fPIE)
 Generate position-independent code if possible (large mode)
 
 fPIE
-Common Report Var(flag_pie,2)
+Common Report Var(flag_pie,2) Negative(fpic)
 Generate position-independent code for executables if possible (large mode)
 
 fpic
-Common Report Var(flag_pic,1)
+Common Report Var(flag_pic,1) Negative(fpie)
 Generate position-independent code if possible (small mode)
 
 fpie
-Common Report Var(flag_pie,1)
+Common Report Var(flag_pie,1) Negative(fPIC)
 Generate position-independent code for executables if possible (small mode)
 
 fplugin=


Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Richard Henderson
On 11/13/2012 05:24 AM, Jakub Jelinek wrote:
> Yes.  And it shouldn't be just based on target CPU, but also based
> on target OS, I don't think libsanitizer supports anything but linux (glibc
> + maybe android) right now, with some smaller or bigger tweaks it could
> support darwin (but see the reports that it doesn't build there right now)
> or mingw/cygwin? (but there is a PR that it doesn't build there).
> So IMHO it should be a whitelist of supported targets with *) case
> adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than
> blacklist of few unsupported ones.  Can you please prepare a patch?

See how libatomic and libitm are structured.

The logic for what targets are supported belongs
inside the library directory, and not at top-level.

Add a configure.tgt script with that knowledge.


r~


Re: [PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Andreas Schwab
Dodji Seketeli  writes:

> I guess The build can be enabled on other targets when they are ready.

How to test that?  There doesn't seem to be a testsuite included.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."


Re: [PATCH v3] Add support for sparc compare-and-branch

2012-11-13 Thread David Miller
From: Eric Botcazou 
Date: Tue, 13 Nov 2012 20:32:40 +0100

> Working on this, I discovered an oddity in GNU as: -xarch=sparc4 -64 yields a 
> 64-bit object file whereas -64 -xarch=sparc4 yields a 32-bit object file.  My 
> understanding is that Sun as will generate a 64-bit object file in both cases.

Thanks for finding this, that's definitely incorrect behavior.  I bet there
is some unintended override triggered by sparc4 selection, and I'll go and
fix that soon.


Re: [PATCH v3] Add support for sparc compare-and-branch

2012-11-13 Thread Eric Botcazou
> We really need to start using the newer names, as Sun is not going to
> provide single letter indicators for sparc4 or future xarch values.
> 
> In fact, that's exactly what needed to be worked on from the beginning
> for the solaris side of this cbcond patch.  We're talking in circles.

OK, sorry for the fiasco...  I can propose the attached patch (for sparc.h and 
sol2.h only).

Working on this, I discovered an oddity in GNU as: -xarch=sparc4 -64 yields a 
64-bit object file whereas -64 -xarch=sparc4 yields a 32-bit object file.  My 
understanding is that Sun as will generate a 64-bit object file in both cases.


* config/sparc/sparc.h (ASM_CPU64_DEFAULT_SPEC): Adjust for Niagara4.
(ASM_CPU_SPEC): Likewise.
(AS_NIAGARA4_FLAG): Define.
* config/sparc/sol2.h (AS_SPARC32_FLAG): Define.
(AS_SPARC64_FLAG): Drop -TSO and adjust.
(AS_NIAGARA4_FLAG): Redefine if !HAVE_AS_SPARC4.
(ASM_CPU32_DEFAULT_SPEC): Use AS_SPARC32_FLAG for Niagara4 only.
(ASM_CPU64_DEFAULT_SPEC): Likewise with AS_NIAGARA4_FLAG.
(ASM_CPU_SPEC): Adjust similarly.
(ASM_SPEC): Add " %(asm_arch)" if HAVE_AS_SPARC4 and adjust.
(ASM_ARCH32_SPEC): Redefine if USE_GAS.
(ASM_ARCH64_SPEC): Likewise.


-- 
Eric BotcazouIndex: config/sparc/sparc.h
===
--- config/sparc/sparc.h	(revision 193416)
+++ config/sparc/sparc.h	(working copy)
@@ -195,7 +195,7 @@ extern enum cmodel sparc_cmodel;
 #endif
 #if TARGET_CPU_DEFAULT == TARGET_CPU_niagara4
 #define CPP_CPU64_DEFAULT_SPEC "-D__sparc_v9__"
-#define ASM_CPU64_DEFAULT_SPEC "-Av9" AS_NIAGARA3_FLAG
+#define ASM_CPU64_DEFAULT_SPEC AS_NIAGARA4_FLAG
 #endif
 
 #else
@@ -337,7 +337,7 @@ extern enum cmodel sparc_cmodel;
 %{mcpu=niagara:%{!mv8plus:-Av9b}} \
 %{mcpu=niagara2:%{!mv8plus:-Av9b}} \
 %{mcpu=niagara3:%{!mv8plus:-Av9" AS_NIAGARA3_FLAG "}} \
-%{mcpu=niagara4:%{!mv8plus:-Av9" AS_NIAGARA3_FLAG "}} \
+%{mcpu=niagara4:%{!mv8plus:" AS_NIAGARA4_FLAG "}} \
 %{!mcpu*:%(asm_cpu_default)} \
 "
 
@@ -1746,6 +1747,12 @@ extern int sparc_indent_opcode;
 #define AS_NIAGARA3_FLAG "b"
 #endif
 
+#ifdef HAVE_AS_SPARC4
+#define AS_NIAGARA4_FLAG "-xarch=sparc4"
+#else
+#define AS_NIAGARA4_FLAG "-Av9" AS_NIAGARA3_FLAG
+#endif
+
 /* We use gcc _mcount for profiling.  */
 #define NO_PROFILE_COUNTERS 0
 
Index: config/sparc/sol2.h
===
--- config/sparc/sol2.h	(revision 193416)
+++ config/sparc/sol2.h	(working copy)
@@ -54,13 +54,14 @@ along with GCC; see the file COPYING3.
 
 /* Supposedly the same as vanilla sparc svr4, except for the stuff below: */
 
-/* This is here rather than in sparc.h because it's not known what
-   other assemblers will accept.  */
-
-#ifndef USE_GAS
-#define AS_SPARC64_FLAG	"-xarch=v9"
+#ifdef HAVE_AS_SPARC4
+#define AS_SPARC32_FLAG	""
+#define AS_SPARC64_FLAG	""
 #else
-#define AS_SPARC64_FLAG	"-TSO -64 -Av9"
+#define AS_SPARC32_FLAG	"-xarch=v8plus"
+#define AS_SPARC64_FLAG	"-xarch=v9"
+#undef AS_NIAGARA4_FLAG
+#define AS_NIAGARA4_FLAG	AS_NIAGARA3_FLAG
 #endif
 
 #undef ASM_CPU32_DEFAULT_SPEC
@@ -83,7 +84,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusa"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "a"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9a"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -94,7 +95,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusb"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "b"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9b"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -105,7 +106,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusb"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "b"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9b"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -116,7 +117,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plusb"
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG "b"
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9b"
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPEC
 #endif
@@ -127,7 +128,7 @@ along with GCC; see the file COPYING3.
 #undef ASM_CPU32_DEFAULT_SPEC
 #define ASM_CPU32_DEFAULT_SPEC "-xarch=v8plus" AS_NIAGARA3_FLAG
 #undef ASM_CPU64_DEFAULT_SPEC
-#define ASM_CPU64_DEFAULT_SPEC AS_SPARC64_FLAG AS_NIAGARA3_FLAG
+#define ASM_CPU64_DEFAULT_SPEC "-xarch=v9" AS_NIAGARA3_FLAG
 #undef ASM_CPU_DEFAULT_SPEC
 #define ASM_CPU_DEFAULT_SPEC ASM_CPU32_DEFAULT_SPE

Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Wei Mi
>> > 3. Full output tests (a .cc file should be build with asan switch,
>> > executable should be run and the stderr is compared with the expected
>> > output)
>> > Example:
>> > http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/lit_tests/stack-overflow.cc?revision=165391&view=markup
>> > The can be ported to GCC, but the uses of FileCheck
>> > (http://llvm.org/docs/CommandGuide/FileCheck.html) will need to be
>> > replaced with GCC's analog.
>> > We should probably start with these tests.
>>
>> Dejagnu in GCC has
>>
>> ! { dg-do run }
>> ! { dg-options "-fbounds-check" }
>> ! { dg-shouldfail "Duplicate value 2 in ORDER argument to RESHAPE
>> intrinsic" }
>> program main
>>   implicit none
>>   integer(kind=1), dimension(6) :: source1 = (/ 1, 2, 3, 4, 5, 6 /)
>>   integer, dimension(2) :: shape1 = (/ 2, 3/)
>>   integer(kind=1), dimension(2) :: pad1 = (/ 0, 0/)
>>   character(len=200) :: l1, l2
>>   integer :: i1, i2
>>
>>   l1 = "2 2"
>>   read(unit=l1,fmt=*) i1, i2
>>   write (unit=l2,fmt=*) reshape(source1, shape1, pad1, (/i1, i2/)) !
>> Invalid
>> end program main
>> ! { dg-output "Fortran runtime error: Duplicate value 2 in ORDER argument
>> to RESHAPE intrinsic" }
>>
>> style markings, dg-shouldfail says that the program is expected to fail
>> rather than pass (if it aborts), and dg-output (perhaps multiple) can
>> contain regexps to match against stderr + stdout joined.  Haven't looked
>> at the asan tests yet, do you expect just one ASAN abort per test,
>
>
> These tests do just one abort (actually, _exit(1)) per test.
> Let's start with these.
>
> --kcc
>

I migrate a test in the third category. Please help to check whether
it is ok. Then I will migrate the left. The files added are as follows
and attached. (Please forgive I use -fasan in asan.exp because I use
an old repository to try the migration)

gcc/testsuite/lib/asan-dg.exp:
copy from libmudflap/lib/mfdg.exp, it rewrites the proc
dg-test, to enable dg-output check in xfail status.  Existing
dg-output check in /usr/share/dejagnu/dg.exp only work in pass status
but not in xfail status.
gcc/testsuite/g++.dg/asan/asan.exp
gcc/testsuite/g++.dg/asan/memcmp_test.cc

A problem: llvm provides llvm-symbolizer and asan_symbolize.py to map
virtual address to its parent func name, which is used in "CHECK:
{{#0.*memcmp}}" in llvm test below.  I don't know whether gcc provides
similar tools. How to deal with it?

memcmp_test.cc in llvm:

  1 // RUN: %clangxx_asan -m64 -O0 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  2 // RUN: %clangxx_asan -m64 -O1 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  3 // RUN: %clangxx_asan -m64 -O2 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  4 // RUN: %clangxx_asan -m64 -O3 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  5 // RUN: %clangxx_asan -m32 -O0 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  6 // RUN: %clangxx_asan -m32 -O1 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  7 // RUN: %clangxx_asan -m32 -O2 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  8 // RUN: %clangxx_asan -m32 -O3 %s -o %t %lib && %t 2>&1 |
%symbolize | FileCheck %s
  9
 10 #include 
 11 int main(int argc, char **argv) {
 12   char a1[] = {argc, 2, 3, 4};
 13   char a2[] = {1, 2*argc, 3, 4};
 14   int res = memcmp(a1, a2, 4 + argc);  // BOOM
 15   // CHECK: AddressSanitizer stack-buffer-overflow
 16   // CHECK: {{#0.*memcmp}}
 17   // CHECK: {{#1.*main}}
 18   return res;
 19 }

memcmp_test.cc planed for gcc:

  1 #include 
  2 int main(int argc, char **argv) {
  3   char a1[] = {argc, 2, 3, 4};
  4   char a2[] = {1, 2*argc, 3, 4};
  5   int res = memcmp(a1, a2, 4 + argc);  // BOOM
  6   return res;
  7 }
  8
  9 /*  { dg-output "AddressSanitizer stack-buffer-overflow.*" } */
 10 /*  { dg-do run { xfail *-*-* } } */
 11

Thanks,
Wei.


asan-dg.exp
Description: Binary data


asan.exp
Description: Binary data


memcmp_test.cc
Description: Binary data


Re: ASAN merge...

2012-11-13 Thread David Miller
From: Diego Novillo 
Date: Tue, 13 Nov 2012 11:21:59 -0500

> On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:
>>
>> This has broken the build on every Linux target that hasn't added
>> the necessary cpu specific code to asan_linux.cc
> 
> This should be fixed by Dodji's recent patch.  ASAN is not currently
> ported to any target other than x86/linux, so it should just be
> completely disabled until the other ports start showing up.
> 
> Dodji is your patch committed?

So I wasted my time by writing the sparc bits necessary to fix
the build?

Please leave enabled the platforms that do actually build.

Thanks.


Re: Patch: PR target/55142: [4.8 Regression] internal compiler error: in plus_constant, at explow.c:88

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 10:03 AM, Uros Bizjak  wrote:
> On Tue, Nov 13, 2012 at 3:30 PM, H.J. Lu  wrote:
>
>>>  Since x32 runs in 64-bit mode, for address -0x4300(%rax), hardware
>>>  sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
>>>  But x32 wants 32-bit -0x4300, not 64-bit -0x4300.  This patch
>>>  uses 32-bit registers instead of 64-bit registers when displacement
>>>  < -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
>>>  still generate -16(%rsp) instead of -16(%esp).
>>> 
>>>  Tested it on Linux/x32.  OK to install?
>>> >>>
>>> >>> This problem uncovers a bug in the middle-end, so I guess it would be
>>> >>> better to fix it there.
>>> >>>
>>> >>> Uros.
>>> >>
>>> >> The problem is it isn't safe to transform
>>> >>
>>> >> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>>> >>
>>> >> to
>>> >>
>>> >> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>> >>
>>> >> when Y is negative and its absolute value is greater than FOO.  However,
>>> >> we have to do this transformation since other parts of GCC depend on
>>> >> it.  If we revert the fix for
>>> >>
>>> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>>> >>
>>> >> we will get
>>> >>
>>> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (internal compiler error)
>>> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (test for excess errors)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>>> >> -funroll-all-loo
>>> >> ps -finline-functions  (internal compiler error)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>>> >> -funroll-all-loo
>>> >> ps -finline-functions  (test for excess errors)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>>> >> -funroll-loops
>>> >> (internal compiler error)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>>> >> -funroll-loops
>>> >> (test for excess errors)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  
>>> >> (internal compi
>>> >> ler error)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (test 
>>> >> for exces
>>> >> s errors)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (internal compiler error)
>>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (test for excess errors)
>>> >> FAIL: gcc.dg/Warray-bounds.c (internal compiler error)
>>> >> FAIL: gcc.dg/Warray-bounds.c (test for excess errors)
>>> >>
>>> >> since we generate pseudo registers to convert SImode to DImode
>>> >> after reload.  Fixing it requires significant changes.
>>> >>
>>> >> This is only a problem for 64-bit register address since the symbolic
>>> >> address is 32-bit.  Using 32-bit base/index registers will work around
>>> >> this issue.
>>> >
>>> > This address
>>> >
>>> > (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>>> >
>>> > is OK for x32 as long as Y, which is encoded as 32-bit immediate,
>>> > is zero-extend from 32-bit to 64-bit.  SImode address does it.
>>> > My patch optimizes it a little bit by using SImode address only
>>> > for Y < -16*1024*1024.
>>>
>>> I was wondering, why we operate with constant -16*1024*1024? Should we
>>> use 0x7FF instead? Since the MSB is always zero, we are safe.
>>>
>>
>> We can check 0x7FF, i.e., disp < 0.  I use -16*1024*1024, which
>> is also used to check legitimate address displacements for PIC, to
>> reduce code sizes for small negative displacements.  Or we can always
>> encode negative displacements with zero-extension, including -1(%rsp).
>>
>>> Please add a fat ??? comment, why we paper-over this issue and repost
>>> the latest patch. I got lost in all the versions :(
>>>
>>
>> Here is the updated patch.
>>
>> gcc/
>>
>> 2012-11-13  Eric Botcazou  
>> H.J. Lu  
>>
>> PR target/55142
>> * config/i386/i386.c (legitimize_pic_address): Properly handle
>> REG + CONST.
>> (ix86_print_operand_address): For x32, zero-extend negative
>> displacement if it < -16*1024*1024.
>>
>> gcc/testsuite/
>>
>> 2012-11-13  H.J. Lu  
>>
>> PR target/55142
>> * gcc.target/i386/pr55142-1.c: New file.
>> * gcc.target/i386/pr55142-2.c: Likewise.
>
> OK, for mainline SVN (with the reservation that middle-end fix was not
> found to be a viable solution, so target fix is papering-over real
> issue. Let's wait for the next victim... ).

That is true.

> Oh, and please fix a typo of mine in the one line above the patch
> hunk; the modifier for SI addresses should be 'k', not 'l'.

Will do.

> BTW: Do we need this patch also for 4.7? x32 address-mode is long by
> default there, but the problem doesn't trigger.
>

This regression is triggered by revision 188008:

http://gcc.gnu.org/ml/gcc-cvs/2012-05/msg00038.html

aka the un-sign-extension of sizetype constants.  We can
backport it to 4.7 branch if we want to be on the safer side.

Thanks.


-- 
H.J.


Re: Patch: PR target/55142: [4.8 Regression] internal compiler error: in plus_constant, at explow.c:88

2012-11-13 Thread Uros Bizjak
On Tue, Nov 13, 2012 at 3:30 PM, H.J. Lu  wrote:

>>  Since x32 runs in 64-bit mode, for address -0x4300(%rax), hardware
>>  sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
>>  But x32 wants 32-bit -0x4300, not 64-bit -0x4300.  This patch
>>  uses 32-bit registers instead of 64-bit registers when displacement
>>  < -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
>>  still generate -16(%rsp) instead of -16(%esp).
>> 
>>  Tested it on Linux/x32.  OK to install?
>> >>>
>> >>> This problem uncovers a bug in the middle-end, so I guess it would be
>> >>> better to fix it there.
>> >>>
>> >>> Uros.
>> >>
>> >> The problem is it isn't safe to transform
>> >>
>> >> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
>> >>
>> >> to
>> >>
>> >> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>> >>
>> >> when Y is negative and its absolute value is greater than FOO.  However,
>> >> we have to do this transformation since other parts of GCC depend on
>> >> it.  If we revert the fix for
>> >>
>> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
>> >>
>> >> we will get
>> >>
>> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (test for excess errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>> >> -funroll-all-loo
>> >> ps -finline-functions  (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>> >> -funroll-all-loo
>> >> ps -finline-functions  (test for excess errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>> >> -funroll-loops
>> >> (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
>> >> -funroll-loops
>> >> (test for excess errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  
>> >> (internal compi
>> >> ler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (test 
>> >> for exces
>> >> s errors)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (internal compiler error)
>> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (test for excess errors)
>> >> FAIL: gcc.dg/Warray-bounds.c (internal compiler error)
>> >> FAIL: gcc.dg/Warray-bounds.c (test for excess errors)
>> >>
>> >> since we generate pseudo registers to convert SImode to DImode
>> >> after reload.  Fixing it requires significant changes.
>> >>
>> >> This is only a problem for 64-bit register address since the symbolic
>> >> address is 32-bit.  Using 32-bit base/index registers will work around
>> >> this issue.
>> >
>> > This address
>> >
>> > (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
>> >
>> > is OK for x32 as long as Y, which is encoded as 32-bit immediate,
>> > is zero-extend from 32-bit to 64-bit.  SImode address does it.
>> > My patch optimizes it a little bit by using SImode address only
>> > for Y < -16*1024*1024.
>>
>> I was wondering, why we operate with constant -16*1024*1024? Should we
>> use 0x7FF instead? Since the MSB is always zero, we are safe.
>>
>
> We can check 0x7FF, i.e., disp < 0.  I use -16*1024*1024, which
> is also used to check legitimate address displacements for PIC, to
> reduce code sizes for small negative displacements.  Or we can always
> encode negative displacements with zero-extension, including -1(%rsp).
>
>> Please add a fat ??? comment, why we paper-over this issue and repost
>> the latest patch. I got lost in all the versions :(
>>
>
> Here is the updated patch.
>
> gcc/
>
> 2012-11-13  Eric Botcazou  
> H.J. Lu  
>
> PR target/55142
> * config/i386/i386.c (legitimize_pic_address): Properly handle
> REG + CONST.
> (ix86_print_operand_address): For x32, zero-extend negative
> displacement if it < -16*1024*1024.
>
> gcc/testsuite/
>
> 2012-11-13  H.J. Lu  
>
> PR target/55142
> * gcc.target/i386/pr55142-1.c: New file.
> * gcc.target/i386/pr55142-2.c: Likewise.

OK, for mainline SVN (with the reservation that middle-end fix was not
found to be a viable solution, so target fix is papering-over real
issue. Let's wait for the next victim... ).

Oh, and please fix a typo of mine in the one line above the patch
hunk; the modifier for SI addresses should be 'k', not 'l'.

BTW: Do we need this patch also for 4.7? x32 address-mode is long by
default there, but the problem doesn't trigger.

Thanks,
Uros.


Re: [tsan] ThreadSanitizer instrumentation part

2012-11-13 Thread Xinliang David Li
On Tue, Nov 13, 2012 at 9:36 AM, Jakub Jelinek  wrote:
> On Tue, Nov 13, 2012 at 09:25:36AM -0800, Xinliang David Li wrote:
>> > That is complete misunderstanding of what update_address_taken does.
>> > It removes TREE_ADDRESSABLE from addressables that are no longer
>> > addressable, rather than adding TREE_ADDRESSABLE bits.
>>
>> It will do the latter too. See iv-opts.
>
> Where?  I strongly doubt that.

You are right. iv-opts is probably a good example that the update_stmt
can do the right job.

thanks,

David


>
>> >  For the latter
>> > there is mark_addressable function.
>>
>> This is certainly cheaper to use.
>
> Even cheaper is just do nothing, tree-ssa-operands.c during
> update_stmt_operands (which will be called for all newly created stmts,
> upon update_stmt etc.) will take care of calling mark_address_taken.
>
> Jakub


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread H.J. Lu
On Mon, Nov 12, 2012 at 10:37 AM, Konstantin Serebryany
 wrote:
> Hi,
> I don't insist that we use gtest for gcc-asan, I just say that this is
> the simplest approach to get 2.5K test suite into gcc,
> and the only approach my team will be able to maintain.
>
> gtest is not as portable as the rest of gcc, but neither is asan
> run-time library (which is more platform-specific than gtest).
>
> On Mon, Nov 12, 2012 at 10:08 AM, Andrew Pinski  wrote:
>> On Mon, Nov 12, 2012 at 10:05 AM, Jakub Jelinek  wrote:
>>> On Mon, Nov 12, 2012 at 09:32:04AM -0800, Wei Mi wrote:
 Using setjmp/longjmp to do multiple tests in a single testfile, the
 test statements in the front could affect the tests in the back. gtest
 will fork a new process for every test statement. The forked process
 will do only one test and skip all the other test statements. That is
 to say, multiple test statements in the same testfile are guaranteed
 to be independent from each other in gtest. If we use setjmp/longjmp
 pattern to do the test, existing testsuite may need to be rewritten if
 their test statements could affect each other.
>>>
>>> So you can either run the program multiple times from within dejagnu, or
>>> fork inside of the macros.  In any case, adding > 5MB of gtest just for that
>>> single test or two is IMHO really too much, and similarly adding gtest
>>> as another requirement to build gcc.  Does gtest support all the targets
>>> that gcc does btw?
>>
>> Also does gtest support cross testing; that is testing over rsh/ssh
>> and testing via a simulator?
>
> I see no reason why gtest will not work via ssh or in simulator.
>

Does gtest support testing different compiler options? I am
using

make check RUNTESTFLAGS="--target_board='unix{-mx32,-mx32,}'"

on Linux/x86-64 to test ia32, x32 as well as x86-64.

-- 
H.J.


Re: [tsan] ThreadSanitizer instrumentation part

2012-11-13 Thread Jakub Jelinek
On Tue, Nov 13, 2012 at 09:25:36AM -0800, Xinliang David Li wrote:
> > That is complete misunderstanding of what update_address_taken does.
> > It removes TREE_ADDRESSABLE from addressables that are no longer
> > addressable, rather than adding TREE_ADDRESSABLE bits.
> 
> It will do the latter too. See iv-opts.

Where?  I strongly doubt that.

> >  For the latter
> > there is mark_addressable function.
> 
> This is certainly cheaper to use.

Even cheaper is just do nothing, tree-ssa-operands.c during
update_stmt_operands (which will be called for all newly created stmts,
upon update_stmt etc.) will take care of calling mark_address_taken.

Jakub


Re: [tsan] ThreadSanitizer instrumentation part

2012-11-13 Thread Xinliang David Li
On Tue, Nov 13, 2012 at 8:40 AM, Jakub Jelinek  wrote:
> On Mon, Nov 05, 2012 at 04:37:41PM -0800, Wei Mi wrote:
>> Thanks for the comments. I fix most of them except the setting of
>> TODO_ The new patch.txt is attached.
>
> Please update the patch against trunk, it doesn't apply cleanly because
> of the asan commit.  I took the liberty to do at least some formatting
> cleanups and other small tweaks against your patch to tsan.c.
>
>> >> +  TODO_verify_all | TODO_update_ssa
>> >
>> > Ideally you shouldn't need TODO_update_ssa.
>> >
>>
>> I got error when I removed TODO_update_ssa, so I kept it.
>
> If it were just for the instrumentations, then you can easily update
> the vdef/vuse yourself, e.g. when inserting before a memory write,
> you can copy over the gimple_vuse of that write to gimple_vuse of the
> instrumentation call, create a new SSA_NAME for the gimple_vdef and
> and set the write's gimple_vuse to that new SSA_NAME.  For call
> instrumentation it would be a tiny bit harder, but for the instrumentation
> of function entry/exit you'd need to find out the current vop at that point.
> So perhaps we can live with that, at least for now.
>
>> >> +| TODO_update_address_taken /* todo_flags_finish  */
>> >
>> > And why this?
>> >
>>
>> If we generate tsan_read(&a) for a non-address taken static variable
>> a, we need to change a to be address taken, right?
>
> That is complete misunderstanding of what update_address_taken does.
> It removes TREE_ADDRESSABLE from addressables that are no longer
> addressable, rather than adding TREE_ADDRESSABLE bits.

It will do the latter too. See iv-opts.

>  For the latter
> there is mark_addressable function.

This is certainly cheaper to use.

David



>>
>> Wrap builtin_decl_implicit in get_tsan_builtin_decl. If
>> builtin_decl_implicit return invalid decl, output error message and
>> then exit.
>
> I've moved that over just to the gate, eventually there should be a routine
> that will initialize the builtins if they aren't by the FE.
>
>> > Please avoid *'s at the beginning of comment continuation lines.
>> > Use is_ctrl_altering_stmt (stmt) to check whether the call must
>> > be the last stmt in a block or not.
>> > And, don't expect there is a single_succ_edge, there could be
>> > no edge at all (e.g. noreturn call), or there could be multiple
>> > edges.
>> >
>>
>> Fixed. Iterate every successive edge of current bb and insert stmt on
>> each edge.
>
> But wrongly, for one adding the same stmt to multiple basic blocks
> is going to crash terribly, but also you IMHO want to insert just
> on fallthru edge, if the routine throws or longjmps, the result isn't
> written.
>
> --- gcc/tsan.c.jj   2012-11-13 16:46:21.0 +0100
> +++ gcc/tsan.c  2012-11-13 17:22:56.054837754 +0100
> @@ -1,4 +1,4 @@
> -/* GCC instrumentation plugin for ThreadSanitizer.
> +/* GCC instrumentation plugin for ThreadSanitizer.
> Copyright (C) 2011, 2012 Free Software Foundation, Inc.
> Contributed by Dmitry Vyukov 
>
> @@ -44,36 +44,27 @@ along with GCC; see the file COPYING3.
> void __tsan_read/writeX (void *addr);  */
>
>  static tree
> -get_tsan_builtin_decl (enum built_in_function fcode)
> -{
> -  tree decl = builtin_decl_implicit (fcode);
> -  if (decl == NULL_TREE)
> -internal_error ("undefined builtin %s", built_in_names[fcode]);
> -  return decl;
> -}
> -
> -static tree
>  get_memory_access_decl (bool is_write, unsigned size)
>  {
>enum built_in_function fcode;
>
>if (size <= 1)
>  fcode = is_write ? BUILT_IN_TSAN_WRITE_1
> - : BUILT_IN_TSAN_READ_1;
> +: BUILT_IN_TSAN_READ_1;
>else if (size <= 3)
> -fcode = is_write ? BUILT_IN_TSAN_WRITE_2
> - : BUILT_IN_TSAN_READ_2;
> +fcode = is_write ? BUILT_IN_TSAN_WRITE_2
> +: BUILT_IN_TSAN_READ_2;
>else if (size <= 7)
> -fcode = is_write ? BUILT_IN_TSAN_WRITE_4
> - : BUILT_IN_TSAN_READ_4;
> +fcode = is_write ? BUILT_IN_TSAN_WRITE_4
> +: BUILT_IN_TSAN_READ_4;
>else if (size <= 15)
> -fcode = is_write ? BUILT_IN_TSAN_WRITE_8
> - : BUILT_IN_TSAN_READ_8;
> +fcode = is_write ? BUILT_IN_TSAN_WRITE_8
> +: BUILT_IN_TSAN_READ_8;
>else
> -fcode = is_write ? BUILT_IN_TSAN_WRITE_16
> - : BUILT_IN_TSAN_READ_16;
> +fcode = is_write ? BUILT_IN_TSAN_WRITE_16
> +: BUILT_IN_TSAN_READ_16;
>
> -  return get_tsan_builtin_decl (fcode);
> +  return builtin_decl_implicit (fcode);
>  }
>
>  /* Check as to whether EXPR refers to a store to vptr.  */
> @@ -81,14 +72,14 @@ get_memory_access_decl (bool is_write, u
>  static tree
>  is_vptr_store (gimple stmt, tree expr, bool is_write)
>  {
> -  if (is_write == true
> +  if (is_write == true
>&& gimple_assign_single_p (stmt)
>&& TREE_CODE (expr) == COMPONENT_REF)
>  {
>tree field = TREE_OPERAND (expr, 1);
>  

Re: [patch RFA middle-end] Fix PR target/41993

2012-11-13 Thread Uros Bizjak
On Tue, Nov 13, 2012 at 6:09 PM, Jakub Jelinek  wrote:

>>   int copy_start, copy_num;
>>   int j;
>>
>> - if (INSN_P (return_copy))
>> + if (INSN_P (return_copy)
>> + && !DEBUG_INSN_P (return_copy))
>
> Please use if (NONDEBUG_INSN_P (return_copy)) instead.

Bah... I did look at this definition and for some reason unknown to
me, I didn't see the equivalence. Anyway, I have committed the change.

Thanks,
Uros.


Re: [patch RFA middle-end] Fix PR target/41993

2012-11-13 Thread Jakub Jelinek
On Tue, Nov 13, 2012 at 06:07:20PM +0100, Uros Bizjak wrote:
> @@ -242,7 +242,8 @@ create_pre_exit (int n_entities, int *entity_map,
>   int copy_start, copy_num;
>   int j;
>  
> - if (INSN_P (return_copy))
> + if (INSN_P (return_copy)
> + && !DEBUG_INSN_P (return_copy))

Please use if (NONDEBUG_INSN_P (return_copy)) instead.

Jakub


Re: [patch RFA middle-end] Fix PR target/41993

2012-11-13 Thread Uros Bizjak
On Mon, Nov 12, 2012 at 9:40 AM, Eric Botcazou  wrote:
>> It looks to me, that we in fact want:
>>
>> --cut here--
>> Index: mode-switching.c
>> ===
>> --- mode-switching.c(revision 193407)
>> +++ mode-switching.c(working copy)
>> @@ -330,7 +330,7 @@
>>   short_block = 1;
>> break;
>>   }
>> -   if (copy_start >= FIRST_PSEUDO_REGISTER)
>> +   if (!targetm.calls.function_value_regno_p (copy_start))
>>   {
>> last_insn = return_copy;
>> continue;
>> --cut here--
>>
>> If we find an unrelated HARD register, we will fail in the same way as
>> described in the PR. This was found by post-reload vzeroupper
>> insertion pass that tripped on unrelated hard reg assignment. At this
>> point, we are interested only in hard registers that are also used for
>> function return value. Actually, even in pre-reload pass, there are no
>> other assignments to hard registers.
>
> Fine with me if this passes testing on x86-avx and SH4.

Committed to mailine SVN with additional restriction, we only want to
scan instructions that are not debug instructions.

2012-11-13  Uros Bizjak  

PR target/41993
* mode-switching.c (create_pre_exit): Set return_copy to last_insn if
copy_start is not a function return regno. Skip debug instructions
in instruction scan loop.

Bootstrapped and regression tested on SH4 by Kaz and Oleg, on AVX
target by Vladimir and by me on x86_64-pc-linux-gnu {,-m32 -} AVX
target, configured with "--with-arch=corei7-avx --with-cpu=corei7-avx
--enable-languages=all,obj-c++,go" with and without postreload
vzeroupper insertion patch.

Uros.
Index: mode-switching.c
===
--- mode-switching.c(revision 193479)
+++ mode-switching.c(working copy)
@@ -242,7 +242,8 @@ create_pre_exit (int n_entities, int *entity_map,
int copy_start, copy_num;
int j;
 
-   if (INSN_P (return_copy))
+   if (INSN_P (return_copy)
+   && !DEBUG_INSN_P (return_copy))
  {
/* When using SJLJ exceptions, the call to the
   unregister function is inserted between the
@@ -330,7 +331,7 @@ create_pre_exit (int n_entities, int *entity_map,
  short_block = 1;
break;
  }
-   if (copy_start >= FIRST_PSEUDO_REGISTER)
+   if (!targetm.calls.function_value_regno_p (copy_start))
  {
last_insn = return_copy;
continue;


Re: [PATCH, PR 55253] Propagate aggs_contain_variable flag in aggregater IPA-CP

2012-11-13 Thread Jan Hubicka
> Hi,
> 
> somehow the propagation of aggs_contain_variable in
> merge_aggregate_lattices got lost when I was shuffling the code
> around, leading to miscompilations.  This patch puts it back in.
> 
> Bootstrapped and tested on x86_64-linux, OK for trunk?
> 
> Thanks,
> 
> Martin
> 
> 
> 2012-11-13  Martin Jambor  
> 
>   PR tree-optimization/55253
>   * ipa-cp.c (merge_aggregate_lattices): Propagate aggs_contain_variable
>   flag.

OK, thanks!
Honza


Re: Asan/Tsan Unit/Regression testing (was [asan] Emit GIMPLE direclty, small cleanups)

2012-11-13 Thread Mike Stump
On Nov 12, 2012, at 4:56 PM, "Joseph S. Myers"  wrote:

> On Mon, 12 Nov 2012, Andrew Pinski wrote:
> 
>> Also does gtest support cross testing; that is testing over rsh/ssh
>> and testing via a simulator?  We should require that as a requirement
>> also when it comes to testing infrastructures.
> 
> Unfortunately the existing guality tests work by C sources built for the 
> target running a GDB binary on the target via popen, rather than having 
> DejaGnu run the GDB binary on the host talking to the program on the 
> target.  So there's an existing not-cross-friendly piece of testsuite 
> code, although it would certainly be good to rework it to work with a 
> cross-GDB and cross-compiler.

An expert or a motivated individual can swap the code around to run gdb on the 
host; not a big deal to me.  I hate more, dozens of non-portable tests cases 
that need to be tweaked in some obscure fashion.  While it is nice if all 
testing worked in a full Canadian environment, I'm ok if people want to do 
native only, at first and just bail out (skip if target, host and build are not 
all the same) on the hard parts.  We then just welcome patches to extend 
coverage to cross targets, when people have the motivation to do it.


[PATCH, PR 55253] Propagate aggs_contain_variable flag in aggregater IPA-CP

2012-11-13 Thread Martin Jambor
Hi,

somehow the propagation of aggs_contain_variable in
merge_aggregate_lattices got lost when I was shuffling the code
around, leading to miscompilations.  This patch puts it back in.

Bootstrapped and tested on x86_64-linux, OK for trunk?

Thanks,

Martin


2012-11-13  Martin Jambor  

PR tree-optimization/55253
* ipa-cp.c (merge_aggregate_lattices): Propagate aggs_contain_variable
flag.

* testsuite/gcc.dg/torture/pr55253.c: New test.
* testsuite/gcc.dg/torture/pr55305.c: Likewise.

Index: src/gcc/ipa-cp.c
===
--- src.orig/gcc/ipa-cp.c
+++ src/gcc/ipa-cp.c
@@ -1276,6 +1276,8 @@ merge_aggregate_lattices (struct cgraph_
 return true;
   if (src_plats->aggs_bottom)
 return set_agg_lats_contain_variable (dest_plats);
+  if (src_plats->aggs_contain_variable)
+ret |= set_agg_lats_contain_variable (dest_plats);
   dst_aglat = &dest_plats->aggs;
 
   for (struct ipcp_agg_lattice *src_aglat = src_plats->aggs;
Index: src/gcc/testsuite/gcc.dg/torture/pr55253.c
===
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/torture/pr55253.c
@@ -0,0 +1,48 @@
+/* { dg-do run } */
+
+struct
+{
+int mallocFailed;
+}
+*a;
+
+struct StrAccum
+{
+int useMalloc;
+}
+b, c;
+
+static void
+fn1 (struct StrAccum *p1, int p2)
+{
+if (p2 == 0)
+return;
+if (p1->useMalloc)
+a->mallocFailed = 0;
+}
+
+void
+fn2 (struct StrAccum *p1)
+{
+fn1 (p1, 1);
+}
+
+void
+fn3 (struct StrAccum *p1)
+{
+fn1 (p1, 1);
+}
+
+void
+fn4 ()
+{
+c.useMalloc = 1;
+fn1 (&c, 0);
+}
+
+int
+main ()
+{
+fn3 (&b);
+return 0;
+}
Index: src/gcc/testsuite/gcc.dg/torture/pr55305.c
===
--- /dev/null
+++ src/gcc/testsuite/gcc.dg/torture/pr55305.c
@@ -0,0 +1,151 @@
+/* { dg-do run } */
+
+extern void exit (int) __attribute__ ((noreturn));
+extern void abort (void) __attribute__ ((noreturn));
+
+struct t
+{
+  char dummy;
+};
+
+struct m
+{
+  const struct t *t;
+  void (*m)(void);
+};
+
+struct s
+{
+  const struct m *m;
+  void *o;
+};
+
+struct e
+{
+  const struct t *t;
+  void *o;
+};
+
+struct ret
+{
+  struct s s;
+  _Bool b;
+};
+
+const struct t t1 = { 1 };
+const struct t t2 = { 2 };
+const struct t t3 = { 3 };
+const struct t t4 = { 4 };
+const struct t t5 = { 5 };
+
+void
+pass (void)
+{
+  exit (0);
+}
+
+void
+fail (void)
+{
+  abort ();
+}
+
+const struct m m1 = { &t4, fail };
+const struct m m2 = { &t5, pass };
+
+static struct e f2 (struct s s2, void *p);
+static struct e f3 (struct s, void *) __attribute__ ((noinline));
+static void f4 (struct s, void *) __attribute__ ((noinline));
+
+struct ret c (struct s, const struct t *) __attribute__ ((noinline));
+
+struct ret
+c (struct s s1, const struct t *t)
+{
+  struct ret r;
+
+  if (s1.m->t == t)
+{
+  r.s.m = &m2;
+  r.s.o = s1.o;
+  r.b = 1;
+}
+  else
+{
+  r.s.m = 0;
+  r.s.o = 0;
+  r.b = 0;
+}
+  return r;
+}
+
+void *m (void) __attribute__ ((noinline));
+
+void *
+m (void)
+{
+  return 0;
+}
+
+struct e
+f1 (struct s s1, void *p)
+{
+  struct ret r;
+  void *a;
+  struct s a2;
+
+  r = c (s1, &t5);
+  if (r.b)
+return f2 (r.s, p);
+  a = m ();
+  a2.m = &m1;
+  a2.o = a;
+  return f2 (a2, p);
+}
+
+static struct e
+f2 (struct s s2, void *p)
+{
+  struct e e1;
+
+  e1 = f3 (s2, p);
+  if (e1.t == &t2 && e1.o == 0)
+{
+  e1.t = 0;
+  e1.o = 0;
+}
+  return e1;
+}
+
+static struct e
+f3 (struct s s1, void *p)
+{
+  struct e r;
+
+  f4 (s1, p);
+  r.t = &t3;
+  r.o = 0;
+  return r;
+}
+
+struct s g1;
+void *g2;
+
+static void
+f4 (struct s s1, void *p)
+{
+  g1 = s1;
+  g2 = p;
+  s1.m->m ();
+}
+
+int
+main ()
+{
+  struct s s1;
+
+  s1.m = &m2;
+  s1.o = 0;
+  f1 (s1, 0);
+  abort ();
+}


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 8:56 AM, Jakub Jelinek  wrote:
> On Tue, Nov 13, 2012 at 08:46:55AM -0800, H.J. Lu wrote:
>> On Tue, Nov 13, 2012 at 08:34:56AM -0800, H.J. Lu wrote:
>> > This patch adds support for GCC multilib run-time libraries to
>> > libsanitizer. I have to delete install-sh.  Otherwise ac_aux_dir will
>> > be wrong:
>> >
>> > ac_aux_dir=
>> > for ac_dir in "$srcdir" "$srcdir/.." "$srcdir/../.."; do
>> >   for ac_t in install-sh install.sh shtool; do
>> > if test -f "$ac_dir/$ac_t"; then
>> >   ac_aux_dir=$ac_dir
>> >   ac_install_sh="$ac_aux_dir/$ac_t -c"
>> >   break 2
>> > fi
>> >
>> > Tested with GCC autoconf/automake on Linux/x86-64.  OK to install?
>> >
>>
>> Here is a smaller patch without removing install-sh.  Instead, it
>> sets AC_CONFIG_AUX_DIR to "..".  OK to install?
>
> I believe libsanitizer/install-sh wasn't imported from LLVM, and
> if it is not needed in the GCC tree, it IMHO should be just removed.
> Similarly libsanitizer/missing, maybe libsanitizer/depcomp (the former
> one isn't present in any other lib*/ in gcc, the latter only in boehm-gc,
> so I'd guess non-essential).  Ditto ltmain.sh, config.sub, config.guess.

That is true.  Those files aren't used in GCC tree.

> libsanitizer/ChangeLog.asan should probably be just libsanitizer/ChangeLog.
>
> In any case, I'd prefer if configury maintainers could review that.

Sure.

Thanks.

-- 
H.J.


Re: [PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Joseph S. Myers
On Tue, 13 Nov 2012, Dodji Seketeli wrote:

>   * configure.ac: Enable libsanitizer just on x86 linux for now.
>   * configure: Re-generate.

Note that the preferred approach is the approach used for libatomic and 
libitm, of the toplevel script using a script fragment from the 
subdirectory to determine what architectures are supported - ideally a 
script fragment designed to be shared by toplevel and by the 
subdirectory's own configure script, if the subdirectory has existing case 
statements over targets.  (It would be good to convert more existing 
projects that share the common toplevel to that scheme as well.)

That way, you don't have a toplevel change that needs merging to the src 
repository for every individual change to the systems supported in a 
particular subdirectory.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread Jakub Jelinek
On Tue, Nov 13, 2012 at 08:46:55AM -0800, H.J. Lu wrote:
> On Tue, Nov 13, 2012 at 08:34:56AM -0800, H.J. Lu wrote:
> > This patch adds support for GCC multilib run-time libraries to
> > libsanitizer. I have to delete install-sh.  Otherwise ac_aux_dir will
> > be wrong:
> > 
> > ac_aux_dir=
> > for ac_dir in "$srcdir" "$srcdir/.." "$srcdir/../.."; do
> >   for ac_t in install-sh install.sh shtool; do
> > if test -f "$ac_dir/$ac_t"; then
> >   ac_aux_dir=$ac_dir
> >   ac_install_sh="$ac_aux_dir/$ac_t -c"
> >   break 2
> > fi
> > 
> > Tested with GCC autoconf/automake on Linux/x86-64.  OK to install?
> > 
> 
> Here is a smaller patch without removing install-sh.  Instead, it
> sets AC_CONFIG_AUX_DIR to "..".  OK to install?

I believe libsanitizer/install-sh wasn't imported from LLVM, and
if it is not needed in the GCC tree, it IMHO should be just removed.
Similarly libsanitizer/missing, maybe libsanitizer/depcomp (the former
one isn't present in any other lib*/ in gcc, the latter only in boehm-gc,
so I'd guess non-essential).  Ditto ltmain.sh, config.sub, config.guess.

libsanitizer/ChangeLog.asan should probably be just libsanitizer/ChangeLog.

In any case, I'd prefer if configury maintainers could review that.

Jakub


Re: ASAN merge...

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 8:21 AM, Diego Novillo  wrote:
> On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:
>>
>> This has broken the build on every Linux target that hasn't added
>> the necessary cpu specific code to asan_linux.cc
>
> This should be fixed by Dodji's recent patch.  ASAN is not currently
> ported to any target other than x86/linux, so it should just be
> completely disabled until the other ports start showing up.
>
> Dodji is your patch committed?
>

What should we do with

http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00951.html

Linux/x32 support is only tested with GCC.  Right now bootstrap
is broken on Linux/x32 since libsanitizer won't compile on Linux/x32.

-- 
H.J.


Re: [PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 08:34:56AM -0800, H.J. Lu wrote:
> Hi,
> 
> This patch adds support for GCC multilib run-time libraries to
> libsanitizer. I have to delete install-sh.  Otherwise ac_aux_dir will
> be wrong:
> 
> ac_aux_dir=
> for ac_dir in "$srcdir" "$srcdir/.." "$srcdir/../.."; do
>   for ac_t in install-sh install.sh shtool; do
> if test -f "$ac_dir/$ac_t"; then
>   ac_aux_dir=$ac_dir
>   ac_install_sh="$ac_aux_dir/$ac_t -c"
>   break 2
> fi
> 
> Tested with GCC autoconf/automake on Linux/x86-64.  OK to install?
> 

Here is a smaller patch without removing install-sh.  Instead, it
sets AC_CONFIG_AUX_DIR to "..".  OK to install?

Thanks.


H.J.
---

PR other/55291
* acinclude.m4: New file.
* Makefile.am (ACLOCAL_AMFLAGS): New.
* configure.ac (AC_PREREQ): Set to 2.64.
(AC_CONFIG_AUX_DIR): Set to "..".
(--enable-version-specific-runtime-libs): New option.
(AC_CANONICAL_SYSTEM): New.
(AM_ENABLE_MULTILIB): Moved right after AM_INIT_AUTOMAKE.
(toolexecdir): Support multilib.
(toolexeclibdir): Likewise.
* Makefile.in: Regenerated.
* aclocal.m4: Likewise.
* configure: Likewise.
* asan/Makefile.in: Likewise.
* interception/Makefile.in: Likewise.
* sanitizer_common/Makefile.in: Likewise.
---
 libsanitizer/Makefile.am   |  2 ++
 libsanitizer/acinclude.m4  | 10 
 libsanitizer/configure.ac  | 63 ++
diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index b28eb32..91e3434 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -1,3 +1,5 @@
+ACLOCAL_AMFLAGS = -I .. -I ../config
+
 SUBDIRS = interception sanitizer_common asan 
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
diff --git a/libsanitizer/acinclude.m4 b/libsanitizer/acinclude.m4
new file mode 100644
index 000..7be603e
--- /dev/null
+++ b/libsanitizer/acinclude.m4
@@ -0,0 +1,10 @@
+dnl --
+dnl This whole bit snagged from libgfortran.
+
+sinclude(../libtool.m4)
+dnl The lines below arrange for aclocal not to bring an installed
+dnl libtool.m4 into aclocal.m4, while still arranging for automake to
+dnl add a definition of LIBTOOL to Makefile.in.
+ifelse(,,,[AC_SUBST(LIBTOOL)
+AC_DEFUN([AC_LIBTOOL_DLOPEN])
+])
diff --git a/libsanitizer/configure.ac b/libsanitizer/configure.ac
index 3f186c2..6129536 100644
--- a/libsanitizer/configure.ac
+++ b/libsanitizer/configure.ac
@@ -1,11 +1,61 @@
 #   -*- Autoconf -*-
 # Process this file with autoconf to produce a configure script.
 
-AC_PREREQ([2.68])
+AC_PREREQ([2.64])
 AC_INIT(package-unused, version-unused, libsanitizer)
 AC_CONFIG_SRCDIR([include/sanitizer/common_interface_defs.h])
-AC_CONFIG_AUX_DIR(.)
+AC_CONFIG_AUX_DIR(..)
+
+AC_MSG_CHECKING([for --enable-version-specific-runtime-libs])
+AC_ARG_ENABLE(version-specific-runtime-libs,
+[  --enable-version-specific-runtime-libsSpecify that runtime libraries 
should be installed in a compiler-specific directory ],
+[case "$enableval" in
+ yes) version_specific_libs=yes ;;
+ no)  version_specific_libs=no ;;
+ *)   AC_MSG_ERROR([Unknown argument to enable/disable version-specific 
libs]);;
+ esac],
+[version_specific_libs=no])
+AC_MSG_RESULT($version_specific_libs)
+
+# Do not delete or change the following two lines.  For why, see
+# http://gcc.gnu.org/ml/libstdc++/2003-07/msg00451.html
+AC_CANONICAL_SYSTEM
+
+target_alias=${target_alias-$host_alias}
+AC_SUBST(target_alias)
+
 AM_INIT_AUTOMAKE(foreign)
+AM_ENABLE_MULTILIB(, ..)
+
+# Calculate toolexeclibdir
+# Also toolexecdir, though it's only used in toolexeclibdir
+case ${version_specific_libs} in
+  yes)
+# Need the gcc compiler version to know where to install libraries
+# and header files if --enable-version-specific-runtime-libs option
+# is selected.
+toolexecdir='$(libdir)/gcc/$(target_alias)'
+toolexeclibdir='$(toolexecdir)/$(gcc_version)$(MULTISUBDIR)'
+;;
+  no)
+if test -n "$with_cross_host" &&
+   test x"$with_cross_host" != x"no"; then
+  # Install a library built with a cross compiler in tooldir, not libdir.
+  toolexecdir='$(exec_prefix)/$(target_alias)'
+  toolexeclibdir='$(toolexecdir)/lib'
+else
+  toolexecdir='$(libdir)/gcc-lib/$(target_alias)'
+  toolexeclibdir='$(libdir)'
+fi
+multi_os_directory=`$CC -print-multi-os-directory`
+case $multi_os_directory in
+  .) ;; # Avoid trailing /.
+  *) toolexeclibdir=$toolexeclibdir/$multi_os_directory ;;
+esac
+;;
+esac
+AC_SUBST(toolexecdir)
+AC_SUBST(toolexeclibdir)
 
 # Checks for programs.
 AC_PROG_CC
@@ -18,15 +68,6 @@ AM_PROG_LIBTOOL
 AC_SUBST(enable_shared)
 AC_SUBST(enable_static)
 
-#AM_ENABLE_MULTILIB(, ..)
-target_alias=${target_alias-$host_alias}
-AC_SUBST(target_alias)
-
-toolexecdir='$(libdi

Re: [tsan] ThreadSanitizer instrumentation part

2012-11-13 Thread Jakub Jelinek
On Mon, Nov 05, 2012 at 04:37:41PM -0800, Wei Mi wrote:
> Thanks for the comments. I fix most of them except the setting of
> TODO_ The new patch.txt is attached.

Please update the patch against trunk, it doesn't apply cleanly because
of the asan commit.  I took the liberty to do at least some formatting
cleanups and other small tweaks against your patch to tsan.c.

> >> +  TODO_verify_all | TODO_update_ssa
> >
> > Ideally you shouldn't need TODO_update_ssa.
> >
> 
> I got error when I removed TODO_update_ssa, so I kept it.

If it were just for the instrumentations, then you can easily update
the vdef/vuse yourself, e.g. when inserting before a memory write,
you can copy over the gimple_vuse of that write to gimple_vuse of the
instrumentation call, create a new SSA_NAME for the gimple_vdef and
and set the write's gimple_vuse to that new SSA_NAME.  For call
instrumentation it would be a tiny bit harder, but for the instrumentation
of function entry/exit you'd need to find out the current vop at that point.
So perhaps we can live with that, at least for now.

> >> +| TODO_update_address_taken /* todo_flags_finish  */
> >
> > And why this?
> >
> 
> If we generate tsan_read(&a) for a non-address taken static variable
> a, we need to change a to be address taken, right?

That is complete misunderstanding of what update_address_taken does.
It removes TREE_ADDRESSABLE from addressables that are no longer
addressable, rather than adding TREE_ADDRESSABLE bits.  For the latter
there is mark_addressable function.
> 
> Wrap builtin_decl_implicit in get_tsan_builtin_decl. If
> builtin_decl_implicit return invalid decl, output error message and
> then exit.

I've moved that over just to the gate, eventually there should be a routine
that will initialize the builtins if they aren't by the FE.

> > Please avoid *'s at the beginning of comment continuation lines.
> > Use is_ctrl_altering_stmt (stmt) to check whether the call must
> > be the last stmt in a block or not.
> > And, don't expect there is a single_succ_edge, there could be
> > no edge at all (e.g. noreturn call), or there could be multiple
> > edges.
> >
> 
> Fixed. Iterate every successive edge of current bb and insert stmt on
> each edge.

But wrongly, for one adding the same stmt to multiple basic blocks
is going to crash terribly, but also you IMHO want to insert just
on fallthru edge, if the routine throws or longjmps, the result isn't
written.

--- gcc/tsan.c.jj   2012-11-13 16:46:21.0 +0100
+++ gcc/tsan.c  2012-11-13 17:22:56.054837754 +0100
@@ -1,4 +1,4 @@
-/* GCC instrumentation plugin for ThreadSanitizer. 
+/* GCC instrumentation plugin for ThreadSanitizer.
Copyright (C) 2011, 2012 Free Software Foundation, Inc.
Contributed by Dmitry Vyukov 
 
@@ -44,36 +44,27 @@ along with GCC; see the file COPYING3.
void __tsan_read/writeX (void *addr);  */
 
 static tree
-get_tsan_builtin_decl (enum built_in_function fcode)
-{
-  tree decl = builtin_decl_implicit (fcode);
-  if (decl == NULL_TREE)
-internal_error ("undefined builtin %s", built_in_names[fcode]);
-  return decl;
-}
-
-static tree
 get_memory_access_decl (bool is_write, unsigned size)
 {
   enum built_in_function fcode;
 
   if (size <= 1)
 fcode = is_write ? BUILT_IN_TSAN_WRITE_1
- : BUILT_IN_TSAN_READ_1;
+: BUILT_IN_TSAN_READ_1;
   else if (size <= 3)
-fcode = is_write ? BUILT_IN_TSAN_WRITE_2 
- : BUILT_IN_TSAN_READ_2;
+fcode = is_write ? BUILT_IN_TSAN_WRITE_2
+: BUILT_IN_TSAN_READ_2;
   else if (size <= 7)
-fcode = is_write ? BUILT_IN_TSAN_WRITE_4 
- : BUILT_IN_TSAN_READ_4;
+fcode = is_write ? BUILT_IN_TSAN_WRITE_4
+: BUILT_IN_TSAN_READ_4;
   else if (size <= 15)
-fcode = is_write ? BUILT_IN_TSAN_WRITE_8 
- : BUILT_IN_TSAN_READ_8;
+fcode = is_write ? BUILT_IN_TSAN_WRITE_8
+: BUILT_IN_TSAN_READ_8;
   else
-fcode = is_write ? BUILT_IN_TSAN_WRITE_16 
- : BUILT_IN_TSAN_READ_16;
+fcode = is_write ? BUILT_IN_TSAN_WRITE_16
+: BUILT_IN_TSAN_READ_16;
 
-  return get_tsan_builtin_decl (fcode);
+  return builtin_decl_implicit (fcode);
 }
 
 /* Check as to whether EXPR refers to a store to vptr.  */
@@ -81,14 +72,14 @@ get_memory_access_decl (bool is_write, u
 static tree
 is_vptr_store (gimple stmt, tree expr, bool is_write)
 {
-  if (is_write == true 
+  if (is_write == true
   && gimple_assign_single_p (stmt)
   && TREE_CODE (expr) == COMPONENT_REF)
 {
   tree field = TREE_OPERAND (expr, 1);
   if (TREE_CODE (field) == FIELD_DECL
-  && DECL_VIRTUAL_P (field))
-return gimple_assign_rhs1 (stmt);
+ && DECL_VIRTUAL_P (field))
+   return gimple_assign_rhs1 (stmt);
 }
   return NULL;
 }
@@ -96,7 +87,7 @@ is_vptr_store (gimple stmt, tree expr, b
 /* Checks as to whether EXPR refers to constant va

[PATCH] PR other/55291: Add support for GCC multilib run-time libraries to libsanitizer

2012-11-13 Thread H.J. Lu
Hi,

This patch adds support for GCC multilib run-time libraries to
libsanitizer. I have to delete install-sh.  Otherwise ac_aux_dir will
be wrong:

ac_aux_dir=
for ac_dir in "$srcdir" "$srcdir/.." "$srcdir/../.."; do
  for ac_t in install-sh install.sh shtool; do
if test -f "$ac_dir/$ac_t"; then
  ac_aux_dir=$ac_dir
  ac_install_sh="$ac_aux_dir/$ac_t -c"
  break 2
fi

Tested with GCC autoconf/automake on Linux/x86-64.  OK to install?

Thanks.


H.J.
---
PR other/55291
* acinclude.m4: New file.
* Makefile.am (ACLOCAL_AMFLAGS): New.
* configure.ac (AC_PREREQ): Set to 2.64.
(AC_CONFIG_AUX_DIR): Removed.
(--enable-version-specific-runtime-libs): New option.
(AC_CANONICAL_SYSTEM): New.
(AM_ENABLE_MULTILIB): Moved right after AM_INIT_AUTOMAKE.
(toolexecdir): Support multilib.
(toolexeclibdir): Likewise.
* install-sh: Removed.
* Makefile.in: Regenerated.
* aclocal.m4: Likewise.
* configure: Likewise.
* asan/Makefile.in: Likewise.
* interception/Makefile.in: Likewise.
* sanitizer_common/Makefile.in: Likewise.

diff --git a/libsanitizer/Makefile.am b/libsanitizer/Makefile.am
index b28eb32..91e3434 100644
--- a/libsanitizer/Makefile.am
+++ b/libsanitizer/Makefile.am
@@ -1,3 +1,5 @@
+ACLOCAL_AMFLAGS = -I .. -I ../config
+
 SUBDIRS = interception sanitizer_common asan 
 
 # Work around what appears to be a GNU make bug handling MAKEFLAGS
diff --git a/libsanitizer/acinclude.m4 b/libsanitizer/acinclude.m4
new file mode 100644
index 000..7be603e
--- /dev/null
+++ b/libsanitizer/acinclude.m4
@@ -0,0 +1,10 @@
+dnl --
+dnl This whole bit snagged from libgfortran.
+
+sinclude(../libtool.m4)
+dnl The lines below arrange for aclocal not to bring an installed
+dnl libtool.m4 into aclocal.m4, while still arranging for automake to
+dnl add a definition of LIBTOOL to Makefile.in.
+ifelse(,,,[AC_SUBST(LIBTOOL)
+AC_DEFUN([AC_LIBTOOL_DLOPEN])
+])
diff --git a/libsanitizer/configure.ac b/libsanitizer/configure.ac
index 3f186c2..30cb6c1 100644
--- a/libsanitizer/configure.ac
+++ b/libsanitizer/configure.ac
@@ -1,11 +1,60 @@
 #   -*- Autoconf -*-
 # Process this file with autoconf to produce a configure script.
 
-AC_PREREQ([2.68])
+AC_PREREQ([2.64])
 AC_INIT(package-unused, version-unused, libsanitizer)
 AC_CONFIG_SRCDIR([include/sanitizer/common_interface_defs.h])
-AC_CONFIG_AUX_DIR(.)
+
+AC_MSG_CHECKING([for --enable-version-specific-runtime-libs])
+AC_ARG_ENABLE(version-specific-runtime-libs,
+[  --enable-version-specific-runtime-libsSpecify that runtime libraries 
should be installed in a compiler-specific directory ],
+[case "$enableval" in
+ yes) version_specific_libs=yes ;;
+ no)  version_specific_libs=no ;;
+ *)   AC_MSG_ERROR([Unknown argument to enable/disable version-specific 
libs]);;
+ esac],
+[version_specific_libs=no])
+AC_MSG_RESULT($version_specific_libs)
+
+# Do not delete or change the following two lines.  For why, see
+# http://gcc.gnu.org/ml/libstdc++/2003-07/msg00451.html
+AC_CANONICAL_SYSTEM
+
+target_alias=${target_alias-$host_alias}
+AC_SUBST(target_alias)
+
 AM_INIT_AUTOMAKE(foreign)
+AM_ENABLE_MULTILIB(, ..)
+
+# Calculate toolexeclibdir
+# Also toolexecdir, though it's only used in toolexeclibdir
+case ${version_specific_libs} in
+  yes)
+# Need the gcc compiler version to know where to install libraries
+# and header files if --enable-version-specific-runtime-libs option
+# is selected.
+toolexecdir='$(libdir)/gcc/$(target_alias)'
+toolexeclibdir='$(toolexecdir)/$(gcc_version)$(MULTISUBDIR)'
+;;
+  no)
+if test -n "$with_cross_host" &&
+   test x"$with_cross_host" != x"no"; then
+  # Install a library built with a cross compiler in tooldir, not libdir.
+  toolexecdir='$(exec_prefix)/$(target_alias)'
+  toolexeclibdir='$(toolexecdir)/lib'
+else
+  toolexecdir='$(libdir)/gcc-lib/$(target_alias)'
+  toolexeclibdir='$(libdir)'
+fi
+multi_os_directory=`$CC -print-multi-os-directory`
+case $multi_os_directory in
+  .) ;; # Avoid trailing /.
+  *) toolexeclibdir=$toolexeclibdir/$multi_os_directory ;;
+esac
+;;
+esac
+AC_SUBST(toolexecdir)
+AC_SUBST(toolexeclibdir)
 
 # Checks for programs.
 AC_PROG_CC
@@ -18,15 +67,6 @@ AM_PROG_LIBTOOL
 AC_SUBST(enable_shared)
 AC_SUBST(enable_static)
 
-#AM_ENABLE_MULTILIB(, ..)
-target_alias=${target_alias-$host_alias}
-AC_SUBST(target_alias)
-
-toolexecdir='$(libdir)/gcc-lib/$(target_alias)'
-toolexeclibdir='$(libdir)'
-AC_SUBST(toolexecdir)
-AC_SUBST(toolexeclibdir)
-
 AC_CONFIG_FILES([Makefile])
 
 AC_CONFIG_FILES(AC_FOREACH([DIR], [interception sanitizer_common asan], 
[DIR/Makefile ]),
diff --git a/libsanitizer/install-sh b/libsanitizer/install-sh
deleted file mode 100644
index a9244eb..000
--- a/libsanitizer/insta

Re: ASAN merge...

2012-11-13 Thread Diego Novillo
On Tue, Nov 13, 2012 at 12:07 AM, David Miller  wrote:
>
> This has broken the build on every Linux target that hasn't added
> the necessary cpu specific code to asan_linux.cc

This should be fixed by Dodji's recent patch.  ASAN is not currently
ported to any target other than x86/linux, so it should just be
completely disabled until the other ports start showing up.

Dodji is your patch committed?


Diego.


Re: [PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Paolo Carlini

On 11/13/2012 04:53 PM, Jack Howarth wrote:
I also noticed while trying to regenerate the libsanitizer Makefile.in 
on darwin that libsanitizer/configure.ac has... AC_PREREQ([2.68]) 
instead of the expected... AC_PREREQ([2.64])

This is other/55304, isn't it?

Paolo.


Re: [PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Jack Howarth
On Tue, Nov 13, 2012 at 04:14:10PM +0100, Eric Botcazou wrote:
> > This patch builds libsanitizer only on x86_64 and i?86 linux targets
> > for now.  I guess The build can be enabled on other targets when they
> > are ready.
> 
> Note that, even on these platforms, it cannot be installed:
> 
> libtool: install: error: cannot install `libasan.la' to a directory not ending
> in /usr/gnat/lib
> make[4]: *** [install-toolexeclibLTLIBRARIES] Error 1
> make[4]: Leaving directory
> 
> -- 
> Eric Botcazou

   I also noticed while trying to regenerate the libsanitizer Makefile.in on 
darwin
that libsanitizer/configure.ac has...

AC_PREREQ([2.68])

instead of the expected...

AC_PREREQ([2.64])

Not good.
Jack


Re: [PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Eric Botcazou
> This patch builds libsanitizer only on x86_64 and i?86 linux targets
> for now.  I guess The build can be enabled on other targets when they
> are ready.

Note that, even on these platforms, it cannot be installed:

libtool: install: error: cannot install `libasan.la' to a directory not ending
in /usr/gnat/lib
make[4]: *** [install-toolexeclibLTLIBRARIES] Error 1
make[4]: Leaving directory

-- 
Eric Botcazou


Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Hans-Peter Nilsson
> From: Dodji Seketeli 
> Date: Tue, 13 Nov 2012 16:04:12 +0100

> I guess when the issue of the missing files is resolved, we can enable
> building libsanitizer on Darwin proper.  Here is the patchlet I am
> proposing so far http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00993.html.

That's the direction I suggested, thanks for resolving this.

brgds, H-P


Re: [PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Diego Novillo
On Tue, Nov 13, 2012 at 10:01 AM, Dodji Seketeli  wrote:
> Hello,
>
> This patch builds libsanitizer only on x86_64 and i?86 linux targets
> for now.  I guess The build can be enabled on other targets when they
> are ready.
>
> OK for trunk?
>
> ChangeLog:
>
> * configure.ac: Enable libsanitizer just on x86 linux for now.
> * configure: Re-generate.

OK.  Thanks.


Diego.


Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Dodji Seketeli
domi...@lps.ens.fr (Dominique Dhumieres) writes:

>> Yes.  And it shouldn't be just based on target CPU, but also based
>> on target OS, I don't think libsanitizer supports anything but linux (glibc
>> + maybe android) right now, with some smaller or bigger tweaks it could
>> support darwin (but see the reports that it doesn't build there right now)
>> ...
>
> Importing the missing files allows the bootstrap to complete.
> However I did not manage so far to make it working on darwin10:
> see http://gcc.gnu.org/ml/gcc-bugs/2012-11/msg01145.html .
>
> I don't think disabling libsanitizer is an emergency for darwin.

I guess when the issue of the missing files is resolved, we can enable
building libsanitizer on Darwin proper.  Here is the patchlet I am
proposing so far http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00993.html.

-- 
Dodji


[PATCH] Enable libsanitizer just on x86 linux for now

2012-11-13 Thread Dodji Seketeli
Hello,

This patch builds libsanitizer only on x86_64 and i?86 linux targets
for now.  I guess The build can be enabled on other targets when they
are ready.

OK for trunk?

ChangeLog:

* configure.ac: Enable libsanitizer just on x86 linux for now.
* configure: Re-generate.
---
 configure| 7 +++
 configure.ac | 7 +++
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index c387e92..39df09f 100755
--- a/configure
+++ b/configure
@@ -3208,12 +3208,11 @@ case "${target}" in
 ;;
 esac
 
-# Disable libsanitizer for some systems.
+# Disable libsanitizer on all systems but x86 linux for now.
 case "${target}" in
-  cris-*-* | crisv32-*-* | mmix-*-*)
-noconfigdirs="$noconfigdirs target-libsanitizer"
+  x86_64-*-linux-* | i?86-*-linux-*)
 ;;
-  powerpc-*-aix* | rs6000-*-aix*)
+  *)
 noconfigdirs="$noconfigdirs target-libsanitizer"
 ;;
 esac
diff --git a/configure.ac b/configure.ac
index 1d958b4..6c1b008 100644
--- a/configure.ac
+++ b/configure.ac
@@ -550,12 +550,11 @@ case "${target}" in
 ;;
 esac
 
-# Disable libsanitizer for some systems.
+# Disable libsanitizer on all systems but x86 linux for now.
 case "${target}" in
-  cris-*-* | crisv32-*-* | mmix-*-*)
-noconfigdirs="$noconfigdirs target-libsanitizer"
+  x86_64-*-linux-* | i?86-*-linux-*)
 ;;
-  powerpc-*-aix* | rs6000-*-aix*)
+  *)
 noconfigdirs="$noconfigdirs target-libsanitizer"
 ;;
 esac
-- 
Dodji


Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Jack Howarth
On Tue, Nov 13, 2012 at 03:06:56PM +0100, Dodji Seketeli wrote:
> Jakub Jelinek  writes:
> 
> > On Tue, Nov 13, 2012 at 02:17:55PM +0100, Dodji Seketeli wrote:
> >> What do the maintainers think?
> >
> > Yes.  And it shouldn't be just based on target CPU, but also based
> > on target OS, I don't think libsanitizer supports anything but linux (glibc
> > + maybe android) right now, with some smaller or bigger tweaks it could
> > support darwin (but see the reports that it doesn't build there right now)
> > or mingw/cygwin? (but there is a PR that it doesn't build there).
> > So IMHO it should be a whitelist of supported targets with *) case
> > adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than
> > blacklist of few unsupported ones.  Can you please prepare a patch?
> 
> Sure, will do.
> 
> -- 
>   Dodji

Dodji,
   I don't think darwin is very far from having usable asan support. Basically
we need the following changes...

1) The missing libsanitizer/interception/mach_override subdirectory with the 
files...

LICENSE.TXT Makefile.mk README.txt  mach_override.c mach_override.h

needs to be imported from compiler-rt's git.

2) In libsanitizer/interception, mach_override/mach_override.c needs to be
added to interception_files and in libsanitizer/asan, the resulting object code
in libsanitizer/interception/mach_override/mach_override.o needs to be linked
into libasan.a and libasan.dylib.

3) In gcc/config/darwin.h, we need to add an entry to LINK_COMMAND_SPEC_A for
faddress-sanitizer to automatically pass -framework CoreFoundation -lasan to
the linker.
  Jack


Patch: PR target/55142: [4.8 Regression] internal compiler error: in plus_constant, at explow.c:88

2012-11-13 Thread H.J. Lu
On Tue, Nov 13, 2012 at 08:28:04AM +0100, Uros Bizjak wrote:
> On Tue, Nov 13, 2012 at 8:15 AM, H.J. Lu  wrote:
> 
>  Since x32 runs in 64-bit mode, for address -0x4300(%rax), hardware
>  sign-extends displacement from 32-bits to 64-bits and adds it to %rax.
>  But x32 wants 32-bit -0x4300, not 64-bit -0x4300.  This patch
>  uses 32-bit registers instead of 64-bit registers when displacement
>  < -16*1024*1024.  -16*1024*1024 is used instead of 0 so that we will
>  still generate -16(%rsp) instead of -16(%esp).
> 
>  Tested it on Linux/x32.  OK to install?
> >>>
> >>> This problem uncovers a bug in the middle-end, so I guess it would be
> >>> better to fix it there.
> >>>
> >>> Uros.
> >>
> >> The problem is it isn't safe to transform
> >>
> >> (zero_extend:DI (plus:SI (FOO:SI) (const_int Y)))
> >>
> >> to
> >>
> >> (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
> >>
> >> when Y is negative and its absolute value is greater than FOO.  However,
> >> we have to do this transformation since other parts of GCC depend on
> >> it.  If we revert the fix for
> >>
> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49721
> >>
> >> we will get
> >>
> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (internal compiler error)
> >> FAIL: gcc.c-torture/compile/990523-1.c  -O3 -g  (test for excess errors)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
> >> -funroll-all-loo
> >> ps -finline-functions  (internal compiler error)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
> >> -funroll-all-loo
> >> ps -finline-functions  (test for excess errors)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
> >> -funroll-loops
> >> (internal compiler error)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer 
> >> -funroll-loops
> >> (test for excess errors)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (internal 
> >> compi
> >> ler error)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -fomit-frame-pointer  (test for 
> >> exces
> >> s errors)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (internal compiler error)
> >> FAIL: gcc.c-torture/compile/pr41634.c  -O3 -g  (test for excess errors)
> >> FAIL: gcc.dg/Warray-bounds.c (internal compiler error)
> >> FAIL: gcc.dg/Warray-bounds.c (test for excess errors)
> >>
> >> since we generate pseudo registers to convert SImode to DImode
> >> after reload.  Fixing it requires significant changes.
> >>
> >> This is only a problem for 64-bit register address since the symbolic
> >> address is 32-bit.  Using 32-bit base/index registers will work around
> >> this issue.
> >
> > This address
> >
> > (plus:DI (zero_extend:DI (FOO:SI)) (const_int Y))
> >
> > is OK for x32 as long as Y, which is encoded as 32-bit immediate,
> > is zero-extend from 32-bit to 64-bit.  SImode address does it.
> > My patch optimizes it a little bit by using SImode address only
> > for Y < -16*1024*1024.
> 
> I was wondering, why we operate with constant -16*1024*1024? Should we
> use 0x7FF instead? Since the MSB is always zero, we are safe.
> 

We can check 0x7FF, i.e., disp < 0.  I use -16*1024*1024, which
is also used to check legitimate address displacements for PIC, to
reduce code sizes for small negative displacements.  Or we can always
encode negative displacements with zero-extension, including -1(%rsp).

> Please add a fat ??? comment, why we paper-over this issue and repost
> the latest patch. I got lost in all the versions :(
> 

Here is the updated patch.

Thanks.

H.J.
---
gcc/

2012-11-13  Eric Botcazou  
H.J. Lu  

PR target/55142
* config/i386/i386.c (legitimize_pic_address): Properly handle
REG + CONST.
(ix86_print_operand_address): For x32, zero-extend negative
displacement if it < -16*1024*1024.

gcc/testsuite/

2012-11-13  H.J. Lu  

PR target/55142
* gcc.target/i386/pr55142-1.c: New file.
* gcc.target/i386/pr55142-2.c: Likewise.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 39ed32e..ee75d8e 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12195,7 +12195,6 @@ legitimize_pic_address (rtx orig, rtx reg)
 {
   rtx addr = orig;
   rtx new_rtx = orig;
-  rtx base;
 
 #if TARGET_MACHO
   if (TARGET_MACHO && !TARGET_64BIT)
@@ -12400,20 +12399,33 @@ legitimize_pic_address (rtx orig, rtx reg)
}
  else
{
- base = legitimize_pic_address (XEXP (addr, 0), reg);
- new_rtx  = legitimize_pic_address (XEXP (addr, 1),
-base == reg ? NULL_RTX : reg);
+ rtx base = legitimize_pic_address (op0, reg);
+ enum machine_mode mode = GET_MODE (base);
+ new_rtx
+   = legitimize_pic_address (op1, base == reg ? NULL_RTX : reg);
 
  if (CONST_INT_P (new_rtx))
-   new_rtx = plus_con

Re: [PATCH] Replace const_vector with match_operand in sse.md

2012-11-13 Thread Andrey Turetskiy
> BTW: Probably, pmulhrsw insn patterns can be merged, too, but this can
> be a follow-up patch.

Please, have a look at patch which merge pmulhrsw patterns.

Changelog:

2012-11-13  Andrey Turetskiy  

   * config/i386/sse.md (*_pmulhrsw3): Merge
   *avx2_pmulhrswv16hi3 and *ssse3_pmulhrswv8hi3 into one pattern.

---
Best regards,
Andrey Turetskiy


pmulhrsw_merge.patch
Description: Binary data


[PATCH] Disable libsanitizer on AIX

2012-11-13 Thread David Edelsohn
Libsanitizer is not supported on AIX. Disable in configure.

* configure.ac: Disable libsanitizer on AIX. Merge libquadmath
sections.
* configure: Regenerate.

Index: configure.ac
===
--- configure.ac(revision 193476)
+++ configure.ac(working copy)
@@ -540,6 +540,9 @@

 # Disable libquadmath for some systems.
 case "${target}" in
+  avr-*-*)
+noconfigdirs="$noconfigdirs target-libquadmath"
+;;
   # libquadmath is unused on AIX and libquadmath build process use of
   # LD_LIBRARY_PATH can break AIX bootstrap.
   powerpc-*-aix* | rs6000-*-aix*)
@@ -552,6 +555,9 @@
   cris-*-* | crisv32-*-* | mmix-*-*)
 noconfigdirs="$noconfigdirs target-libsanitizer"
 ;;
+  powerpc-*-aix* | rs6000-*-aix*)
+noconfigdirs="$noconfigdirs target-libsanitizer"
+;;
 esac

 # Disable libssp for some systems.
@@ -571,13 +577,6 @@
 ;;
 esac

-# Disable libquadmath for some systems.
-case "${target}" in
-  avr-*-*)
-noconfigdirs="$noconfigdirs target-libquadmath"
-;;
-esac
-
 # Disable libstdc++-v3 for some systems.
 # Allow user to override this if they pass --enable-libstdc++-v3
 if test "${ENABLE_LIBSTDCXX}" = "default" ; then


Re: Committed: framework bits for disabling libsanitizer. RFC on which targets for which to disable it.

2012-11-13 Thread Dodji Seketeli
Jakub Jelinek  writes:

> On Tue, Nov 13, 2012 at 02:17:55PM +0100, Dodji Seketeli wrote:
>> What do the maintainers think?
>
> Yes.  And it shouldn't be just based on target CPU, but also based
> on target OS, I don't think libsanitizer supports anything but linux (glibc
> + maybe android) right now, with some smaller or bigger tweaks it could
> support darwin (but see the reports that it doesn't build there right now)
> or mingw/cygwin? (but there is a PR that it doesn't build there).
> So IMHO it should be a whitelist of supported targets with *) case
> adding noconfigdirs="$noconfigdirs target-libsanitizer", rather than
> blacklist of few unsupported ones.  Can you please prepare a patch?

Sure, will do.

-- 
Dodji


Re: [PATCH] Include tm_p.h in asan.c

2012-11-13 Thread David Edelsohn
On Tue, Nov 13, 2012 at 2:39 AM, Jakub Jelinek  wrote:
> On Mon, Nov 12, 2012 at 10:13:08PM -0500, David Edelsohn wrote:
>> gcc/asan.c probably should have been split into two files because it
>> works at multiple levels.  But given that it invokes
>> ASM_GENERATE_INTERNAL_LABEL, it needs to include tm_p.h to include
>> -protos.h.
>>
>> Committed as obvious to allow AIX bootstrap to proceed.
>>
>> * asan.c: Include tm_p.h
>
> Thanks, but Makefile.in needs to be adjusted accordingly too.  Committed

Thanks.

I am disappointed that this patch was not better tested and the
dependencies of other targets were not considered more thoroughly
before the merge.

- David


Re: [6/8] Add strict volatile handling to bit_field_mode_iterator

2012-11-13 Thread Eric Botcazou
> This patch makes bit_field_mode_iterator take -fstrict-volatile-bitfields
> into account, in cases where the size of the underlying object is known.
> This is used in the next patch.

Do we really need to add that to the iterator?  The -fstrict-volatile-
bitfields implementation is still controversial so I'm not sure that we want
let it spread.  Can't the client code just skip the problematic modes?

-- 
Eric Botcazou


Re: [5/8] Tweak bitfield alignment handling

2012-11-13 Thread Eric Botcazou
> This patch replaces:
> 
>   /* Stop if the mode is wider than the alignment of the containing
>object.
> 
>It is tempting to omit the following line unless STRICT_ALIGNMENT
>is true.  But that is incorrect, since if the bitfield uses part
>of 3 bytes and we use a 4-byte mode, we could get a spurious segv
>if the extra 4th byte is past the end of memory.
>(Though at least one Unix compiler ignores this problem:
>that on the Sequent 386 machine.  */
>   if (unit > align_)
>   break;
> 
> with two checks: one for whether the final byte of the mode is known
> to be mapped, and one for whether the bitfield is sufficiently aligned.
> Later patches in the series rely on this in order not to pessimise
> memory handling.
> 
> However, as described in the patch, I found that extending this
> behaviour to get_best_mode affected code generation for x86_64-linux-gnu
> and powerpc64-linux-gnu, and led to a failure in bb-slp-5.c on both.
> I therefore chickened out and added the original check back to
> get_best_mode.
> 
> I'm certainly interested in any feedback on the comment, but at the
> same time I'd like this series to be a no-op on targets that keep
> the traditional .md patterns.  Any change to get_best_mode is probably
> best done as a separate patch.

I agree with your conservative approach here.

> gcc/
>   * stor-layout.c (bit_field_mode_iterator::bit_field_mode_iterator):
>   Set up a default value of bitregion_end_.
>   (bit_field_mode_iterator::next_mode): Always apply bitregion_end_
>   check.  Include SLOW_UNALIGNED_ACCESS in the alignment check.
>   (get_best_mode): Ignore modes that are wider than the alignment.

Fine with me, modulo:

> @@ -2754,6 +2753,62 @@ get_best_mode (int bitsize, int bitpos,
>enum machine_mode widest_mode = VOIDmode;
>enum machine_mode mode;
>while (iter.next_mode (&mode)
> +  /* ??? For compatiblity, reject modes that are wider than the
> + alignment.  This has both advantages and disadvantages.

"For compatibility" is a bit vague (compatibility with what?).  I'd write "For 
historical reasons" or something along these lines.

-- 
Eric Botcazou


[PATCH, ARM] Improved core -> NEON extend

2012-11-13 Thread Ulrich Weigand
Hello,

here's another of Andrew's patches to improve NEON usage.  This one was
originally posted here:
http://gcc.gnu.org/ml/gcc-patches/2012-02/msg01213.html

The idea to improve SImode to DImode extends that also move from core
registers to NEON registers.  In this situation, the compiler used to
perform the extension in core registers first and then moves to NEON,
wasting a core register in the process.

The patch changes this to move to NEON first and extend there.

[ This patch requires both the NEON shift and the lower-subreg patches,
both of which are now in mainline, so this patch is ready to merge
as well at this point.  ]

Tested on arm-linux-gnueabi.

OK for mainline?

Bye,
Ulrich


2012-11-13  Andrew Stubbs  
Ulrich Weigand  

gcc/
* config/arm/arm.md (zero_extenddi2): Add extra alternatives
for NEON registers.
Add alternative for one-instruction extend-in-place.
(extenddi2): Likewise.
Add constraints for Thumb-mode memory loads.
Prevent extend splitters doing NEON alternatives.
* config/arm/iterators.md (qhs_extenddi_cstr, qhs_zextenddi_cstr):
Adjust constraints to add new alternatives.
* config/arm/neon.md: Add splitters for zero- and sign-extend.

gcc/testsuite/
* gcc.target/arm/neon-extend-1.c: New file.
* gcc.target/arm/neon-extend-2.c: New file.

=== modified file 'gcc/config/arm/arm.md'
--- gcc/config/arm/arm.md   2012-09-19 12:57:52 +
+++ gcc/config/arm/arm.md   2012-09-19 13:19:31 +
@@ -4567,33 +4567,36 @@
 ;; Zero and sign extension instructions.
 
 (define_insn "zero_extenddi2"
-  [(set (match_operand:DI 0 "s_register_operand" "=r")
+  [(set (match_operand:DI 0 "s_register_operand" "=w,r,?r")
 (zero_extend:DI (match_operand:QHSI 1 ""
"")))]
   "TARGET_32BIT "
   "#"
-  [(set_attr "length" "8")
+  [(set_attr "length" "8,4,8")
(set_attr "ce_count" "2")
(set_attr "predicable" "yes")]
 )
 
 (define_insn "extenddi2"
-  [(set (match_operand:DI 0 "s_register_operand" "=r")
+  [(set (match_operand:DI 0 "s_register_operand" "=w,r,?r,?r")
 (sign_extend:DI (match_operand:QHSI 1 ""
"")))]
   "TARGET_32BIT "
   "#"
-  [(set_attr "length" "8")
+  [(set_attr "length" "8,4,8,8")
(set_attr "ce_count" "2")
(set_attr "shift" "1")
-   (set_attr "predicable" "yes")]
+   (set_attr "predicable" "yes")
+   (set_attr "arch" "*,*,a,t")]
 )
 
 ;; Splits for all extensions to DImode
 (define_split
   [(set (match_operand:DI 0 "s_register_operand" "")
 (zero_extend:DI (match_operand 1 "nonimmediate_operand" "")))]
-  "TARGET_32BIT"
+  "TARGET_32BIT && (!TARGET_NEON
+   || (reload_completed
+   && !(IS_VFP_REGNUM (REGNO (operands[0])"
   [(set (match_dup 0) (match_dup 1))]
 {
   rtx lo_part = gen_lowpart (SImode, operands[0]);
@@ -4619,7 +4622,9 @@
 (define_split
   [(set (match_operand:DI 0 "s_register_operand" "")
 (sign_extend:DI (match_operand 1 "nonimmediate_operand" "")))]
-  "TARGET_32BIT"
+  "TARGET_32BIT && (!TARGET_NEON
+   || (reload_completed
+   && !(IS_VFP_REGNUM (REGNO (operands[0])"
   [(set (match_dup 0) (ashiftrt:SI (match_dup 1) (const_int 31)))]
 {
   rtx lo_part = gen_lowpart (SImode, operands[0]);

=== modified file 'gcc/config/arm/iterators.md'
--- gcc/config/arm/iterators.md 2012-09-19 12:57:52 +
+++ gcc/config/arm/iterators.md 2012-09-19 13:19:31 +
@@ -412,8 +412,8 @@
 (define_mode_attr qhs_extenddi_op [(SI "s_register_operand")
   (HI "nonimmediate_operand")
   (QI "arm_reg_or_extendqisi_mem_op")])
-(define_mode_attr qhs_extenddi_cstr [(SI "r") (HI "rm") (QI "rUq")])
-(define_mode_attr qhs_zextenddi_cstr [(SI "r") (HI "rm") (QI "rm")])
+(define_mode_attr qhs_extenddi_cstr [(SI "r,0,r,r") (HI "r,0,rm,rm") (QI 
"r,0,rUq,rm")])
+(define_mode_attr qhs_zextenddi_cstr [(SI "r,0,r") (HI "r,0,rm") (QI 
"r,0,rm")])
 
 ;; Mode attributes used for fixed-point support.
 (define_mode_attr qaddsub_suf [(V4UQQ "8") (V2UHQ "16") (UQQ "8") (UHQ "16")

=== modified file 'gcc/config/arm/neon.md'
--- gcc/config/arm/neon.md  2012-09-19 12:57:52 +
+++ gcc/config/arm/neon.md  2012-09-19 13:19:31 +
@@ -5878,3 +5878,65 @@
(const_string "neon_fp_vadd_qqq_vabs_qq"))
  (const_string "neon_int_5")))]
 )
+
+;; Copy from core-to-neon regs, then extend, not vice-versa
+
+(define_split
+  [(set (match_operand:DI 0 "s_register_operand" "")
+   (sign_extend:DI (match_operand:SI 1 "s_register_operand" "")))]
+  "TARGET_NEON && reload_completed && IS_VFP_REGNUM (REGNO (operands[0]))"
+  [(set (match_dup 2) (vec_duplicate:V2SI (match_dup 1)))
+   (set (match_dup 0) (ashiftrt:DI (match_dup 0) (const_int 32)))]
+  {
+operands[2] = gen_

  1   2   >