Re: [PATCH] libgcc refactor aarch64 sfp-machine.h
On 03/12/12 16:00, Marcus Shawcroft wrote: PING I would have thought that both this and the other patch (the update from GLibC) comes under the remit of the port maintainer, rather than the libgcc maintainer. So I would work on the assumption that if Ian doesn't say anything in the next 24 hours, this is OK to go in. R. /Marcus On 15 Nov 2012, at 15:58, Marcus Shawcroft marcus.shawcr...@arm.com wrote: This patch reorganizes the AArch64 sfp-machine.h file, splitting out FP_HANDLE_EXCEPTIONS into a new file sfp-exceptions.c following the same idiom used in ia64 and i386. OK ? Cheers /Marcus 2012-11-15 Marcus Shawcroft marcus.shawcr...@arm.com * config/aarch64/sfp-machine.h (FP_RND_MASK): Define. (FP_ROUNDMODE): Use FP_RND_MASK. * config/aarch64/sfp-exceptions.c: New. * config/aarch64/sfp-machine.h (FP_HANDLE_EXCEPTIONS): Use __sfp_handle_exceptions.refactor-sfp-machine.diff
[libquadmath, patch, committed] Fix exponent reading
Somehow that got lot when updating the file. I think, we really need a
proper test suite - the current quad_{1,2,3}.f90 are a first step, but
rather incomplete.
Committed as Rev. 194100 after build+regtesting.
Tobias
Index: libquadmath/ChangeLog
===
--- libquadmath/ChangeLog (Revision 194099)
+++ libquadmath/ChangeLog (Arbeitskopie)
@@ -1,3 +1,8 @@
+2012-12-03 Tobias Burnus bur...@net-b.de
+
+ * strtod/strtod_l.c (___STRTOF_INTERNAL): Fix exponent
+ reading.
+
2012-11-25 Tobias Burnus bur...@net-b.de
PR libquadmath/55462
Index: libquadmath/strtod/strtod_l.c
===
--- libquadmath/strtod/strtod_l.c (Revision 194099)
+++ libquadmath/strtod/strtod_l.c (Arbeitskopie)
@@ -1005,6 +1005,9 @@ STRTOF_INTERNAL (nptr, endptr, group)
/* NOTREACHED */
}
+ exponent *= 10;
+ exponent += c - L_('0');
+
c = *++cp;
}
while (c = L_('0') c = L_('9'));
Index: gcc/testsuite/ChangeLog
===
--- gcc/testsuite/ChangeLog (Revision 194099)
+++ gcc/testsuite/ChangeLog (Arbeitskopie)
@@ -1,3 +1,7 @@
+2012-12-03 Tobias Burnus bur...@net-b.de
+
+ * gfortran.dg/quad_3.f90: New.
+
2012-12-03 Paolo Carlini paolo.carl...@oracle.com
PR c++/54170
Index: gcc/testsuite/gfortran.dg/quad_3.f90
===
--- gcc/testsuite/gfortran.dg/quad_3.f90 (Revision 0)
+++ gcc/testsuite/gfortran.dg/quad_3.f90 (Arbeitskopie)
@@ -0,0 +1,27 @@
+! { dg-do run }
+!
+! I/O test for REAL(16)
+!
+! Contributed by Dominique d'Humieres
+!
+program test_qp
+ use iso_fortran_env, only: real_kinds
+ implicit none
+ integer, parameter :: QP = real_kinds(ubound(real_kinds,dim=1))
+ real(kind=qp) :: a,b(2), c
+ integer :: exponent
+ character(len=180) :: tmp
+
+ ! Run this only with libquadmath; assume that all those systems
+ ! have also kind=10.
+ if (size (real_kinds) = 4 .and. real_kinds(3) == 10 .and. qp == 16) then
+ exponent = 4000
+ b(:) = huge (1.0_qp)/10.0_qp**exponent
+! print *, 'real(16) big value: ', b(1)
+ write (tmp, *) b
+ read (tmp, *) a, c
+! print *, 'same value read again: ', a, c
+! print *, 'difference: looks OK now ', a-b(1)
+ if (a-b(1) /= 0.0_qp .or. c-b(1) /= 0.0_qp) call abort()
+ end if
+end program test_qp
Re: [Patch AArch64] Add support for vectorizable standard math patterns.
On 27/11/12 20:47, Mike Stump wrote: On Nov 27, 2012, at 8:51 AM, James Greenhalgh james.greenha...@arm.com wrote: In particular, we add support for vectorizing across: ceil (), ceilf (), lceil (), We add testcases ensuring that each of the expected functions are vectorized. As the i386 and rs6000 backends both ostensibly support these optimisations we add these tests to the generic testsuites, but only wire them up for AArch64. As a target may support any subset of these vectorizations we need a check_effective_target macro for each of them. Because of this change to the generic test code I've CCed Janis Johnson and Mike Stump. Gosh… leaves a bad taste in my mouth.I see why you did it that way… So, let me just ping folks, anyone see a better way to do this? If no one admits to having a better solution, I'll approve it. Let's give them a few days to come up with something concrete, if they fail, Ok. Mike, Are you happy to go with the proposal from James or would like to give folk more time to think this one over? Cheers /Marcus
Re: [PATCH] Convert asan to use sanitizer.def builtins, initialize them if the FE didn't
Jakub Jelinek ja...@redhat.com writes:
Ok for trunk (the patch is on top of the tsan patch)?
2012-11-22 Jakub Jelinek ja...@redhat.com
* sanitizer.def: Add Address Sanitizer builtins.
Rename BUILT_IN_TSAN_READ_* to BUILT_IN_TSAN_READ* and
BUILT_IN_TSAN_WRITE_* to BUILT_IN_TSAN_WRITE*.
* Makefile.in (asan.o): Depend on langhooks.h.
(tsan.o): Depend on asan.h.
* asan.h (initialize_sanitizer_builtins): New prototype.
* asan.c: Include langhooks.h.
(report_error_func): Use builtin_decl_implicit of corresponding
BUILT_IN_ASAN_REPORT_{LOAD,STORE}*.
(asan_init_func): Removed.
(initialize_sanitizer_builtins): New function.
(asan_finish_file): Call it. Use builtin_decl_implicit
on BUILT_IN_ASAN_{INIT,{,UN}REGISTER_GLOBALS}.
(asan_instrument): Call initialize_sanitizer_builtins.
* builtins.def (DEF_SANITIZER_BUILTIN): Change condition to
(flag_asan || flag_tsan).
* tsan.c: Include asan.h and tsan.h.
(get_memory_access_decl): Rename BUILT_IN_TSAN_{READ,WRITE}_*
to BUILT_IN_TSAN_{READ,WRITE}*.
(tsan_pass): Call initialize_sanitizer_builtins.
(tsan_gate, tsan_gate_O0): Don't check if
builtin_decl_implicit_p (BUILT_IN_TSAN_INIT) is true.
(tsan_finish_file): Call initialize_sanitizer_builtins.
* builtin-types.def (BT_FN_VOID_PTR_PTRMODE): New fn type.
--- gcc/sanitizer.def.jj 2012-11-22 13:17:24.0 +0100
+++ gcc/sanitizer.def 2012-11-22 15:45:55.873655417 +0100
@@ -1,3 +1,34 @@
+/* Address Sanitizer */
Maybe we could use some introductory comment at the beginning of this
file, a bit like what we have for sync-builtins.def. I think this kind
of comments are a great asset for people who are new to the file.
Otherwise, the asan parts of the patch looks OK to me.
Thanks.
--
Dodji
[PATCH, libatomic] detect and build for ARM architecture armv8-a
libatomic does not build for --arch=armv8-a because there is no architecture version detection. This patch allows libatomic to build for armv8-a, but does not exploit any of the new features of v8. /Marcus 2012-12-03 Marcus Shawcroft marcus.shawcr...@arm.com * config/arm/arm-config.h (__ARM_ARCH_8A__): New.diff --git a/libatomic/config/arm/arm-config.h b/libatomic/config/arm/arm-config.h index b94a372..b99c82e 100644 --- a/libatomic/config/arm/arm-config.h +++ b/libatomic/config/arm/arm-config.h @@ -57,6 +57,10 @@ # define __ARM_ARCH__ 7 #endif +#if defined(__ARM_ARCH_8A__) +# define __ARM_ARCH__ 8 +#endif + #ifndef __ARM_ARCH__ #error Unable to determine architecture. #endif -- 1.7.9.5
[PATCH] AArch64 fix ICE due to missing TYPE_STUB_DECL on va_list.
This patch fixes an ICE due to a missing TYPE_STUB_DECL on the builtin
va_list tree node.
/Marcus
gcc/
2012-11-27 Marcus Shawcroft marcus.shawcr...@arm.com
* config/aarch64/aarch64.c (aarch64_build_builtin_va_list): Set
TYPE_STUB_DECL.
testsuite/
2012-11-27 Marcus Shawcroft marcus.shawcr...@arm.com
* gcc.target/aarch64/121127.c: New test.diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index d4708bf..05e1da8 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -5053,6 +5053,7 @@ aarch64_build_builtin_va_list (void)
va_list_type);
DECL_ARTIFICIAL (va_list_name) = 1;
TYPE_NAME (va_list_type) = va_list_name;
+ TYPE_STUB_DECL (va_list_type) = va_list_name;
/* Create the fields. */
f_stack = build_decl (BUILTINS_LOCATION,
diff --git a/gcc/testsuite/gcc.target/aarch64/121127.c b/gcc/testsuite/gcc.target/aarch64/121127.c
new file mode 100644
index 000..a7dca09
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/121127.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-options -g -femit-struct-debug-baseonly } */
+
+typedef __builtin_va_list __gnuc_va_list;
--
1.7.9.5
Re: [PATCH, PR 54394] Compute loops when generating inline summaries
On Fri, Aug 31, 2012 at 7:41 AM, Martin Jambor mjam...@suse.cz wrote: Hi, On Fri, Aug 31, 2012 at 12:06:33PM +0200, Jan Hubicka wrote: This is not required to make hints working, it is necessary because of the following line a in estimate_function_body_sizes: es-loop_depth = bb_loop_depth (bb); which always yields zero if we don't have loops computed. So I can skip the computation only if we don't care about loop depths in early inlining either. Should I skip it? Only place we care is the badness computation and only if profile guessing is off, so just initialize it to 0 for early inliner. Thanks. For the record, this is what I have committed. Martin 2012-08-31 Martin Jambor mjam...@suse.cz PR middle-end/54394 * ipa-inline-analysis.c (estimate_function_body_sizes): Compute dominance info and loops whenever optimizing. This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55564 -- H.J.
Re: [i386] scalar ops that preserve the high part of a vector
On Mon, Dec 3, 2012 at 4:34 PM, Marc Glisse marc.gli...@inria.fr wrote: However, looking a bit more into the usage cases for these patterns, they are only used through intrinsics with _m128 operands. While your proposed patch makes these patterns more general (they can use 64bit aligned memory), this is not their usual usage, and for their intended usage, your proposed improvement complicates these patterns unnecessarily. Following on these facts, I'd say that we leave these special patters (since they serve their purpose well) and rather introduce new patterns for other uses. You mean like in the original patch? http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01279.html (it only had the V2DF version, not the V4SF one) Funny how we switched sides, now I am the one who would rather have a single pattern instead of having one for the builtin and one for recog. It seems that once we add the new pattern, keeping the old one is a waste of maintenance time, and the few extra rtx from the slightly longer pattern for these seldomly used builtins should be negligible. Yes, I didn't notice at the time that the intention of existing patterns was to implement intrinsics that exclusively use _m128 operands. But I don't mind, if that's the version you prefer, I'll update the patch. Actually, both approaches have their benefits and drawbacks. Specialized vec_merge patterns can be efficiently macroized, and support builtins with _m128 operands in a simple and efficient way. You are proposing patterns that do not macroize well (this is what was learned from your last patch) and require breakup of existing macroized patterns. So, we are actually adding new functionality - operations on an array of values. IMO, this warrants new patterns, but please find a way for V2DF and V4SF to macroize in the same way. Uros.
Re: [PATCH] Add --with-build-config=bootstrap-asan support
On Mon, Dec 3, 2012 at 3:52 AM, Jakub Jelinek ja...@redhat.com wrote: Hi! On Thu, Nov 29, 2012 at 12:20:39PM -0800, H.J. Lu wrote: On Thu, Nov 29, 2012 at 11:06 AM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Nov 29, 2012 at 07:24:38PM +0100, Paolo Carlini wrote: On 11/29/2012 06:36 PM, Tobias Burnus wrote: H.J. Lu wrote: This patch adds --with-build-config=bootstrap-asan support. Tested on Linux/x86-64. OK to install? I think that patch has broken bootstrap for me. If I do a normal bootstrap, Stage1 fails with: libtool: compile: unrecognized option `-D_GNU_SOURCE' libtool: compile: Try `libtool --help' for more information. make[4]: *** [interception_linux.lo] Error 1 make[4]: Leaving directory `/home/burnus/gcc/build/x86_64-unknown-linux-gnu/libsanitizer/interception' Likewise here. Would it be possible to revert the offending commit, in the meanwhile? Yes, H.J., please revert the patch, I thought you have tested it alone without any further patches. For the -I patch, I really would prefer if libsanitizer just had a dependency on libstdc++ at toplevel (configure-target-sanitizer depending on all-target-libstdc++-v3), then you can (and similarly for host variants if we need host sanitizer at all). Then you should be able to use scripts/testsuite_flags --build-includes just fine. I opened: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55533 to explain why scripts/testsuite_flags doesn't work when bootstrapping libsanitizer. CCing Paolo and Alex as build maintainers on this, using the script is just my preference and not sure whether it is feasible or not, though IMHO if there is a dependency on libstdc++-v3 being built before libsanitizer is configured (or perhaps just configure when it has been configured and build when it has been built), I don't see why it couldn't be used in theory. I'll defer this to them. Hi Alex, Paolo, This is related to http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01430.html http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46026 The problem is CXX_FOR_TARGET used to configure libsanitizer was expanded before libstdc++-v3 is configured when we are bootstrapping libsanitizer. I opened: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55533 with a fix at http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02480.html Can you take a look at it? Thanks. -- H.J.
Re: Sparc ASAN
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com Date: Tue, 27 Nov 2012 18:12:00 +0400 On Wed, Nov 21, 2012 at 9:05 PM, Konstantin Serebryany konstantin.s.serebry...@gmail.com wrote: On Wed, Nov 21, 2012 at 8:40 PM, David Miller da...@davemloft.net wrote: From: Konstantin Serebryany konstantin.s.serebry...@gmail.com Date: Wed, 21 Nov 2012 19:39:52 +0400 There are various other things that asan library does not support. I'm trying to understand why making the page size variable is such a difficult endeavour. Maybe it's not *that* difficult. Looking at it carefully, the major problem I can see is that some other constants are defined based on this one. Give me a few days to resolve it... http://code.google.com/p/address-sanitizer/issues/detail?id=128 http://gcc.gnu.org/viewcvs?root=gccview=revrev=193849 removes the kPageSize constant in favor of a function call. Please give it a try. BTW, libsanitizer now has a usable process to quickly pull the upstream changes. It should make it easier for us to iterate on platform-specific patches. So, with the hacks for unaligned accesses, Sparc seems to be working here. The only changes to libsantizier is to put __sparc__ checks where __powerpc__ checks exist in the unwind code. Are there any clear thoughts about what we should do in the end wrt. the stack ASAN alignment issues? Do we plan to 32-byte align the stack variables or similar? Otherwise we need to add some ugly code to do half-word/byte at a time accesses, as needed.
Re: [PATCH, PR 54394] Compute loops when generating inline summaries
On Mon, Dec 3, 2012 at 9:50 AM, H.J. Lu hjl.to...@gmail.com wrote: On Fri, Aug 31, 2012 at 7:41 AM, Martin Jambor mjam...@suse.cz wrote: Hi, On Fri, Aug 31, 2012 at 12:06:33PM +0200, Jan Hubicka wrote: This is not required to make hints working, it is necessary because of the following line a in estimate_function_body_sizes: es-loop_depth = bb_loop_depth (bb); which always yields zero if we don't have loops computed. So I can skip the computation only if we don't care about loop depths in early inlining either. Should I skip it? Only place we care is the badness computation and only if profile guessing is off, so just initialize it to 0 for early inliner. Thanks. For the record, this is what I have committed. Martin 2012-08-31 Martin Jambor mjam...@suse.cz PR middle-end/54394 * ipa-inline-analysis.c (estimate_function_body_sizes): Compute dominance info and loops whenever optimizing. This caused: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55564 Oops. Wrong link. Please ignore this. -- H.J.
Re: Sparc ASAN
On Mon, Dec 3, 2012 at 10:02 PM, David Miller da...@davemloft.net wrote:
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com
Date: Tue, 27 Nov 2012 18:12:00 +0400
On Wed, Nov 21, 2012 at 9:05 PM, Konstantin Serebryany
konstantin.s.serebry...@gmail.com wrote:
On Wed, Nov 21, 2012 at 8:40 PM, David Miller da...@davemloft.net wrote:
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com
Date: Wed, 21 Nov 2012 19:39:52 +0400
There are various other things that asan library does not support.
I'm trying to understand why making the page size variable
is such a difficult endeavour.
Maybe it's not *that* difficult.
Looking at it carefully, the major problem I can see is that some
other constants are defined based on this one.
Give me a few days to resolve it...
http://code.google.com/p/address-sanitizer/issues/detail?id=128
http://gcc.gnu.org/viewcvs?root=gccview=revrev=193849 removes the
kPageSize constant in favor of a function call.
Please give it a try.
BTW, libsanitizer now has a usable process to quickly pull the upstream
changes.
It should make it easier for us to iterate on platform-specific patches.
So, with the hacks for unaligned accesses, Sparc seems to be working
here.
The only changes to libsantizier is to put __sparc__ checks where
__powerpc__ checks exist in the unwind code.
Like this?
===
--- asan/asan_linux.cc (revision 169136)
+++ asan/asan_linux.cc (working copy)
@@ -158,7 +158,9 @@
stack-trace[0] = pc;
if ((max_s) 1) {
stack-max_size = max_s;
-#if defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__)
+#if defined(__arm__) || \
+defined(__powerpc__) || defined(__powerpc64__) || \
+defined(__sparc__)
_Unwind_Backtrace(Unwind_Trace, stack);
// Pop off the two ASAN functions from the backtrace.
stack-PopStackFrames(2);
Are there any clear thoughts about what we should do in the end
wrt. the stack ASAN alignment issues? Do we plan to 32-byte
align the stack variables or similar? Otherwise we need to add
some ugly code to do half-word/byte at a time accesses, as needed.
The LLVM implementation always used 32-byte alignment for stack redzones.
I never actually did any performance checking on x86 (32-byte aligned
vs 8-byte aligned),
although I suspect 32-byte aligned redzones should be ~2x faster.
8-byte aligned redzones trade some CPU for some stack memory and
probably slightly increase
the chances of catching large (33+ bytes off) buffer overflows.
For strict alignment architectures 8-byte aligned redzones is
obviously not a choice.
We either need to align the redzones by 32 always, or for some platforms.
Either is fine for me.
--kcc
Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
2. gcse.c: gcse_emit_move_after added notes, but none of them was very useful as far as I could tell, and almost all of them turned self-referencing after CPROP. So I propose we just never add notes in this case. gcse_emit_move_after also preserves existing notes. Are they problematic? Yes, they tend to be invalid after PRE because the registers used in the PRE'd expression usually are not live anymore (making the note invalid). Sometimes CPROP re-validates the notes, but it doesn't seem wise to me to rely on that. So the compiler doesn't bootstrap with the gcse.c patch you posted earlier in the thread? Still this seems too bold to me, the note datum could be a constant and should be preserved in this case. 3. cprop.c: It seems to me that the purpose here is to propagate constants. If a reg could not be propagated, then the REG_EQUAL note will not help much either. Propagating constants via REG_EQUAL notes still helps folding comparisons sometimes, so I'm proposing we only propagate those. As a bonus: less garbage RTL because a cprop_constant_p can be shared. That seems a bit radical to me, especially in try_replace_reg which is used for copy propagation as well. In cprop_jump, why is attaching a note to the jump problematic? Most of the time a note from copy-propagation was not valid because the copy-prop'd reg was not live at the point of the note. This one I think we should drop for now, or just avoid the self-referential case. There is a comment explicitly saying that the REG_EQUAL note added by try_replace_reg are part of the algorithm. Not really. It uses single_set in a few places, including delete_trivially_dead_insns and cse_extended_basic_block. so it seems like we're back to the earlier trick of using df_note_add_problem to clean up pre-existing REG_EQ* notes. Again: Not really. I also bootstrappedtested without the cse.c change. The cse.c hunk is OK then. I plan (and promise ;-) to add a REG_EQ* note verifier for GCC 4.9. Thanks (no need to promise though :-), that will be helpful. In the meantime, I don't think that we should aim for perfection in 4.8, these REG_EQ* notes and their quirks have been with us for a long time... -- Eric Botcazou
Re: Sparc ASAN
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com
Date: Mon, 3 Dec 2012 22:18:56 +0400
On Mon, Dec 3, 2012 at 10:02 PM, David Miller da...@davemloft.net wrote:
The only changes to libsantizier is to put __sparc__ checks where
__powerpc__ checks exist in the unwind code.
Like this?
===
--- asan/asan_linux.cc (revision 169136)
+++ asan/asan_linux.cc (working copy)
@@ -158,7 +158,9 @@
stack-trace[0] = pc;
if ((max_s) 1) {
stack-max_size = max_s;
-#if defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__)
+#if defined(__arm__) || \
+defined(__powerpc__) || defined(__powerpc64__) || \
+defined(__sparc__)
_Unwind_Backtrace(Unwind_Trace, stack);
// Pop off the two ASAN functions from the backtrace.
stack-PopStackFrames(2);
Yes, that's perfect.
We could also add a __sparc__ block to sanitizer_stacktrace.cc:patch_pc().
The Sparc PC is actually 8 bytes after the caller's jump. Sparc has
a delay slot, the place to return to is 2 instructions after the call/jump,
and instructions are all 4 bytes long.
We either need to align the redzones by 32 always, or for some platforms.
Either is fine for me.
I'm ambivalent as well.
Re: Sparc ASAN
On Mon, Dec 03, 2012 at 10:18:56PM +0400, Konstantin Serebryany wrote: The LLVM implementation always used 32-byte alignment for stack redzones. I never actually did any performance checking on x86 (32-byte aligned vs 8-byte aligned), although I suspect 32-byte aligned redzones should be ~2x faster. Why? The 32-byte realigning has significant cost, plus often one extra register is eaten (the DRAP register), even bigger cost on non-i?86/x86_64 targets. Jakub
Re: [PATCH] asan unit tests from llvm lit-test
Hi,
Jakub, thank you for your so detailed comments! I will fix them
according to your comments. About the lto options, llvm test does't
include it too so I skipped it in torture options. Is it because most
cases we only use asan under O1/O2? Kostya, could you tell us is there
any reason to not test lto+asan in llvm side?
Thanks,
Wei.
On Mon, Dec 3, 2012 at 3:00 AM, Jakub Jelinek ja...@redhat.com wrote:
Hi!
Mike, CCing you especially on the proposed lib/gcc-dg.exp dg-env-var
changes and I have one question about cleanup of files (file delete
vs. remote_file target (or is that host or build) delete).
But of course if you could eyeball the rest and comment, I'd be even happier.
On Fri, Nov 30, 2012 at 12:35:35PM -0800, Wei Mi wrote:
Thanks for the comments! Here is the second version patch. Please see
if it is ok.
(-Wno-attributes is kept or else we will get a warning because of
__attribute__((always_inline))).
--- gcc/testsuite/gcc.dg/asan/asan.exp(revision 194002)
+++ gcc/testsuite/gcc.dg/asan/asan.exp(working copy)
@@ -30,6 +30,10 @@ if ![check_effective_target_faddress_san
dg-init
asan_init
+# Set default torture options
+set default_asan_torture_options [list { -O0 } { -O1 } { -O2 } { -O3 }]
+set-torture-options $default_asan_torture_options
Why this? What is undesirable on the default torture options?
Do those tests fail with lto or similar?
tests on llvm side don't contain lto option so I do the same. Some
tests fail with lto because more aggressive inline.
Index: gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c
===
--- gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c (revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do run } */
+/* { dg-skip-if { *-*-* } { * } { -O2 -m64 } } */
The -m64 here is just wrong. If you want to run the test only
for -O2 and x86_64-linux compilation (why?, what is so specific
about it to that combination?), then you'd do
/* { dg-do run { target { { i?86-*-* x86_64-*-* } lp64 } } } */
/* { dg-skip-if { *-*-* } { * } { -O2 } } */
or so. But again, why?
I copied it from llvm test. I think it just think -m64 test is enough
to check the feature.
--- gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c (revision 0)
@@ -0,0 +1,16 @@
+/* This test checks that we are no instrumenting a memory access twice
+ (before and after inlining) */
+
+/* { dg-do run } */
+/* { dg-options -Wno-attributes } */
+/* { dg-skip-if { *-*-* } { * } { -O0 -m64 -O1 -m64 } } */
As I said above. Why is this not tested for 32-bit testing?
From the name, -O0/-O1 limit could make sense, but then even for -O2 and
above it should do the same.
I also copied it from llvm.
--- gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0)
@@ -0,0 +1,33 @@
+// Check that we can store lots of stack frames if asked to.
+
+// { dg-do run }
+// { dg-env-var ASAN_OPTIONS malloc_context_size=120:redzone=512 }
+// { dg-shouldfail asan }
Can you please replace the two spaces after // with just one?
Dejagnu directives are often quite long, and thus it is IMHO better to make
the lines longer than necessary.
For this test, don't you need
// { dg-options -fno-optimize-sibling-calls }
and __attribute__((noinline)) on the free method? Otherwise I'd expect
that either at least at -O3 it could be all inlined, or if not inlined, then
at least tail call optimized (and thus not showing up in the backtrace
either).
Ok, will fix it.
+#include stdlib.h
+#include stdio.h
+
+template int depth
+struct DeepFree {
+ static void free(char *x) {
+DeepFreedepth - 1::free(x);
+ }
+};
+
+template
+struct DeepFree0 {
+ static void free(char *x) {
+::free(x);
+ }
+};
+
+int main() {
+ char *x = new char[10];
+ // deep_free(x);
+ DeepFree200::free(x);
+ return x[5];
+}
+
+// { dg-output ERROR: AddressSanitizer:? heap-use-after-free on
address.*(\n|\r\n|\r) }
+// { dg-output #37 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*36|\[(\]).*(\n|\r\n|\r) }
+// { dg-output #99 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*98|\[(\]).*(\n|\r\n|\r) }
+// { dg-output #116 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*115|\[(\])\[^\n\r]*(\n|\r\n|\r) }
--- gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0)
@@ -0,0 +1,20 @@
+// { dg-do run }
+// { dg-options -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer
-fno-optimize-sibling-calls }
-mno-omit-leaf-frame-pointer is i?86/x86_64 options, so you need to leave it
from dg-options and add
// { dg-additional-options -mno-omit-leaf-frame-pointer { target {
Re: Sparc ASAN
On Mon, Dec 3, 2012 at 10:29 PM, David Miller da...@davemloft.net wrote:
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com
Date: Mon, 3 Dec 2012 22:18:56 +0400
On Mon, Dec 3, 2012 at 10:02 PM, David Miller da...@davemloft.net wrote:
The only changes to libsantizier is to put __sparc__ checks where
__powerpc__ checks exist in the unwind code.
Like this?
===
--- asan/asan_linux.cc (revision 169136)
+++ asan/asan_linux.cc (working copy)
@@ -158,7 +158,9 @@
stack-trace[0] = pc;
if ((max_s) 1) {
stack-max_size = max_s;
-#if defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__)
+#if defined(__arm__) || \
+defined(__powerpc__) || defined(__powerpc64__) || \
+defined(__sparc__)
_Unwind_Backtrace(Unwind_Trace, stack);
// Pop off the two ASAN functions from the backtrace.
stack-PopStackFrames(2);
Yes, that's perfect.
We could also add a __sparc__ block to sanitizer_stacktrace.cc:patch_pc().
The Sparc PC is actually 8 bytes after the caller's jump. Sparc has
a delay slot, the place to return to is 2 instructions after the call/jump,
and instructions are all 4 bytes long.
Like this?
--- sanitizer_common/sanitizer_stacktrace.cc(revision 169136)
+++ sanitizer_common/sanitizer_stacktrace.cc(working copy)
@@ -36,6 +36,8 @@
#if defined(__powerpc__) || defined(__powerpc64__)
// PCs are always 4 byte aligned.
return pc - 4;
+#elif defined(__sparc__)
+ return pc - 8;
#else
return pc - 1;
#endif
We either need to align the redzones by 32 always, or for some platforms.
Either is fine for me.
I'm ambivalent as well.
Re: Sparc ASAN
On Mon, Dec 3, 2012 at 10:31 PM, Jakub Jelinek ja...@redhat.com wrote: On Mon, Dec 03, 2012 at 10:18:56PM +0400, Konstantin Serebryany wrote: The LLVM implementation always used 32-byte alignment for stack redzones. I never actually did any performance checking on x86 (32-byte aligned vs 8-byte aligned), although I suspect 32-byte aligned redzones should be ~2x faster. Why? The 32-byte realigning has significant cost, plus often one extra register is eaten (the DRAP register), even bigger cost on non-i?86/x86_64 targets. Maybe because my understanding of x86 is rather old (or plain wrong). I tried a micro benchmark on Xeon E5-2690 and unaligned strores are just slightly more expensive ( 10%). I'll do more benchmarks with the actual asan instrumentation ~ tomorrow. So, I guess we need to align the redzones conditionally for sparc, etc. --kcc Jakub
Re: Sparc ASAN
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com Date: Mon, 3 Dec 2012 22:33:12 +0400 On Mon, Dec 3, 2012 at 10:29 PM, David Miller da...@davemloft.net wrote: We could also add a __sparc__ block to sanitizer_stacktrace.cc:patch_pc(). The Sparc PC is actually 8 bytes after the caller's jump. Sparc has a delay slot, the place to return to is 2 instructions after the call/jump, and instructions are all 4 bytes long. Like this? --- sanitizer_common/sanitizer_stacktrace.cc(revision 169136) +++ sanitizer_common/sanitizer_stacktrace.cc(working copy) @@ -36,6 +36,8 @@ #if defined(__powerpc__) || defined(__powerpc64__) // PCs are always 4 byte aligned. return pc - 4; +#elif defined(__sparc__) + return pc - 8; #else return pc - 1; #endif Perfect.
Re: Sparc ASAN
On Mon, Dec 03, 2012 at 10:18:56PM +0400, Konstantin Serebryany wrote:
The LLVM implementation always used 32-byte alignment for stack redzones.
I never actually did any performance checking on x86 (32-byte aligned
vs 8-byte aligned),
although I suspect 32-byte aligned redzones should be ~2x faster.
If the ~2x faster comes from unaligned vs. aligned integer stores, I can't
spot anything like that on e.g.
__attribute__((noinline, noclone)) void
foo (int *p)
{
int i;
for (i = 0; i 32; i++)
p[i] = 0x12345678;
}
int
main (int argc, const char **argv)
{
char buf[1024];
int *p = buf[argc - 1];
int i;
__builtin_printf (%p\n, p);
for (i = 0; i 1; i++)
foo (p);
return 0;
}
Time with zero arguments (i.e. argc 1) is the same as time with 1, 2 or 3
arguments on SandyBridge CPU. I guess there could be penalties on page
boundaries, etc., but I think hot caches is the usual operation on the
stack.
Jakub
Re: Sparc ASAN
On Mon, Dec 3, 2012 at 10:37 PM, David Miller da...@davemloft.net wrote: From: Konstantin Serebryany konstantin.s.serebry...@gmail.com Date: Mon, 3 Dec 2012 22:33:12 +0400 On Mon, Dec 3, 2012 at 10:29 PM, David Miller da...@davemloft.net wrote: We could also add a __sparc__ block to sanitizer_stacktrace.cc:patch_pc(). The Sparc PC is actually 8 bytes after the caller's jump. Sparc has a delay slot, the place to return to is 2 instructions after the call/jump, and instructions are all 4 bytes long. Like this? --- sanitizer_common/sanitizer_stacktrace.cc(revision 169136) +++ sanitizer_common/sanitizer_stacktrace.cc(working copy) @@ -36,6 +36,8 @@ #if defined(__powerpc__) || defined(__powerpc64__) // PCs are always 4 byte aligned. return pc - 4; +#elif defined(__sparc__) + return pc - 8; #else return pc - 1; #endif Perfect. http://llvm.org/viewvc/llvm-project?view=revrevision=169141 Will reach gcc with the next libsanitizer merge (or feel free to commit the same patch directly to gcc). --kcc
Re: Sparc ASAN
From: Konstantin Serebryany konstantin.s.serebry...@gmail.com Date: Mon, 3 Dec 2012 22:44:15 +0400 On Mon, Dec 3, 2012 at 10:37 PM, David Miller da...@davemloft.net wrote: From: Konstantin Serebryany konstantin.s.serebry...@gmail.com Date: Mon, 3 Dec 2012 22:33:12 +0400 On Mon, Dec 3, 2012 at 10:29 PM, David Miller da...@davemloft.net wrote: We could also add a __sparc__ block to sanitizer_stacktrace.cc:patch_pc(). The Sparc PC is actually 8 bytes after the caller's jump. Sparc has a delay slot, the place to return to is 2 instructions after the call/jump, and instructions are all 4 bytes long. Like this? --- sanitizer_common/sanitizer_stacktrace.cc(revision 169136) +++ sanitizer_common/sanitizer_stacktrace.cc(working copy) @@ -36,6 +36,8 @@ #if defined(__powerpc__) || defined(__powerpc64__) // PCs are always 4 byte aligned. return pc - 4; +#elif defined(__sparc__) + return pc - 8; #else return pc - 1; #endif Perfect. http://llvm.org/viewvc/llvm-project?view=revrevision=169141 Will reach gcc with the next libsanitizer merge (or feel free to commit the same patch directly to gcc). Thanks for taking care of this.
Re: [PATCH] asan unit tests from llvm lit-test
On Mon, Dec 3, 2012 at 10:32 PM, Wei Mi w...@google.com wrote:
Hi,
Jakub, thank you for your so detailed comments! I will fix them
according to your comments. About the lto options, llvm test does't
include it too so I skipped it in torture options. Is it because most
cases we only use asan under O1/O2? Kostya, could you tell us is there
any reason to not test lto+asan in llvm side?
The existing tests may fail with lto because lto does more aggressive
optimizations.
No other reasons.
--kcc
Thanks,
Wei.
On Mon, Dec 3, 2012 at 3:00 AM, Jakub Jelinek ja...@redhat.com wrote:
Hi!
Mike, CCing you especially on the proposed lib/gcc-dg.exp dg-env-var
changes and I have one question about cleanup of files (file delete
vs. remote_file target (or is that host or build) delete).
But of course if you could eyeball the rest and comment, I'd be even happier.
On Fri, Nov 30, 2012 at 12:35:35PM -0800, Wei Mi wrote:
Thanks for the comments! Here is the second version patch. Please see
if it is ok.
(-Wno-attributes is kept or else we will get a warning because of
__attribute__((always_inline))).
--- gcc/testsuite/gcc.dg/asan/asan.exp(revision 194002)
+++ gcc/testsuite/gcc.dg/asan/asan.exp(working copy)
@@ -30,6 +30,10 @@ if ![check_effective_target_faddress_san
dg-init
asan_init
+# Set default torture options
+set default_asan_torture_options [list { -O0 } { -O1 } { -O2 } { -O3 }]
+set-torture-options $default_asan_torture_options
Why this? What is undesirable on the default torture options?
Do those tests fail with lto or similar?
tests on llvm side don't contain lto option so I do the same. Some
tests fail with lto because more aggressive inline.
Index: gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c
===
--- gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c (revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do run } */
+/* { dg-skip-if { *-*-* } { * } { -O2 -m64 } } */
The -m64 here is just wrong. If you want to run the test only
for -O2 and x86_64-linux compilation (why?, what is so specific
about it to that combination?), then you'd do
/* { dg-do run { target { { i?86-*-* x86_64-*-* } lp64 } } } */
/* { dg-skip-if { *-*-* } { * } { -O2 } } */
or so. But again, why?
I copied it from llvm test. I think it just think -m64 test is enough
to check the feature.
--- gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c (revision 0)
@@ -0,0 +1,16 @@
+/* This test checks that we are no instrumenting a memory access twice
+ (before and after inlining) */
+
+/* { dg-do run } */
+/* { dg-options -Wno-attributes } */
+/* { dg-skip-if { *-*-* } { * } { -O0 -m64 -O1 -m64 } } */
As I said above. Why is this not tested for 32-bit testing?
From the name, -O0/-O1 limit could make sense, but then even for -O2 and
above it should do the same.
I also copied it from llvm.
--- gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0)
@@ -0,0 +1,33 @@
+// Check that we can store lots of stack frames if asked to.
+
+// { dg-do run }
+// { dg-env-var ASAN_OPTIONS malloc_context_size=120:redzone=512 }
+// { dg-shouldfail asan }
Can you please replace the two spaces after // with just one?
Dejagnu directives are often quite long, and thus it is IMHO better to make
the lines longer than necessary.
For this test, don't you need
// { dg-options -fno-optimize-sibling-calls }
and __attribute__((noinline)) on the free method? Otherwise I'd expect
that either at least at -O3 it could be all inlined, or if not inlined, then
at least tail call optimized (and thus not showing up in the backtrace
either).
Ok, will fix it.
+#include stdlib.h
+#include stdio.h
+
+template int depth
+struct DeepFree {
+ static void free(char *x) {
+DeepFreedepth - 1::free(x);
+ }
+};
+
+template
+struct DeepFree0 {
+ static void free(char *x) {
+::free(x);
+ }
+};
+
+int main() {
+ char *x = new char[10];
+ // deep_free(x);
+ DeepFree200::free(x);
+ return x[5];
+}
+
+// { dg-output ERROR: AddressSanitizer:? heap-use-after-free on
address.*(\n|\r\n|\r) }
+// { dg-output #37 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*36|\[(\]).*(\n|\r\n|\r) }
+// { dg-output #99 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*98|\[(\]).*(\n|\r\n|\r) }
+// { dg-output #116 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*115|\[(\])\[^\n\r]*(\n|\r\n|\r) }
--- gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0)
@@ -0,0 +1,20 @@
+// { dg-do run }
+// { dg-options -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer
Re: [PATCH] asan unit tests from llvm lit-test
On Dec 3, 2012, at 3:00 AM, Jakub Jelinek ja...@redhat.com wrote:
Mike, CCing you especially on the proposed lib/gcc-dg.exp dg-env-var
changes and I have one question about cleanup of files (file delete
vs. remote_file target (or is that host or build) delete).
But of course if you could eyeball the rest and comment, I'd be even happier.
+file delete dlclose-test-1.exe-so.so
+file delete shared-lib-test-1.exe-so.so
Ah, it is here, but wonder what it will do for cross testing.
Shouldn't that be remove_file ? delete where ? is either target, or host, or
build (not sure which one). Mike?
Sounds about right.
#
@@ -287,6 +327,10 @@ proc search_for { file pattern } {
# as c-torture does.
proc gcc-dg-runtest { testcases default-extra-flags } {
global runtests
+global set_env_var
+
+# Init set_env_var
+set set_env_var [list]
# Some callers set torture options themselves; don't override those.
set existing_torture_options [torture-options-exist]
For this, I'd appreciate Mike's input.
When documented, it will say if you want to change the environment variables on
the host, target or build machines. When it says which one, it will be
apparent when tested, if it does as documented. Since I don't know what you
guys want to do (better not to imagine one thinks they know)… I'd leave it to
you guys to figure it out and test. Also, if untested, I'd not see any reason
for it to work; but, maybe I'm just cautious that way. If you can only test
native, just bail out if not native, and try and recruit someone that can test
canadian or cross.
If it is useful for all tests
generally (I'd say it is, we could use it e.g. for testing some of the
libgomp env vars), then it should stay here or so, otherwise it would need
to be moved into asan-dg.exp and have asan in the name.
More importantly, I'm wondering about dg-env-var vs. cross testing, I guess
env var is set on host only, not remotely set on the target. So, either
we should mark all tests that use dg-env-var with some special effective
target that would be basically [is_native]
Ick, no.
- or what is the way to limit tests to native testing only,
roughly:
if [is3way] || ! [isnative]
return
or dg-evn-var itself should arrange to just
make the whole test unsupported if not native (don't call ${tool}_load
at all and return something else?).
If you want to expend the energy, it is easier to just fix the two lines that
are wrong and find some way to smuggle variables to the target. In the end, if
there is no existing way, you'd need to add a way and use it, and if that way
doesn't exist, either omit it, or unsupport it. Of course, that presupposes
you want environment variables on the target.
Re: [cxx-conversion] various hash tables, part 1/2
On Sat, Dec 1, 2012 at 8:47 PM, Lawrence Crowl cr...@googlers.com wrote:
+inline bool
+attribute_hasher::equal (const value_type *spec, const compare_type *str)
+{
+ return (!strncmp (spec-name, str-str, str-length)
I have a slight preference for strncmp() == 0. It's easier to read (I
realize that you are just cut-n-pasting old code here :)
@@ -76,13 +82,11 @@ bitmap_descriptor (const char *file, int
loc.function = function;
loc.line = line;
- if (!bitmap_desc_hash)
-bitmap_desc_hash = htab_create (10, hash_descriptor, eq_descriptor,
NULL);
+ if (!bitmap_desc_hash.is_created ())
+bitmap_desc_hash.create (10);
- slot = (struct bitmap_descriptor **)
-htab_find_slot_with_hash (bitmap_desc_hash, loc,
- htab_hash_pointer (file) + line,
- INSERT);
+ slot = bitmap_desc_hash
+.find_slot_with_hash (loc, htab_hash_pointer (file) + line, INSERT);
I'd rather split the argument list here.
}
-static int
-htab_cu_eq (const void *of1, const void *of2)
+inline bool
+cu_hash_table_entry_hasher::equal (const value_type *entry1,
+ const compare_type *entry2)
No static?
-static hashval_t
-ttypes_filter_hash (const void *pentry)
+inline hashval_t
+ttypes_filter_hasher::hash (const value_type *entry)
{
- const struct ttypes_filter *entry = (const struct ttypes_filter *) pentry;
return TREE_HASH (entry-t);
The patch seems to have changed the types here. Is it value_type or
ttypes_filter?
Looks OK otherwise.
Diego.
Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin
On Dec 3, 2012, at 8:02 AM, Jack Howarth howa...@bromo.med.uc.edu wrote: The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin from using mach_override to mac function interposition So, I'm thinking the sanitizer people will just review and approve it, even though it says darwin and is heavily darwin specific… It's ok by me.
Re: [Patch AArch64] Add support for vectorizable standard math patterns.
On Dec 3, 2012, at 8:43 AM, Marcus Shawcroft marcus.shawcr...@arm.com wrote: Mike, Are you happy to go with the proposal from James or would like to give folk more time to think this one over? Good to go. If people want to improve it, if they see a way, then can submit a patch anytime they want. I didn't see anyone lay down a clearly better way to do this.
Re: [cxx-conversion] various hash tables, part 1/2
On 12/3/12, Diego Novillo dnovi...@google.com wrote:
On Sat, Dec 1, 2012 at 8:47 PM, Lawrence Crowl cr...@googlers.com wrote:
+inline bool
+attribute_hasher::equal (const value_type *spec, const compare_type
*str)
+{
+ return (!strncmp (spec-name, str-str, str-length)
I have a slight preference for strncmp() == 0. It's easier to read (I
realize that you are just cut-n-pasting old code here :)
Done.
@@ -76,13 +82,11 @@ bitmap_descriptor (const char *file, int
loc.function = function;
loc.line = line;
- if (!bitmap_desc_hash)
-bitmap_desc_hash = htab_create (10, hash_descriptor, eq_descriptor,
NULL);
+ if (!bitmap_desc_hash.is_created ())
+bitmap_desc_hash.create (10);
- slot = (struct bitmap_descriptor **)
-htab_find_slot_with_hash (bitmap_desc_hash, loc,
- htab_hash_pointer (file) + line,
- INSERT);
+ slot = bitmap_desc_hash
+.find_slot_with_hash (loc, htab_hash_pointer (file) + line,
INSERT);
I'd rather split the argument list here.
Done, though it becomes three lines instead of two.
}
-static int
-htab_cu_eq (const void *of1, const void *of2)
+inline bool
+cu_hash_table_entry_hasher::equal (const value_type *entry1,
+ const compare_type *entry2)
No static?
The in-class declaration has the static keyword. (It's still a
global symbol though, not local.)
-static hashval_t
-ttypes_filter_hash (const void *pentry)
+inline hashval_t
+ttypes_filter_hasher::hash (const value_type *entry)
{
- const struct ttypes_filter *entry = (const struct ttypes_filter *)
pentry;
return TREE_HASH (entry-t);
The patch seems to have changed the types here. Is it value_type or
ttypes_filter?
The hasher class defines value_type as a typedef to ttypes_filter.
There has been no change in actual type.
Looks OK otherwise.
Okay, I'll put it in the commit queue.
--
Lawrence Crowl
Re: [cxx-conversion] various hash tables, part 1/2
On 2012-12-03 14:24 , Lawrence Crowl wrote: } -static int -htab_cu_eq (const void *of1, const void *of2) +inline bool +cu_hash_table_entry_hasher::equal (const value_type *entry1, + const compare_type *entry2) No static? The in-class declaration has the static keyword. (It's still a global symbol though, not local.) Ah, right. I've seen other cases where the patch removes the 'static' qualifier in free functions, though (in the second patch too). Did you need to make those functions externally available? Diego.
Re: [PATCH] asan unit tests from llvm lit-test
On Mon, Dec 03, 2012 at 11:09:07AM -0800, Mike Stump wrote:
On Dec 3, 2012, at 3:00 AM, Jakub Jelinek ja...@redhat.com wrote:
Mike, CCing you especially on the proposed lib/gcc-dg.exp dg-env-var
changes and I have one question about cleanup of files (file delete
vs. remote_file target (or is that host or build) delete).
But of course if you could eyeball the rest and comment, I'd be even
happier.
+file delete dlclose-test-1.exe-so.so
+file delete shared-lib-test-1.exe-so.so
Ah, it is here, but wonder what it will do for cross testing.
Shouldn't that be remove_file ? delete where ? is either target, or host, or
build (not sure which one). Mike?
Sounds about right.
E.g. cleanup-dump uses remove-build-file, so maybe we should use that.
#
@@ -287,6 +327,10 @@ proc search_for { file pattern } {
# as c-torture does.
proc gcc-dg-runtest { testcases default-extra-flags } {
global runtests
+global set_env_var
+
+# Init set_env_var
+set set_env_var [list]
# Some callers set torture options themselves; don't override those.
set existing_torture_options [torture-options-exist]
For this, I'd appreciate Mike's input.
When documented, it will say if you want to change the environment
variables on the host, target or build machines. When it says which one,
it will be apparent when tested, if it does as documented. Since I don't
know what you guys want to do (better not to imagine one thinks they
know)… I'd leave it to you guys to figure it out and test. Also, if
untested, I'd not see any reason for it to work; but, maybe I'm just
cautious that way. If you can only test native, just bail out if not
native, and try and recruit someone that can test canadian or cross.
The env vars are used by the target libs when running the test executable.
So, are you suggesting we name it dg-set-target-env-var instead of
dg-set-env-var, so that in the future we can also have
dg-set-{host,build}-env-var?
If it is useful for all tests
generally (I'd say it is, we could use it e.g. for testing some of the
libgomp env vars), then it should stay here or so, otherwise it would need
to be moved into asan-dg.exp and have asan in the name.
More importantly, I'm wondering about dg-env-var vs. cross testing, I guess
env var is set on host only, not remotely set on the target. So, either
we should mark all tests that use dg-env-var with some special effective
target that would be basically [is_native]
Ick, no.
- or what is the way to limit tests to native testing only,
roughly:
if [is3way] || ! [isnative]
return
Or if [is_remote target] ? As Wei's dg-set-env-var implementation is
in the ${tool}_load override, we need to return something, so probably
if [is_remote target]
return [list unsupported ]
or so (of course, only if any dg-set-target-env-var directives have been
actually seen in the testcase).
Jakub
Re: [PATCH] asan unit tests from llvm lit-test
On Mon, Dec 03, 2012 at 10:32:52AM -0800, Wei Mi wrote:
Jakub, thank you for your so detailed comments! I will fix them
according to your comments. About the lto options, llvm test does't
include it too so I skipped it in torture options. Is it because most
cases we only use asan under O1/O2? Kostya, could you tell us is there
any reason to not test lto+asan in llvm side?
The former lit-tests are usually single source file anyway, so I think
lto doesn't change much (and by testing also with lto (or -g) we actually
test that those option combinations work with asan too). For the gtest
based tests it matters more, but those are also much more test time
intensive (at least asan_test.C is), so that one is for -O2 only.
--- gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/asan/strip-path-prefix-1.c (revision 0)
@@ -0,0 +1,14 @@
+/* { dg-do run } */
+/* { dg-skip-if { *-*-* } { * } { -O2 -m64 } } */
The -m64 here is just wrong. If you want to run the test only
for -O2 and x86_64-linux compilation (why?, what is so specific
about it to that combination?), then you'd do
/* { dg-do run { target { { i?86-*-* x86_64-*-* } lp64 } } } */
/* { dg-skip-if { *-*-* } { * } { -O2 } } */
or so. But again, why?
I copied it from llvm test. I think it just think -m64 test is enough
to check the feature.
Yeah, I could understand it wants to check somewhere, and with
FILECHECK/llvm that is the way to do that. The above dg-skip-if
will mean though that if you test say on i?86-linux target rather than
x86_64-linux, then it won't be tested at all, and I guess on x86_64-linux
when not using RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}' it won't be
tested either, because -m64 is then not explicitly passed (it is the
default).
--- gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/asan/force-inline-opt0-1.c (revision 0)
@@ -0,0 +1,16 @@
+/* This test checks that we are no instrumenting a memory access twice
+ (before and after inlining) */
+
+/* { dg-do run } */
+/* { dg-options -Wno-attributes } */
+/* { dg-skip-if { *-*-* } { * } { -O0 -m64 -O1 -m64 } } */
As I said above. Why is this not tested for 32-bit testing?
From the name, -O0/-O1 limit could make sense, but then even for -O2 and
above it should do the same.
I also copied it from llvm.
As unlike the gtest based tests, these tests are copied + modified, I think
we should just do what makes sense for GCC testing. And, please do
something about always_inline here too (== no -Wno-attributes). Best if you
could for each comment of mine grep for similar things elsewhere in the
patch, I've commented only on some of the occurences, some things happen
in lots of testcases.
Jakub
Re: [cxx-conversion] various hash tables, part 2/2
On 2012-12-01 20:48 , Lawrence Crowl wrote:
+inline bool
+cselib_hasher::equal (const value_type *v, const compare_type *x_arg)
+{
+ struct elt_loc_list *l;
+ rtx x = CONST_CAST_RTX (x_arg);
+ enum machine_mode mode = GET_MODE (x);
+
+ gcc_assert (!CONST_SCALAR_INT_P (x) GET_CODE (x) != CONST_FIXED);
+
+ if (mode != GET_MODE (v-val_rtx))
+return 0;
+
+ /* Unwrap X if necessary. */
+ if (GET_CODE (x) == CONST
+ (CONST_SCALAR_INT_P (XEXP (x, 0))
+ || GET_CODE (XEXP (x, 0)) == CONST_FIXED))
+x = XEXP (x, 0);
+
+ /* We don't guarantee that distinct rtx's have different hash values,
+ so we need to do a comparison. */
+ for (l = v-locs; l; l = l-next)
+if (rtx_equal_for_cselib_1 (l-loc, x, find_slot_memmode))
+ {
+ promote_debug_loc (l);
+ return 1;
+ }
+
+ return 0;
s/1/true/
s/0/false/
@@ -504,8 +557,10 @@ cselib_get_next_uid (void)
return next_uid;
}
+#if 0
/* See the documentation of cselib_find_slot below. */
static enum machine_mode find_slot_memmode;
+#endif
Remove?
+#if 0
/* The equality test for our hash table. The first argument ENTRY is a table
element (i.e. a cselib_val), while the second arg X is an rtx. We know
that all callers of htab_find_slot_with_hash will wrap CONST_INTs into a
@@ -570,6 +626,7 @@ get_value_hash (const void *entry)
const cselib_val *const v = (const cselib_val *) entry;
return v-hash;
}
+#endif
Remove?
OK otherwise.
This only leaves the GC htabs, right? What was the issue there?
gengtype being silly?
Diego.
Re: [PATCH] asan unit tests from llvm lit-test
On Dec 3, 2012, at 11:36 AM, Jakub Jelinek ja...@redhat.com wrote:
The env vars are used by the target libs when running the test executable.
So, are you suggesting we name it dg-set-target-env-var instead of
dg-set-env-var, so that in the future we can also have
dg-set-{host,build}-env-var?
Yes. The set-build-env I think is non-sensical. The set-host-env version, I
think makes since for hosted reasons, even if we don't implement it today (no
need, yet).
Or if [is_remote target] ?
Yeah.
if [is_remote target]
return [list unsupported ]
Looks nice.
Re: [cxx-conversion] various hash tables, part 1/2
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-03 14:24 , Lawrence Crowl wrote: -static int -htab_cu_eq (const void *of1, const void *of2) +inline bool +cu_hash_table_entry_hasher::equal (const value_type *entry1, + const compare_type *entry2) No static? The in-class declaration has the static keyword. (It's still a global symbol though, not local.) Ah, right. I've seen other cases where the patch removes the 'static' qualifier in free functions, though (in the second patch too). Did you need to make those functions externally available? In C++03, any function argument to a template needs to be externally available. These functions are parameters to the traverse templates. -- Lawrence Crowl
Re: [cxx-conversion] ggc-common hash tables
On 2012-12-01 20:46 , Lawrence Crowl wrote: Index: gcc/ChangeLog 2012-11-30 Lawrence Crowl cr...@google.com * hash-table.h (class hash_table): Correct many methods with parameter types compare_type to the correct value_type. (Correct code was unlikely to notice the change.) (hash_table::elements_with_deleted) New. ':' after ')'. @@ -1024,11 +1035,10 @@ cmp_statistic (const void *loc1, const v /* Collect array of the descriptors from hashtable. */ static struct loc_descriptor **loc_array; -static int -add_statistics (void **slot, void *b) +int +ggc_add_statistics (loc_descriptor **slot, int *n) Why remove 'static'? OK otherwise. Diego.
Re: [cxx-conversion] various hash tables, part 2/2
On 12/3/12, Diego Novillo dnovi...@google.com wrote:
On 2012-12-01 20:48 , Lawrence Crowl wrote:
+inline bool
+cselib_hasher::equal (const value_type *v, const compare_type *x_arg)
+{
+ struct elt_loc_list *l;
+ rtx x = CONST_CAST_RTX (x_arg);
+ enum machine_mode mode = GET_MODE (x);
+
+ gcc_assert (!CONST_SCALAR_INT_P (x) GET_CODE (x) != CONST_FIXED);
+
+ if (mode != GET_MODE (v-val_rtx))
+return 0;
+
+ /* Unwrap X if necessary. */
+ if (GET_CODE (x) == CONST
+ (CONST_SCALAR_INT_P (XEXP (x, 0))
+ || GET_CODE (XEXP (x, 0)) == CONST_FIXED))
+x = XEXP (x, 0);
+
+ /* We don't guarantee that distinct rtx's have different hash values,
+ so we need to do a comparison. */
+ for (l = v-locs; l; l = l-next)
+if (rtx_equal_for_cselib_1 (l-loc, x, find_slot_memmode))
+ {
+promote_debug_loc (l);
+return 1;
+ }
+
+ return 0;
s/1/true/
s/0/false/
Done.
@@ -504,8 +557,10 @@ cselib_get_next_uid (void)
return next_uid;
}
+#if 0
/* See the documentation of cselib_find_slot below. */
static enum machine_mode find_slot_memmode;
+#endif
Remove?
Done.
+#if 0
/* The equality test for our hash table. The first argument ENTRY is a
table
element (i.e. a cselib_val), while the second arg X is an rtx. We
know
that all callers of htab_find_slot_with_hash will wrap CONST_INTs
into a
@@ -570,6 +626,7 @@ get_value_hash (const void *entry)
const cselib_val *const v = (const cselib_val *) entry;
return v-hash;
}
+#endif
Remove?
Done.
OK otherwise.
This only leaves the GC htabs, right?
No, there are still more non-GC htabs.
What was the issue there? gengtype being silly?
Yes.
--
Lawrence Crowl
Re: [cxx-conversion] various hash tables, part 1/2
On Mon, Dec 3, 2012 at 2:50 PM, Lawrence Crowl cr...@googlers.com wrote: On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-03 14:24 , Lawrence Crowl wrote: -static int -htab_cu_eq (const void *of1, const void *of2) +inline bool +cu_hash_table_entry_hasher::equal (const value_type *entry1, + const compare_type *entry2) No static? The in-class declaration has the static keyword. (It's still a global symbol though, not local.) Ah, right. I've seen other cases where the patch removes the 'static' qualifier in free functions, though (in the second patch too). Did you need to make those functions externally available? In C++03, any function argument to a template needs to be externally available. These functions are parameters to the traverse templates. Oh, right. Ignore all my other static-related comments then. Diego.
Re: [cxx-conversion] tree-related hash tables
On 2012-12-01 20:45 , Lawrence Crowl wrote: Index: gcc/tree-hasher.h === --- gcc/tree-hasher.h (revision 0) +++ gcc/tree-hasher.h (revision 0) @@ -0,0 +1,56 @@ +/* Data and Control Flow Analysis for Trees. This is the wrong description for this file. + Copyright (C) 2001, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, + 2012 Free Software Foundation, Inc. + Contributed by Diego Novillo dnovi...@redhat.com Only mention year 2012 and don't blame me for sins I never committed ;) OK otherwise. Diego.
Re: [cxx-conversion] ggc-common hash tables
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:46 , Lawrence Crowl wrote: Index: gcc/ChangeLog 2012-11-30 Lawrence Crowl cr...@google.com * hash-table.h (class hash_table): Correct many methods with parameter types compare_type to the correct value_type. (Correct code was unlikely to notice the change.) (hash_table::elements_with_deleted) New. ':' after ')'. Done. @@ -1024,11 +1035,10 @@ cmp_statistic (const void *loc1, const v /* Collect array of the descriptors from hashtable. */ static struct loc_descriptor **loc_array; -static int -add_statistics (void **slot, void *b) +int +ggc_add_statistics (loc_descriptor **slot, int *n) Why remove 'static'? It is now a template argument, and cannot be file-local. OK otherwise. Thanks. -- Lawrence Crowl
Re: [cxx-conversion] gimplify_ctx::temp_htab hash table
On 2012-12-01 20:44 , Lawrence Crowl wrote: Index: gcc/gimple-fold.c === --- gcc/gimple-fold.c (revision 193902) +++ gcc/gimple-fold.c (working copy) @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. #include tree-ssa-propagate.h #include target.h #include gimple-fold.h +#include gimplify-ctx.h /* Return true when DECL can be referenced from current unit. FROM_DECL (if non-null) specify constructor of variable DECL was taken from. Index: gcc/tree-mudflap.c === --- gcc/tree-mudflap.c (revision 193902) +++ gcc/tree-mudflap.c (working copy) @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. #include ggc.h #include cgraph.h #include gimple.h +#include gimplify-ctx.h extern void add_bb_to_loop (basic_block, struct loop *); Index: gcc/tree-inline.c === --- gcc/tree-inline.c (revision 193902) +++ gcc/tree-inline.c (working copy) @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. #include value-prof.h #include tree-pass.h #include target.h +#include gimplify-ctx.h I don't follow. It seems that factoring into gimplify-ctx.h does not actually buy much. The files using it are just including *another* file. Whereas previously, they were getting that content from gimple.h. Unless we can stop including gimple.h from these files, I don't see a lot of gain in this factoring. Am I missing something? Diego.
Re: [cxx-conversion] tree-related hash tables
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:45 , Lawrence Crowl wrote: Index: gcc/tree-hasher.h === --- gcc/tree-hasher.h(revision 0) +++ gcc/tree-hasher.h(revision 0) @@ -0,0 +1,56 @@ +/* Data and Control Flow Analysis for Trees. This is the wrong description for this file. + Copyright (C) 2001, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, + 2012 Free Software Foundation, Inc. + Contributed by Diego Novillo dnovi...@redhat.com Only mention year 2012 and don't blame me for sins I never committed ;) The dreaded copy and fail to fully edit. Fixed. OK otherwise. Thanks. -- Lawrence Crowl
Re: PATCH: pass sysroot to cc1plus
First ping... anyone?
On 28/11/12 1:21 PM, Etienne Le Sueur wrote:
Hi,
With a sysroot of /dev/null, passing a .i file to cc1plus causes it to attempt to open /dev/null/usr/include, which fails. This causes
problems for ccache and distcc. There is an open bugzilla ticket at [1].
The patch below applies on to 4.6.3, but it appears the bug is still present in
4.7.2.
If this is not the correct way to solve this problem, please suggest a better
approach.
Regards,
Etienne Le Sueur
[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54560
diff --git a/gcc-4.6.3/gcc/cp/lang-specs.h b/gcc-4.6.3/gcc/cp/lang-specs.h
index a73aba3..873609a 100644
--- a/gcc-4.6.3/gcc/cp/lang-specs.h
+++ b/gcc-4.6.3/gcc/cp/lang-specs.h
@@ -64,5 +64,5 @@ along with GCC; see the file COPYING3. If not see
{.ii, @c++-cpp-output, 0, 0, 0},
{@c++-cpp-output,
%{!M:%{!MM:%{!E:\
-cc1plus -fpreprocessed %i %(cc1_options) %2\
+cc1plus -fpreprocessed %i %I %(cc1_options) %2\
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
diff --git a/gcc-4.6.3/gcc/gcc.c b/gcc-4.6.3/gcc/gcc.c
index 75f522e..214ef29 100644
--- a/gcc-4.6.3/gcc/gcc.c
+++ b/gcc-4.6.3/gcc/gcc.c
@@ -950,7 +950,7 @@ static const struct compiler default_compilers[] =
%W{o*:--output-pch=%*}}%V}}, 0, 0, 0},
{.i, @cpp-output, 0, 0, 0},
{@cpp-output,
- %{!M:%{!MM:%{!E:cc1 -fpreprocessed %i %(cc1_options)
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
+ %{!M:%{!MM:%{!E:cc1 -fpreprocessed %i %I %(cc1_options)
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
{.s, @assembler, 0, 0, 0},
{@assembler,
%{!M:%{!MM:%{!E:%{!S:as %(asm_debug) %(asm_options) %i %A , 0, 0, 0},
Re: [cxx-conversion] LTO-related hash tables
On 2012-12-01 20:40 , Lawrence Crowl wrote: Change LTO-related hash tables from htab_t to hash_table: lto-streamer.h output_block::string_hash_table lto-streamer-in.c file_name_hash_table lto-streamer.c tree_htab The struct string_slot moves from data-streamer.h to lto-streamer.h to resolve compilation dependences. What compilation dependences? If it was fine before in data-streamer.h why does it need to move to lto-streamer.h? I'm missing that connection. Diego.
PATCH: pass sysroot to cc1plus
Hi,
With a sysroot of /dev/null, passing a .i file to cc1plus causes it to attempt to open /dev/null/usr/include, which fails. This causes problems
for ccache and distcc. There is an open bugzilla ticket at [1].
The patch below applies on to 4.6.3, but it appears the bug is still present in
4.7.2.
If this is not the correct way to solve this problem, please suggest a better
approach.
Regards,
Etienne Le Sueur
[1] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54560
diff --git a/gcc-4.6.3/gcc/cp/lang-specs.h b/gcc-4.6.3/gcc/cp/lang-specs.h
index a73aba3..873609a 100644
--- a/gcc-4.6.3/gcc/cp/lang-specs.h
+++ b/gcc-4.6.3/gcc/cp/lang-specs.h
@@ -64,5 +64,5 @@ along with GCC; see the file COPYING3. If not see
{.ii, @c++-cpp-output, 0, 0, 0},
{@c++-cpp-output,
%{!M:%{!MM:%{!E:\
-cc1plus -fpreprocessed %i %(cc1_options) %2\
+cc1plus -fpreprocessed %i %I %(cc1_options) %2\
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
diff --git a/gcc-4.6.3/gcc/gcc.c b/gcc-4.6.3/gcc/gcc.c
index 75f522e..214ef29 100644
--- a/gcc-4.6.3/gcc/gcc.c
+++ b/gcc-4.6.3/gcc/gcc.c
@@ -950,7 +950,7 @@ static const struct compiler default_compilers[] =
%W{o*:--output-pch=%*}}%V}}, 0, 0, 0},
{.i, @cpp-output, 0, 0, 0},
{@cpp-output,
- %{!M:%{!MM:%{!E:cc1 -fpreprocessed %i %(cc1_options)
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
+ %{!M:%{!MM:%{!E:cc1 -fpreprocessed %i %I %(cc1_options)
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
{.s, @assembler, 0, 0, 0},
{@assembler,
%{!M:%{!MM:%{!E:%{!S:as %(asm_debug) %(asm_options) %i %A , 0, 0, 0},
Re: [cxx-conversion] gimplify_ctx::temp_htab hash table
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:44 , Lawrence Crowl wrote: Index: gcc/gimple-fold.c === --- gcc/gimple-fold.c(revision 193902) +++ gcc/gimple-fold.c(working copy) @@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. #include tree-ssa-propagate.h #include target.h #include gimple-fold.h +#include gimplify-ctx.h /* Return true when DECL can be referenced from current unit. FROM_DECL (if non-null) specify constructor of variable DECL was taken from. Index: gcc/tree-mudflap.c === --- gcc/tree-mudflap.c (revision 193902) +++ gcc/tree-mudflap.c (working copy) @@ -43,6 +43,7 @@ along with GCC; see the file COPYING3. #include ggc.h #include cgraph.h #include gimple.h +#include gimplify-ctx.h extern void add_bb_to_loop (basic_block, struct loop *); Index: gcc/tree-inline.c === --- gcc/tree-inline.c(revision 193902) +++ gcc/tree-inline.c(working copy) @@ -48,6 +48,7 @@ along with GCC; see the file COPYING3. #include value-prof.h #include tree-pass.h #include target.h +#include gimplify-ctx.h I don't follow. It seems that factoring into gimplify-ctx.h does not actually buy much. The files using it are just including *another* file. Whereas previously, they were getting that content from gimple.h. Unless we can stop including gimple.h from these files, I don't see a lot of gain in this factoring. Am I missing something? There at least 70 files that include gimple.h, and only 5 that need gimple-ctx.h. By splitting it out, at least 65 files will not need to parse the gimplify_ctx struct, the gimple_temp_hash_elt struct, the gimplify_hasher template struct, and may not need to include hash-table.h. It's all about avoiding superfluous compilation in other files. -- Lawrence Crowl
Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
Il 01/12/2012 15:54, Eric Botcazou ha scritto: Attached is a different fix. It splits DF_REF_IN_NOTE in two: One flag for each kind of note. This allows the dead note removal code to distinguish the source note for the EQ_USES. I needed to remove one flag to keep the df_ref_flags 16-bit, but the DF_REF_SUBREG flag looks completely unnecessary to me, and only ira.c uses it, so it wasn't to hard to scavenge a single bit. I'll defer this to Paolo. Ok, I'll review this tomorrow. Paolo
Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
On Mon, Dec 3, 2012 at 7:23 PM, Eric Botcazou wrote: 2. gcse.c: gcse_emit_move_after added notes, but none of them was very useful as far as I could tell, and almost all of them turned self-referencing after CPROP. So I propose we just never add notes in this case. gcse_emit_move_after also preserves existing notes. Are they problematic? Yes, they tend to be invalid after PRE because the registers used in the PRE'd expression usually are not live anymore (making the note invalid). Sometimes CPROP re-validates the notes, but it doesn't seem wise to me to rely on that. So the compiler doesn't bootstrap with the gcse.c patch you posted earlier in the thread? Still this seems too bold to me, the note datum could be a constant and should be preserved in this case. You mean the patch at http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02275.html right? I haven't tried that other patch. I'll test that one. 3. cprop.c: It seems to me that the purpose here is to propagate constants. If a reg could not be propagated, then the REG_EQUAL note will not help much either. Propagating constants via REG_EQUAL notes still helps folding comparisons sometimes, so I'm proposing we only propagate those. As a bonus: less garbage RTL because a cprop_constant_p can be shared. That seems a bit radical to me, especially in try_replace_reg which is used for copy propagation as well. In cprop_jump, why is attaching a note to the jump problematic? Most of the time a note from copy-propagation was not valid because the copy-prop'd reg was not live at the point of the note. This one I think we should drop for now, or just avoid the self-referential case. There is a comment explicitly saying that the REG_EQUAL note added by try_replace_reg are part of the algorithm. I suppose so. But this was all added before RTL fwprop and way before GIMPLE optimizations. Avoiding the self-referential case is just more difficult to do, quite expensive (have to scan the SET_SRC pattern), and AFAICT doesn't bring much pay-off. I'll prepare something to avoid the self-referential case, but I think we're making our lives complicated for no good reason. Not really. It uses single_set in a few places, including delete_trivially_dead_insns and cse_extended_basic_block. so it seems like we're back to the earlier trick of using df_note_add_problem to clean up pre-existing REG_EQ* notes. Again: Not really. I also bootstrappedtested without the cse.c change. The cse.c hunk is OK then. Thanks, I'll commit it separately. I plan (and promise ;-) to add a REG_EQ* note verifier for GCC 4.9. Thanks (no need to promise though :-), that will be helpful. In the meantime, I don't think that we should aim for perfection in 4.8, these REG_EQ* notes and their quirks have been with us for a long time... Well, yes they've been with us for a long time, but my LR_RD change exposed all these problems that were simply hidden before. I think we're safe for GCC 4.8 but I don't feel comfortable about this situation... Ciao! Steven
Re: [Patch, Fortran] Auxiliary functions/fixes for FINAL
OK for trunk. I think that Janus approved the other patch that we talked about last night, did he not? Spent evening fixing unlimited polymorphic bugs - all of them associated with character targets! Cheers Paul On 3 December 2012 16:54, Tobias Burnus bur...@net-b.de wrote: Dear all, this patch adds some auxiliary functions for FINAL - and it fixes some issues which mainly occur with FINAL. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [cxx-conversion] LTO-related hash tables
On 12/3/12, Diego Novillo dnovi...@google.com wrote: On 2012-12-01 20:40 , Lawrence Crowl wrote: Change LTO-related hash tables from htab_t to hash_table: lto-streamer.h output_block::string_hash_table lto-streamer-in.c file_name_hash_table lto-streamer.c tree_htab The struct string_slot moves from data-streamer.h to lto-streamer.h to resolve compilation dependences. What compilation dependences? If it was fine before in data-streamer.h why does it need to move to lto-streamer.h? I'm missing that connection. Before the change, lto-streamer.h output_block::string_hash_table was an htab_t. The element type was opaque, i.e. implicit in the void* casting in the hash functions provided at the htab_create call site. With the change to hash_table, the output_block::string_hash_table declaration needed string_slot_hasher which in turn needed string_slot. But string_slot was defined in data-streamer.h after output_block in lto-streamer.h. So, something had to move. -- Lawrence Crowl
Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
You mean the patch at http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02275.html right? I haven't tried that other patch. I'll test that one. Yes, thanks. I suppose so. But this was all added before RTL fwprop and way before GIMPLE optimizations. Avoiding the self-referential case is just more difficult to do, quite expensive (have to scan the SET_SRC pattern), and AFAICT doesn't bring much pay-off. I'll prepare something to avoid the self-referential case, but I think we're making our lives complicated for no good reason. Can't you just use the same best-effort approach used for fwprop.c here? -- Eric Botcazou
Re: [Patch, Fortran] PR 55548: SYSTEM_CLOCK with integer(8) provides nanosecond resolution, but only microsecond precision (without -lrt)
Unless somebody else has objections, together with a documentation patch and a ChangeLog entry, this is Ok for trunk. thanks, Janne. Here is an updated patch with docu and ChangeLog. I will wait another day before committing, in order to allow for further comments. Committed as r194105. Cheers, Janus 2012-12-02 Janus Weil ja...@gcc.gnu.org PR fortran/55548 * intrinsics/system_clock.c (gf_gettime_mono): Add argument 'tck', which returns the clock resolution. (system_clock_4): Get resolution from gf_gettime_mono, but limit to 1000/s. (system_clock_8): Get resolution from gf_gettime_mono. 2012-12-02 Janus Weil ja...@gcc.gnu.org PR fortran/55548 * intrinsic.texi (SYSTEM_CLOCK): Update documentation of SYSTEM_CLOCK.
Re: [Google 4.7] Backport upstream fission changes (issue6852101)
2012-11-27 Sterling Augustine saugust...@google.com Backport changes to fission implementation required by trunk. See http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02684.html and susbsequent messages for a full description of what needed to change for it to be accepted into trunk. Most of the original changes were already in google-4_7, so this is just the diff between the two implementations. These changes to make the resulting debug info smaller, so are useful to have on this branch. * dwarf2out.h (addr_table_entry_struct): Forward declare. (dw_val_struct): Change name and associated type of field val_index to val_entry. * dwarf2out.c (NOT_INDEXED, NO_INDEX_ASSIGNED, UNRELOCATED_OFFSET, RELOCATED_OFFSET): New defines. (ate_kind): New enum with enumerators ate_kind_rtx, ate_kind_rtx_dtprel, ate_kind_label. (addr_table_entry_struct, addr_table_entry): New structure and type. (dw_loc_list_struct): Change field name from begin_index to begin_entry, likewise with the type. (new_loc_descr, build_cfa_loc, new_addr_loc_descr, add_AT_flag, add_AT_int, add_AT_unsigned, add_AT_double, add_AT_vec, add_AT_string, add_AT_die_ref, add_AT_fde_ref, add_AT_loc, add_AT_loc_list, add_AT_addr, add_AT_file, add_AT_vms_delta, add_AT_lbl_id, add_AT_lineptr, add_AT_macptr, add_AT_offset): Change field name from val_index to val_entry, and intialize properly. (size_of_loc_descr, output_loc_operands): Add assertion and update call. (set_AT_index): Remove obsolete function. (remove_addr_table_entry, add_AT_lbl_id, add_AT_range_list, add_ranges_by_labels): Update prototypes, adjust all calls in file. (dtprel_bool): New enum with dtprel_false and dtprel_true. (dw_addr_op, new_addr_loc_descr): Update functions to use dtprel_bool, plus additional logic to handle val_entries. Use RELOCATED_OFFSET and UNRELOCATED_OFFSET. (AT_index, output_loc_list): Update comment. Update logic to handle val_entry. (add_AT_low_high_pc): Update comment. Change field name from val_index to val_entry, and call add_addr_table entry. (indirect_string_node, index_string_table): Delete obsolete types, variables and tables. (AT_string_form): Move most logic to ... (find_string_form): ... here and ... (set_indirect_string): ... here. New function. (addr_index_table): Change type from vector to hash table. (addr_table_entry_do_hash, addr_table_entry_eq, init_addr_table_entry, index_addr_table_entry): New functions. (add_addr_table_entry): Update prototype. Convert logic to hash tables. (remove_loc_list_addr_table_entries): Convert logic to hash tables. (size_of_die, value_format, output_attr_index_or_value): Use NOT_INDEXED and NO_INDEX_ASSIGNED. (output_range_list_offset): Use RELOCATED_OFFSET. Update comment. (output_comdat_type_unit): Check OBJECT_FORMAT_ELF. (add_ranges_by_labels, loc_descriptor, mem_loc_descriptor, loc_list_from_tree, add_const_value_attribute): Update comment. Use dtprel_bool enumerators. (output_macinfo_op): Fix FIXME by handling DW_FORM_strp's and DW_FORM_GNU_str_index. (save_macinfo_strings, index_string, output_index_string_offset, output_index_string, output_indirect_strings, output_addr_table_entry): New functions. (output_indirect_string): Check refcount. (output_addr_table): Update logic for hash tables. (resolve_addr_in_expr, resolve_addr, hash_loc_operands, compare_loc_operands, index_location_lists): Update logic for val_entry. (dwarf2out_finish): Update logic for val_entry. Call htab_traverse_noresize multiple times, save macinfo_strings, index_string output_addr_table, output_indirect_strings. Add comments. Remove calls to output_addr_table, output_index_strings, and index_location_lists. This is OK for the google/gcc-4_7 branch. Thanks! -cary
Re: Sparc ASAN
On 2012-11-21 11:28, Peter Bergner wrote:
+ if (Unwind_GetBP(ctx) == p-bp) {
I've mentioned multiple times that BP is unusable on most RISC.
You need to be looking at SP.
r~
Re: [PATCH, libatomic] detect and build for ARM architecture armv8-a
On 2012-12-03 10:59, Marcus Shawcroft wrote: 2012-12-03 Marcus Shawcroft marcus.shawcr...@arm.com * config/arm/arm-config.h (__ARM_ARCH_8A__): New. Ok. r~
Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)
On Mon, Dec 3, 2012 at 9:19 PM, Steven Bosscher wrote:
So the compiler doesn't bootstrap with the gcse.c patch you posted earlier in
the thread? Still this seems too bold to me, the note datum could be a
constant and should be preserved in this case.
You mean the patch at
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02275.html right?
I haven't tried that other patch. I'll test that one.
...
The cse.c hunk is OK then.
Thanks, I'll commit it separately.
This is what I'll commit in a second.
A new proposed cprop.c change will follow later.
Ciao!
Steven
* gcse.c (struct reg_use): Remove unused struct.
(gcse_emit_move_after): Do not create REG_EQUAL notes that reference
the SET_DEST of the instruction the note would be attached to.
* cse.c (cse_main): Add the DF_NOTE problem.
Index: gcse.c
===
--- gcse.c (revision 194084)
+++ gcse.c (working copy)
@@ -255,8 +255,6 @@ int flag_rerun_cse_after_global_opts;
/* An obstack for our working variables. */
static struct obstack gcse_obstack;
-struct reg_use {rtx reg_rtx; };
-
/* Hash table of expressions. */
struct expr
@@ -2491,23 +2489,27 @@ gcse_emit_move_after (rtx dest, rtx src, rtx insn)
rtx new_rtx;
rtx set = single_set (insn), set2;
rtx note;
- rtx eqv;
+ rtx eqv = NULL_RTX;
/* This should never fail since we're creating a reg-reg copy
we've verified to be valid. */
new_rtx = emit_insn_after (gen_move_insn (dest, src), insn);
- /* Note the equivalence for local CSE pass. */
+ /* Note the equivalence for local CSE pass. Take the note from the old
+ set if there was one. Otherwise record the SET_SRC from the old set
+ unless DEST is also an operand of the SET_SRC. */
set2 = single_set (new_rtx);
if (!set2 || !rtx_equal_p (SET_DEST (set2), dest))
return new_rtx;
if ((note = find_reg_equal_equiv_note (insn)))
eqv = XEXP (note, 0);
- else
+ else if (! REG_P (dest)
+ || ! reg_mentioned_p (dest, SET_SRC (set)))
eqv = SET_SRC (set);
- set_unique_reg_note (new_rtx, REG_EQUAL, copy_insn_1 (eqv));
+ if (eqv != NULL_RTX)
+set_unique_reg_note (new_rtx, REG_EQUAL, copy_insn_1 (eqv));
return new_rtx;
}
Index: cse.c
===
--- cse.c (revision 194084)
+++ cse.c (working copy)
@@ -6520,6 +6520,7 @@ cse_main (rtx f ATTRIBUTE_UNUSED, int nregs)
int i, n_blocks;
df_set_flags (DF_LR_RUN_DCE);
+ df_note_add_problem ();
df_analyze ();
df_set_flags (DF_DEFER_INSN_RESCAN);
Re: [IA-64] Fix dynamic allocation in leaf functions
On 2012-11-19 16:12, Eric Botcazou wrote: 2012-11-19 Eric Botcazou ebotca...@adacore.com * config/ia64/ia64.c (ia64_compute_frame_size): Allocate the scratch area if the function allocates dynamic stack space. (ia64_initial_elimination_offset): Adjust offsets to above change. Ok for any branch. r~
Re: [PATCH] libgcc refactor aarch64 sfp-machine.h
Richard Earnshaw rearn...@arm.com writes: On 03/12/12 16:00, Marcus Shawcroft wrote: PING I would have thought that both this and the other patch (the update from GLibC) comes under the remit of the port maintainer, rather than the libgcc maintainer. So I would work on the assumption that if Ian doesn't say anything in the next 24 hours, this is OK to go in. It is always true that the port maintainer can approve changes to libgcc files that are specific to that port. The port maintainer knows those files better than the libgcc maintainers do. Ian R. /Marcus On 15 Nov 2012, at 15:58, Marcus Shawcroft marcus.shawcr...@arm.com wrote: This patch reorganizes the AArch64 sfp-machine.h file, splitting out FP_HANDLE_EXCEPTIONS into a new file sfp-exceptions.c following the same idiom used in ia64 and i386. OK ? Cheers /Marcus 2012-11-15 Marcus Shawcroft marcus.shawcr...@arm.com * config/aarch64/sfp-machine.h (FP_RND_MASK): Define. (FP_ROUNDMODE): Use FP_RND_MASK. * config/aarch64/sfp-exceptions.c: New. * config/aarch64/sfp-machine.h (FP_HANDLE_EXCEPTIONS): Use __sfp_handle_exceptions.refactor-sfp-machine.diff
Ping: Synopsys DesignWare ARC Port
This patch set hasn't been reviewed for more than a week: libgcc: 2012-10-09 Joern Rennecke joern.renne...@embecosm.com * config.host (arc-*-elf*, arc*-*-linux-uclibc*): New configurations. gcc: 2012-11-22 Joern Rennecke joern.renne...@embecosm.com Brendan Kehoe bren...@zen.org * config.gcc (arc-*-elf*, arc*-*-linux-uclibc*): New configurations. * doc/install.texi (--with-cpu): Mention ARC. (arc-*-elf32): New paragraph. (arc-linux-uclibc): Likewise. * doc/md.texi (Machine Constraints): Add ARC part. * doc/invoke.texi: (menu): Add ARC Options. (Machine Dependent Options) ARC Options: Add synopsis. (node ARC Options): Add. * doc/extend.texi (long_call / short_call attribute): Add ARC. gcc/testsuite: 2012-11-22 Joern Rennecke joern.renne...@embecosm.com * gcc.c-torture/execute/20101011-1.c [__arc__] (DO_TEST): Define as 0. * gcc.dg/torture/pr37868.c: Also skip for arc*-*-*. * gcc.dg/stack-usage-1.c [__arc__] (SIZE): Define. libstdc++-v3: 2012-08-16 Joern Rennecke joern.renne...@embecosm.com * acinclude.m4 (GLIBCXX_ENABLE_SJLJ_EXCEPTIONS): Also check for _Unwind_SjLj_Register when deciding if to set enable_sjlj_exceptions. * configure: Regenerate. gcc: 2012-11-25 Saurabh Verma saurabh.ve...@codito.com Ramana Radhakrishnan ramana.radhakrish...@codito.com Joern Rennecke joern.renne...@embecosm.com Muhammad Khurram Riaz khurram.r...@arc.com Brendan Kehoe bren...@zen.org Michael Eager ea...@eagercon.com * config/arc, common/config/arc: New directories. gcc/testsuite: 2012-08-28 Joern Rennecke joern.renne...@embecosm.com * gcc.target/arc: New directory. libgcc: 2012-10-18 Joern Rennecke joern.renne...@embecosm.com Brendan Kehoe bren...@zen.org * libgcc/config/arc: New directory. contents of config/arc: http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02051.html http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02052.html http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02055.html Plus the other stuff from here: http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01891.html
Go patch committed: Don't permit go/defer calls to be parenthesized
The Go language spec does not permit go or defer calls to be
parenthesized, but gccgo was permitting it. This patch issues an error
for that case. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 4427b2edf858 go/parse.cc
--- a/go/parse.cc Sun Dec 02 23:20:02 2012 -0800
+++ b/go/parse.cc Mon Dec 03 16:26:16 2012 -0800
@@ -4089,13 +4089,16 @@
|| this-peek_token()-is_keyword(KEYWORD_DEFER));
bool is_go = this-peek_token()-is_keyword(KEYWORD_GO);
Location stat_location = this-location();
- this-advance_token();
+
+ const Token* token = this-advance_token();
Location expr_location = this-location();
+ bool is_parenthesized = token-is_op(OPERATOR_LPAREN);
+
Expression* expr = this-expression(PRECEDENCE_NORMAL, false, true, NULL);
Call_expression* call_expr = expr-call_expression();
- if (call_expr == NULL)
-{
- error_at(expr_location, expected call expression);
+ if (is_parenthesized || call_expr == NULL)
+{
+ error_at(expr_location, argument to go/defer must be function call);
return;
}
Backported r185231 from trunk. (issue 6878045)
Reviewers: davidxl, xur,
Message:
When I backported this patch to google/gcc-4.6, I forgot to do it for
main. So now I am backporting this to google/main, google/4_7 and
google/4_7-mobile.
This is the first of the 3 (google/main is the target).
I am running crosstool_validate.py and it should most likely pass.
Meanwhile, please look at the patch.
Description:
2012-03-12 Richard Guenther rguent...@suse.de
* gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
* gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
(__gthread_mutex_init_function): New function.
* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
PR gcov/49484
* libgcov.c: Include gthr.h.
(__gcov_flush_mx): New global variable.
(init_mx, init_mx_once): New functions.
(__gcov_flush): Protect self with a mutex.
(__gcov_fork): Re-initialize mutex after forking.
* unwind-dw2-fde.c: Change condition under which to use
__GTHREAD_MUTEX_INIT_FUNCTION.
Please review this at https://codereview.appspot.com/6878045/
Affected files:
M.
Mcontrib
Mgcc
M gcc/ChangeLog.google-main
Mgcc/testsuite/gcc.target/powerpc/ppc-round.c
M libgcc/ChangeLog
M libgcc/gthr-posix.h
M libgcc/gthr-single.h
M libgcc/gthr.h
M libgcc/libgcov.c
M libgcc/unwind-dw2-fde.c
Mlibjava/classpath
Property changes on: .
___
Modified: svn:mergeinfo
Merged /trunk:r185231
Property changes on: gcc
___
Modified: svn:mergeinfo
Merged /trunk/gcc:r185231
Property changes on: gcc/testsuite/gcc.target/powerpc/ppc-round.c
___
Modified: svn:mergeinfo
Merged /trunk/gcc/testsuite/gcc.target/powerpc/ppc-round.c:r185231
Index: gcc/ChangeLog.google-main
===
--- gcc/ChangeLog.google-main (revision 194106)
+++ gcc/ChangeLog.google-main (working copy)
@@ -1,3 +1,24 @@
+2012-12-03 Ahmad Sharif asha...@google.com
+
+ Backport r185231 from trunk.
+
+ 2012-03-12 Richard Guenther rguent...@suse.de
+
+ * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
+ * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+ (__gthread_mutex_init_function): New function.
+ * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+
+ PR gcov/49484
+ * libgcov.c: Include gthr.h.
+ (__gcov_flush_mx): New global variable.
+ (init_mx, init_mx_once): New functions.
+ (__gcov_flush): Protect self with a mutex.
+ (__gcov_fork): Re-initialize mutex after forking.
+ * unwind-dw2-fde.c: Change condition under which to use
+ __GTHREAD_MUTEX_INIT_FUNCTION.
+
+
2012-11-09 Harshit Chopra hars...@google.com
* gcc/c-family/c-common.c
(handle_always_patch_for_instrumentation_attribute):
Index: libgcc/unwind-dw2-fde.c
===
--- libgcc/unwind-dw2-fde.c (revision 194106)
+++ libgcc/unwind-dw2-fde.c (working copy)
@@ -47,11 +47,10 @@
#ifdef __GTHREAD_MUTEX_INIT
static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT;
+#define init_object_mutex_once()
#else
static __gthread_mutex_t object_mutex;
-#endif
-#ifdef __GTHREAD_MUTEX_INIT_FUNCTION
static void
init_object_mutex (void)
{
@@ -64,8 +63,6 @@
static __gthread_once_t once = __GTHREAD_ONCE_INIT;
__gthread_once (once, init_object_mutex);
}
-#else
-#define init_object_mutex_once()
#endif
/* Called from crtbegin.o to register the unwind info for an object. */
Index: libgcc/gthr.h
===
--- libgcc/gthr.h (revision 194106)
+++ libgcc/gthr.h (working copy)
@@ -52,11 +52,12 @@
to initialize __gthread_mutex_t to get a fast
non-recursive mutex.
__GTHREAD_MUTEX_INIT_FUNCTION
- some systems can't initialize a mutex without a
- function call. On such systems, define this to a
- function which looks like this:
+ to initialize __gthread_mutex_t to get a fast
+ non-recursive mutex.
+ Define this to a function which looks like this:
void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
- Don't define __GTHREAD_MUTEX_INIT in this case
+ Some systems can't initialize a mutex without a
+ function call. Don't define __GTHREAD_MUTEX_INIT in this case.
__GTHREAD_RECURSIVE_MUTEX_INIT
__GTHREAD_RECURSIVE_MUTEX_INIT_FUNCTION
as above, but for a recursive mutex.
Index: libgcc/gthr-posix.h
===
---
Re: Backported r185231 from trunk. (issue 6878045)
On 2012/12/04 00:50:45, asharif wrote: When I backported this patch to google/gcc-4.6, I forgot to do it for main. So now I am backporting this to google/main, google/4_7 and google/4_7-mobile. This is the first of the 3 (google/main is the target). I am running crosstool_validate.py and it should most likely pass. Meanwhile, please look at the patch. FYI, the original patch is here: https://codereview.appspot.com/6139063/, and it was submitted in CL: http://gcc.gnu.org/viewcvs?view=revisionrevision=187026. https://codereview.appspot.com/6878045/
Re: Backported r185231 from trunk. (issue 6878045)
Looks good for gcc-4_7* branches. There is no need to port any trunk
changes to google/main (it will be synced to trunk at some point).
David
On Mon, Dec 3, 2012 at 4:50 PM, asha...@chromium.org wrote:
Reviewers: davidxl, xur,
Message:
When I backported this patch to google/gcc-4.6, I forgot to do it for
main. So now I am backporting this to google/main, google/4_7 and
google/4_7-mobile.
This is the first of the 3 (google/main is the target).
I am running crosstool_validate.py and it should most likely pass.
Meanwhile, please look at the patch.
Description:
2012-03-12 Richard Guenther rguent...@suse.de
* gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
* gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
(__gthread_mutex_init_function): New function.
* gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
PR gcov/49484
* libgcov.c: Include gthr.h.
(__gcov_flush_mx): New global variable.
(init_mx, init_mx_once): New functions.
(__gcov_flush): Protect self with a mutex.
(__gcov_fork): Re-initialize mutex after forking.
* unwind-dw2-fde.c: Change condition under which to use
__GTHREAD_MUTEX_INIT_FUNCTION.
Please review this at https://codereview.appspot.com/6878045/
Affected files:
M.
Mcontrib
Mgcc
M gcc/ChangeLog.google-main
Mgcc/testsuite/gcc.target/powerpc/ppc-round.c
M libgcc/ChangeLog
M libgcc/gthr-posix.h
M libgcc/gthr-single.h
M libgcc/gthr.h
M libgcc/libgcov.c
M libgcc/unwind-dw2-fde.c
Mlibjava/classpath
Property changes on: .
___
Modified: svn:mergeinfo
Merged /trunk:r185231
Property changes on: gcc
___
Modified: svn:mergeinfo
Merged /trunk/gcc:r185231
Property changes on: gcc/testsuite/gcc.target/powerpc/ppc-round.c
___
Modified: svn:mergeinfo
Merged /trunk/gcc/testsuite/gcc.target/powerpc/ppc-round.c:r185231
Index: gcc/ChangeLog.google-main
===
--- gcc/ChangeLog.google-main (revision 194106)
+++ gcc/ChangeLog.google-main (working copy)
@@ -1,3 +1,24 @@
+2012-12-03 Ahmad Sharif asha...@google.com
+
+ Backport r185231 from trunk.
+
+ 2012-03-12 Richard Guenther rguent...@suse.de
+
+ * gthr.h (__GTHREAD_MUTEX_INIT_FUNCTION): Adjust specification.
+ * gthr-posix.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+ (__gthread_mutex_init_function): New function.
+ * gthr-single.h (__GTHREAD_MUTEX_INIT_FUNCTION): Define.
+
+ PR gcov/49484
+ * libgcov.c: Include gthr.h.
+ (__gcov_flush_mx): New global variable.
+ (init_mx, init_mx_once): New functions.
+ (__gcov_flush): Protect self with a mutex.
+ (__gcov_fork): Re-initialize mutex after forking.
+ * unwind-dw2-fde.c: Change condition under which to use
+ __GTHREAD_MUTEX_INIT_FUNCTION.
+
+
2012-11-09 Harshit Chopra hars...@google.com
* gcc/c-family/c-common.c
(handle_always_patch_for_instrumentation_attribute):
Index: libgcc/unwind-dw2-fde.c
===
--- libgcc/unwind-dw2-fde.c (revision 194106)
+++ libgcc/unwind-dw2-fde.c (working copy)
@@ -47,11 +47,10 @@
#ifdef __GTHREAD_MUTEX_INIT
static __gthread_mutex_t object_mutex = __GTHREAD_MUTEX_INIT;
+#define init_object_mutex_once()
#else
static __gthread_mutex_t object_mutex;
-#endif
-#ifdef __GTHREAD_MUTEX_INIT_FUNCTION
static void
init_object_mutex (void)
{
@@ -64,8 +63,6 @@
static __gthread_once_t once = __GTHREAD_ONCE_INIT;
__gthread_once (once, init_object_mutex);
}
-#else
-#define init_object_mutex_once()
#endif
/* Called from crtbegin.o to register the unwind info for an object. */
Index: libgcc/gthr.h
===
--- libgcc/gthr.h (revision 194106)
+++ libgcc/gthr.h (working copy)
@@ -52,11 +52,12 @@
to initialize __gthread_mutex_t to get a fast
non-recursive mutex.
__GTHREAD_MUTEX_INIT_FUNCTION
- some systems can't initialize a mutex without a
- function call. On such systems, define this to a
- function which looks like this:
+ to initialize __gthread_mutex_t to get a fast
+ non-recursive mutex.
+ Define this to a function which looks like this:
void __GTHREAD_MUTEX_INIT_FUNCTION (__gthread_mutex_t *)
- Don't define __GTHREAD_MUTEX_INIT in this case
+ Some systems can't initialize a mutex without a
+
Ping: RE: [Patch, ARM] Fix the check on arg reg number in function thumb_find_work_register
Hi Ramana, Can you please help to review this patch? Thanks. BR, Terry -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Terry Guo Sent: Wednesday, November 28, 2012 1:53 PM To: gcc-patches@gcc.gnu.org Subject: [Patch, ARM] Fix the check on arg reg number in function thumb_find_work_register Hello, Attached patch intends to fix a bug on how to check argument register number which should consider the PCS. A test case is also included. Without this fix, one of the function argument will be overridden in the case. Tested on QEMU for cortex-m3, no regression found. Is it OK to trunk? BR, Terry gcc/ChangeLog: 2012-11-28 Terry Guo terry@arm.com * config/arm/arm.c (thumb_find_work_register): Check argument register number based on current PCS. gcc/testsuite/ChangeLog: 2012-11-28 Terry Guo terry@arm.com * gcc.target/arm/thumb-find-work-register.c: New.
Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin
Jack, Note that MAC_INTERPOSE_FUNCTIONS is always defined in interception.h to either 0 or 1. I'm going to keep #if MAC_INTERPOSE_FUNCTIONS (adding !defined(MISSING_BLOCKS_SUPPORT) where appropriate) in libsanitizer. On Mon, Dec 3, 2012 at 11:17 AM, Mike Stump mikest...@comcast.net wrote: On Dec 3, 2012, at 8:02 AM, Jack Howarth howa...@bromo.med.uc.edu wrote: The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin from using mach_override to mac function interposition So, I'm thinking the sanitizer people will just review and approve it, even though it says darwin and is heavily darwin specific… It's ok by me. -- Alexander Potapenko Software Engineer Google Moscow
Ping-2: RE: [RFC] New feature to reuse one multilib among different targets
Hi Joseph, Can you please review this patch? If I missed something, please point out. Thanks. BR, Terry -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Terry Guo Sent: Friday, November 23, 2012 5:12 PM To: jos...@codesourcery.com Cc: gcc-patches@gcc.gnu.org Subject: Ping: RE: [RFC] New feature to reuse one multilib among different targets Hi Joseph, Can you please help to review this patch and share your thoughts on this feature? Thanks. BR, Terry -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Terry Guo Sent: Tuesday, November 13, 2012 12:47 PM To: jos...@codesourcery.com Cc: gcc-patches@gcc.gnu.org Subject: RE: [RFC] New feature to reuse one multilib among different targets -Original Message- From: Joseph Myers [mailto:jos...@codesourcery.com] Sent: Saturday, November 10, 2012 12:35 AM To: Terry Guo Cc: gcc-patches@gcc.gnu.org Subject: RE: [RFC] New feature to reuse one multilib among different targets On Fri, 9 Nov 2012, Terry Guo wrote: You are right that we should make script POSIX compliant. This v3 patch removed function and local which don't belong to POSIX standard. I also verified that CONFIG_SHELL is passed to this script with value /bin/sh. Suppose /bin/sh is not a POSIX shell but the user sets CONFIG_SHELL to something else (which is a POSIX shell). Will SHELL in the makefile get set to the POSIX shell the user specified as CONFIG_SHELL? That's what's needed to be able to use POSIX shell features in this script. The attached patch is updated to use sub-script rather than the function to reuse code. Is it ok to avoid the issue you just mentioned? BR, Terry 2012-11-13 Terry Guo terry@arm.com * genmultilib (tmpmultilib3): New refactored sub-script to convert the option combination into folder name. (tmpmultilib4): New refactored sub-script to output the options in a option combination. (MULTILIB_REUSE): New argument. * Makefile.in (s-mlib): Add a new argument MULTILIB_REUSE. * gcc.c (multilib_reuse): New spec. (set_multilib_dir): Use multilib_reuse. * doc/fragments.texi: Mention MULTILIB_REUSE.
Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin
I've added MISSING_BLOCKS_SUPPORT to LLVM compiler-rt in r169206. The rest of your change looks good to me as well. On Mon, Dec 3, 2012 at 6:33 PM, Alexander Potapenko gli...@google.com wrote: Jack, Note that MAC_INTERPOSE_FUNCTIONS is always defined in interception.h to either 0 or 1. I'm going to keep #if MAC_INTERPOSE_FUNCTIONS (adding !defined(MISSING_BLOCKS_SUPPORT) where appropriate) in libsanitizer. On Mon, Dec 3, 2012 at 11:17 AM, Mike Stump mikest...@comcast.net wrote: On Dec 3, 2012, at 8:02 AM, Jack Howarth howa...@bromo.med.uc.edu wrote: The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin from using mach_override to mac function interposition So, I'm thinking the sanitizer people will just review and approve it, even though it says darwin and is heavily darwin specific… It's ok by me. -- Alexander Potapenko Software Engineer Google Moscow -- Alexander Potapenko Software Engineer Google Moscow
Go patch committed: Fix crash on go/defer of some builtin functions
This patch to the Go compiler fixes a crash when using go or defer with
some builtin functions, namely those that have no side-effects.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline and 4.7 branch.
Ian
diff -r 952fc7825cfa go/expressions.cc
--- a/go/expressions.cc Mon Dec 03 16:27:21 2012 -0800
+++ b/go/expressions.cc Mon Dec 03 20:57:51 2012 -0800
@@ -80,10 +80,11 @@
// expression is being discarded. By default, we give an error.
// Expressions with side effects override.
-void
+bool
Expression::do_discarding_value()
{
this-unused_value_error();
+ return false;
}
// This virtual function is called to export expressions. This will
@@ -100,7 +101,7 @@
void
Expression::unused_value_error()
{
- error_at(this-location(), value computed is not used);
+ this-report_error(_(value computed is not used));
}
// Note that this expression is an error. This is called by children
@@ -786,9 +787,9 @@
return true;
}
- void
+ bool
do_discarding_value()
- { }
+ { return true; }
Type*
do_type()
@@ -1149,9 +1150,9 @@
{ }
protected:
- void
+ bool
do_discarding_value()
- { }
+ { return true; }
Type*
do_type();
@@ -5326,13 +5327,19 @@
// Note that the value is being discarded.
-void
+bool
Binary_expression::do_discarding_value()
{
if (this-op_ == OPERATOR_OROR || this-op_ == OPERATOR_ANDAND)
-this-right_-discarding_value();
- else
-this-unused_value_error();
+{
+ this-right_-discarding_value();
+ return true;
+}
+ else
+{
+ this-unused_value_error();
+ return false;
+}
}
// Get type.
@@ -6536,7 +6543,7 @@
bool
do_numeric_constant_value(Numeric_constant*) const;
- void
+ bool
do_discarding_value();
Type*
@@ -7338,7 +7345,7 @@
// discarding the value of an ordinary function call, but we do for
// builtin functions, purely for consistency with the gc compiler.
-void
+bool
Builtin_call_expression::do_discarding_value()
{
switch (this-code_)
@@ -7359,7 +7366,7 @@
case BUILTIN_OFFSETOF:
case BUILTIN_SIZEOF:
this-unused_value_error();
- break;
+ return false;
case BUILTIN_CLOSE:
case BUILTIN_COPY:
@@ -7368,7 +7375,7 @@
case BUILTIN_PRINT:
case BUILTIN_PRINTLN:
case BUILTIN_RECOVER:
- break;
+ return true;
}
}
diff -r 952fc7825cfa go/expressions.h
--- a/go/expressions.h Mon Dec 03 16:27:21 2012 -0800
+++ b/go/expressions.h Mon Dec 03 20:57:51 2012 -0800
@@ -360,10 +360,11 @@
// This is called if the value of this expression is being
// discarded. This issues warnings about computed values being
- // unused.
- void
+ // unused. This returns true if all is well, false if it issued an
+ // error message.
+ bool
discarding_value()
- { this-do_discarding_value(); }
+ { return this-do_discarding_value(); }
// Return whether this is an error expression.
bool
@@ -689,7 +690,7 @@
{ return false; }
// Called by the parser if the value is being discarded.
- virtual void
+ virtual bool
do_discarding_value();
// Child class holds type.
@@ -1205,7 +1206,7 @@
bool
do_numeric_constant_value(Numeric_constant*) const;
- void
+ bool
do_discarding_value();
Type*
@@ -1373,9 +1374,9 @@
virtual Expression*
do_lower(Gogo*, Named_object*, Statement_inserter*, int);
- void
+ bool
do_discarding_value()
- { }
+ { return true; }
virtual Type*
do_type();
@@ -2056,9 +2057,9 @@
do_traverse(Traverse* traverse)
{ return Expression::traverse(this-channel_, traverse); }
- void
+ bool
do_discarding_value()
- { }
+ { return true; }
Type*
do_type();
diff -r 952fc7825cfa go/statements.cc
--- a/go/statements.cc Mon Dec 03 16:27:21 2012 -0800
+++ b/go/statements.cc Mon Dec 03 20:57:51 2012 -0800
@@ -2006,6 +2006,8 @@
void
Thunk_statement::do_check_types(Gogo*)
{
+ if (!this-call_-discarding_value())
+return;
Call_expression* ce = this-call_-call_expression();
if (ce == NULL)
{
@@ -2471,11 +2473,15 @@
Expression_statement* es =
static_castExpression_statement*(call_statement);
Call_expression* ce = es-expr()-call_expression();
- go_assert(ce != NULL);
- if (may_call_recover)
- ce-set_is_deferred();
- if (recover_arg != NULL)
- ce-set_recover_arg(recover_arg);
+ if (ce == NULL)
+ go_assert(saw_errors());
+ else
+ {
+ if (may_call_recover)
+ ce-set_is_deferred();
+ if (recover_arg != NULL)
+ ce-set_recover_arg(recover_arg);
+ }
}
// That is all the thunk has to do.
Re: Sparc ASAN
I've committed a flag to the LLVM implementation to not realign the
stack (-mllvm -asan-realign-stack=0).
On Xeon W3690 I've measured no performance difference (tried C/C++
part of SPEC2006).
So, on x86 it's probably the right thing to not realign the stack.
--kcc
On Mon, Dec 3, 2012 at 10:41 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 03, 2012 at 10:18:56PM +0400, Konstantin Serebryany wrote:
The LLVM implementation always used 32-byte alignment for stack redzones.
I never actually did any performance checking on x86 (32-byte aligned
vs 8-byte aligned),
although I suspect 32-byte aligned redzones should be ~2x faster.
If the ~2x faster comes from unaligned vs. aligned integer stores, I can't
spot anything like that on e.g.
__attribute__((noinline, noclone)) void
foo (int *p)
{
int i;
for (i = 0; i 32; i++)
p[i] = 0x12345678;
}
int
main (int argc, const char **argv)
{
char buf[1024];
int *p = buf[argc - 1];
int i;
__builtin_printf (%p\n, p);
for (i = 0; i 1; i++)
foo (p);
return 0;
}
Time with zero arguments (i.e. argc 1) is the same as time with 1, 2 or 3
arguments on SandyBridge CPU. I guess there could be penalties on page
boundaries, etc., but I think hot caches is the usual operation on the
stack.
Jakub
Re: [patch] libgo - fix build errors and add ARM bits
On Mon, Nov 19, 2012 at 8:27 AM, Matthias Klose d...@ubuntu.com wrote: libgo-fix-arm.diff: Work around parse error of struct timex_ on ARM (both trunk and 4.7 branch). libgo-hardening.diff: Avoid compiler warnings in libgo with -D_FORTIFY_SOURCE=2, which let the build fail with -Werror. first chunk for the trunk and 4.7, second chunk for trunk only. libgo-mksysinfo.diff: Fix TIOCNOTTY and TIOCSCTTY definitions, afaicr needed for ARM as well. for trunk and 4.7. Thanks. I committed the libgo-hardening and libgo-mksysinfo patches to mainline and 4.7 branch. Can you tell me more about the libgo-fix-arm patch? The patch adds these lines to mksysinfo.sh: +# ARM +sed -i '/type _timex/s/INVALID-bit-field/int32/g;/type _timex/s,^// ,,' gen-sysinfo.go I don't understand why there would an INVALID-bit-field on ARM. This struct comes from the sys/timex.h, which as far as I can see should be the same on every glibc system. What does struct timex look like in your sys/timex.h file? What does the line look like in gen-sysinfo.go before the sed script above is run? Ian
Go patch committed: Reject nil == nil
This patch to the Go frontend rejects invalid comparisons in Go of nil
with nil. These comparisons have no type and no particular meaning, and
they are not permitted. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline.
Ian
diff -r 4d8871577da5 go/expressions.cc
--- a/go/expressions.cc Mon Dec 03 22:22:34 2012 -0800
+++ b/go/expressions.cc Mon Dec 03 22:38:58 2012 -0800
@@ -5610,6 +5610,11 @@
|| this-op_ == OPERATOR_GT
|| this-op_ == OPERATOR_GE)
{
+ if (left_type-is_nil_type() right_type-is_nil_type())
+ {
+ this-report_error(_(invalid comparison of nil with nil));
+ return;
+ }
if (!Type::are_assignable(left_type, right_type, NULL)
!Type::are_assignable(right_type, left_type, NULL))
{
Re: [patch] libgo - fix build errors and add ARM bits
Am 04.12.2012 07:26, schrieb Ian Lance Taylor:
On Mon, Nov 19, 2012 at 8:27 AM, Matthias Klose d...@ubuntu.com wrote:
libgo-fix-arm.diff: Work around parse error of struct timex_ on ARM (both
trunk
and 4.7 branch).
libgo-hardening.diff: Avoid compiler warnings in libgo with
-D_FORTIFY_SOURCE=2,
which let the build fail with -Werror. first chunk for the trunk and 4.7,
second
chunk for trunk only.
libgo-mksysinfo.diff: Fix TIOCNOTTY and TIOCSCTTY definitions, afaicr needed
for
ARM as well. for trunk and 4.7.
Thanks. I committed the libgo-hardening and libgo-mksysinfo patches
to mainline and 4.7 branch.
Can you tell me more about the libgo-fix-arm patch? The patch adds
these lines to mksysinfo.sh:
+# ARM
+sed -i '/type _timex/s/INVALID-bit-field/int32/g;/type _timex/s,^//
,,' gen-sysinfo.go
I don't understand why there would an INVALID-bit-field on ARM. This
struct comes from the sys/timex.h, which as far as I can see should
be the same on every glibc system.
What does struct timex look like in your sys/timex.h file? What
does the line look like in gen-sysinfo.go before the sed script above
is run?
defined in bits/timex.h
struct timex
{
unsigned int modes; /* mode selector */
__syscall_slong_t offset; /* time offset (usec) */
__syscall_slong_t freq; /* frequency offset (scaled ppm) */
__syscall_slong_t maxerror; /* maximum error (usec) */
__syscall_slong_t esterror; /* estimated error (usec) */
int status; /* clock command/status */
__syscall_slong_t constant; /* pll time constant */
__syscall_slong_t precision; /* clock precision (usec) (ro) */
__syscall_slong_t tolerance; /* clock frequency tolerance (ppm) (ro) */
struct timeval time; /* (read only) */
__syscall_slong_t tick; /* (modified) usecs between clock ticks */
__syscall_slong_t ppsfreq;/* pps frequency (scaled ppm) (ro) */
__syscall_slong_t jitter; /* pps jitter (us) (ro) */
int shift;/* interval duration (s) (shift) (ro) */
__syscall_slong_t stabil; /* pps stability (scaled ppm) (ro) */
__syscall_slong_t jitcnt; /* jitter limit exceeded (ro) */
__syscall_slong_t calcnt; /* calibration intervals (ro) */
__syscall_slong_t errcnt; /* calibration errors (ro) */
__syscall_slong_t stbcnt; /* stability limit exceeded (ro) */
int tai; /* TAI offset (ro) */
/* ??? */
int :32; int :32; int :32; int :32;
int :32; int :32; int :32; int :32;
int :32; int :32; int :32;
};
I'll have to re-run the build with out the patch, but this replaces just the
32bit bit fields with an int32.
Matthias
Re: [PATCh][Revisedx2] Fix PR55521 by switching libsanitizer from mach_overrideto mac interpose functions on darwin
r194120. I've tested it on Linux, but not on Mac. On Tue, Dec 4, 2012 at 6:44 AM, Alexander Potapenko gli...@google.com wrote: I've added MISSING_BLOCKS_SUPPORT to LLVM compiler-rt in r169206. The rest of your change looks good to me as well. On Mon, Dec 3, 2012 at 6:33 PM, Alexander Potapenko gli...@google.com wrote: Jack, Note that MAC_INTERPOSE_FUNCTIONS is always defined in interception.h to either 0 or 1. I'm going to keep #if MAC_INTERPOSE_FUNCTIONS (adding !defined(MISSING_BLOCKS_SUPPORT) where appropriate) in libsanitizer. On Mon, Dec 3, 2012 at 11:17 AM, Mike Stump mikest...@comcast.net wrote: On Dec 3, 2012, at 8:02 AM, Jack Howarth howa...@bromo.med.uc.edu wrote: The attached patch eliminates PR 55521/sanitizer by switching libasan on darwin from using mach_override to mac function interposition So, I'm thinking the sanitizer people will just review and approve it, even though it says darwin and is heavily darwin specific… It's ok by me. -- Alexander Potapenko Software Engineer Google Moscow -- Alexander Potapenko Software Engineer Google Moscow
Re: [patch] libgo - fix build errors and add ARM bits
On Mon, Dec 3, 2012 at 10:47 PM, Matthias Klose d...@ubuntu.com wrote:
Am 04.12.2012 07:26, schrieb Ian Lance Taylor:
On Mon, Nov 19, 2012 at 8:27 AM, Matthias Klose d...@ubuntu.com wrote:
libgo-fix-arm.diff: Work around parse error of struct timex_ on ARM (both
trunk
and 4.7 branch).
libgo-hardening.diff: Avoid compiler warnings in libgo with
-D_FORTIFY_SOURCE=2,
which let the build fail with -Werror. first chunk for the trunk and 4.7,
second
chunk for trunk only.
libgo-mksysinfo.diff: Fix TIOCNOTTY and TIOCSCTTY definitions, afaicr
needed for
ARM as well. for trunk and 4.7.
Thanks. I committed the libgo-hardening and libgo-mksysinfo patches
to mainline and 4.7 branch.
Can you tell me more about the libgo-fix-arm patch? The patch adds
these lines to mksysinfo.sh:
+# ARM
+sed -i '/type _timex/s/INVALID-bit-field/int32/g;/type _timex/s,^//
,,' gen-sysinfo.go
I don't understand why there would an INVALID-bit-field on ARM. This
struct comes from the sys/timex.h, which as far as I can see should
be the same on every glibc system.
What does struct timex look like in your sys/timex.h file? What
does the line look like in gen-sysinfo.go before the sed script above
is run?
defined in bits/timex.h
struct timex
{
unsigned int modes; /* mode selector */
__syscall_slong_t offset; /* time offset (usec) */
__syscall_slong_t freq; /* frequency offset (scaled ppm) */
__syscall_slong_t maxerror; /* maximum error (usec) */
__syscall_slong_t esterror; /* estimated error (usec) */
int status; /* clock command/status */
__syscall_slong_t constant; /* pll time constant */
__syscall_slong_t precision; /* clock precision (usec) (ro) */
__syscall_slong_t tolerance; /* clock frequency tolerance (ppm) (ro) */
struct timeval time; /* (read only) */
__syscall_slong_t tick; /* (modified) usecs between clock ticks */
__syscall_slong_t ppsfreq;/* pps frequency (scaled ppm) (ro) */
__syscall_slong_t jitter; /* pps jitter (us) (ro) */
int shift;/* interval duration (s) (shift) (ro) */
__syscall_slong_t stabil; /* pps stability (scaled ppm) (ro) */
__syscall_slong_t jitcnt; /* jitter limit exceeded (ro) */
__syscall_slong_t calcnt; /* calibration intervals (ro) */
__syscall_slong_t errcnt; /* calibration errors (ro) */
__syscall_slong_t stbcnt; /* stability limit exceeded (ro) */
int tai; /* TAI offset (ro) */
/* ??? */
int :32; int :32; int :32; int :32;
int :32; int :32; int :32; int :32;
int :32; int :32; int :32;
};
I'll have to re-run the build with out the patch, but this replaces just the
32bit bit fields with an int32.
Thanks. That's more or less what timex.h looks like on my system, but
I don't see the bitfields. GCC treats the :32 fields as int32, in
both 32-bit and 64-bit mode.
Ian
Emitting correct DWARF location descriptor for multi-reg frame pointer
As a follow-up to this discussion
(http://gcc.gnu.org/ml/gcc/2012-11/msg00307.html), I have a patch that
makes dwarf2out.c emit correct location information for a FP register spanning
multiple hard regs.
Before the patch, the DW_AT_location for a local variable on the AVR
target (whose FP spans registers R29 and R28) looks like this
4f DW_AT_location: 2 byte block: 8c 1 (DW_OP_breg28 (r28): 1)
After the patch, it looks like this
4f DW_AT_location: 11 byte block: 6c 93 1 6d 93 1 11 1 22 94 2
(DW_OP_reg28 (r28); DW_OP_piece: 1; DW_OP_reg29 (r29); DW_OP_piece: 1;
DW_OP_consts: 1; DW_OP_plus; DW_OP_deref_size: 2)
What do you guys think?
Regards
Senthil
diff --git gcc/dwarf2out.c gcc/dwarf2out.c
index f0256ae..53a4e56 100644
--- gcc/dwarf2out.c
+++ gcc/dwarf2out.c
@@ -10875,11 +10875,25 @@ based_loc_descr (rtx reg, HOST_WIDE_INT offset,
return new_loc_descr (DW_OP_fbreg, offset, 0);
}
- if (regno = 31)
-result = new_loc_descr ((enum dwarf_location_atom) (DW_OP_breg0 + regno),
- offset, 0);
+ rtx regs = targetm.dwarf_register_span (reg);
+
+ if (hard_regno_nregs[REGNO (reg)][GET_MODE (reg)] 1 || regs)
+{
+ result = multiple_reg_loc_descriptor (reg, regs, initialized);
+ add_loc_descr (result, new_loc_descr (DW_OP_consts, offset, 0));
+ add_loc_descr (result, new_loc_descr (DW_OP_plus, 0, 0));
+ add_loc_descr (result, new_loc_descr (DW_OP_deref_size,
+ GET_MODE_SIZE (GET_MODE (reg)),
+ 0));
+}
else
-result = new_loc_descr (DW_OP_bregx, regno, offset);
+{
+ if (regno = 31)
+ result = new_loc_descr ((enum dwarf_location_atom) (DW_OP_breg0 +
regno),
+ offset, 0);
+ else
+ result = new_loc_descr (DW_OP_bregx, regno, offset);
+}
if (initialized == VAR_INIT_STATUS_UNINITIALIZED)
add_loc_descr (result, new_loc_descr (DW_OP_GNU_uninit, 0, 0));
Re: [PATCH] asan unit tests from llvm lit-test
On Tue, Dec 04, 2012 at 11:22:40AM +0400, Kostya Serebryany wrote: Please note that tsan has 20+ more tests like this (projects/compiler-rt/lib/tsan/lit_tests) and asan will be getting more such tests too (mostly for the new features such as use-after-return, use-after-scope, global-init). If we do not find a way to make these tests clang-friendly and gcc-friendly at the same time, the maintenance burden for the gcc fork could be quite high. Well, the dejagnu comments could coexist with the FILECHECK comments, LLVM would ignore the dejagnu comments and GCC would ignore the FILECHECK comments. Doing some automatic translation of FILECHECK into dejagnu procedures is too complicated and too hackish, e.g. it would need to abstract from the multiple compiler + invocation: // RUN: %clangxx_asan -m64 -O0 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m64 -O1 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m64 -O2 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m64 -O3 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m32 -O0 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m32 -O1 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m32 -O2 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out // RUN: %clangxx_asan -m32 -O3 %s -o %t %t 21 | %symbolize %t.out // RUN: FileCheck %s %t.out FileCheck %s --check-prefix=CHECK-%os %t.out ah, this is a C++ test run at all torture options, etc. Plus as soon as the tests are enabled on more than i?86/x86_64, some of the tests will need to be limited to some particular targets, or skipped on some, tweaked options etc. You don't have thousands of tests, so doing it by hand and reviewing those changes sounds much better way to me. Jakub
Re: [patch] libgo - fix build errors and add ARM bits
Am 04.12.2012 08:03, schrieb Ian Lance Taylor:
On Mon, Dec 3, 2012 at 10:47 PM, Matthias Klose d...@ubuntu.com wrote:
Am 04.12.2012 07:26, schrieb Ian Lance Taylor:
On Mon, Nov 19, 2012 at 8:27 AM, Matthias Klose d...@ubuntu.com wrote:
libgo-fix-arm.diff: Work around parse error of struct timex_ on ARM (both
trunk
and 4.7 branch).
libgo-hardening.diff: Avoid compiler warnings in libgo with
-D_FORTIFY_SOURCE=2,
which let the build fail with -Werror. first chunk for the trunk and 4.7,
second
chunk for trunk only.
libgo-mksysinfo.diff: Fix TIOCNOTTY and TIOCSCTTY definitions, afaicr
needed for
ARM as well. for trunk and 4.7.
Thanks. I committed the libgo-hardening and libgo-mksysinfo patches
to mainline and 4.7 branch.
Can you tell me more about the libgo-fix-arm patch? The patch adds
these lines to mksysinfo.sh:
+# ARM
+sed -i '/type _timex/s/INVALID-bit-field/int32/g;/type _timex/s,^//
,,' gen-sysinfo.go
I don't understand why there would an INVALID-bit-field on ARM. This
struct comes from the sys/timex.h, which as far as I can see should
be the same on every glibc system.
What does struct timex look like in your sys/timex.h file? What
does the line look like in gen-sysinfo.go before the sed script above
is run?
defined in bits/timex.h
struct timex
{
unsigned int modes; /* mode selector */
__syscall_slong_t offset; /* time offset (usec) */
__syscall_slong_t freq; /* frequency offset (scaled ppm) */
__syscall_slong_t maxerror; /* maximum error (usec) */
__syscall_slong_t esterror; /* estimated error (usec) */
int status; /* clock command/status */
__syscall_slong_t constant; /* pll time constant */
__syscall_slong_t precision; /* clock precision (usec) (ro) */
__syscall_slong_t tolerance; /* clock frequency tolerance (ppm) (ro) */
struct timeval time; /* (read only) */
__syscall_slong_t tick; /* (modified) usecs between clock ticks */
__syscall_slong_t ppsfreq;/* pps frequency (scaled ppm) (ro) */
__syscall_slong_t jitter; /* pps jitter (us) (ro) */
int shift;/* interval duration (s) (shift) (ro) */
__syscall_slong_t stabil; /* pps stability (scaled ppm) (ro) */
__syscall_slong_t jitcnt; /* jitter limit exceeded (ro) */
__syscall_slong_t calcnt; /* calibration intervals (ro) */
__syscall_slong_t errcnt; /* calibration errors (ro) */
__syscall_slong_t stbcnt; /* stability limit exceeded (ro) */
int tai; /* TAI offset (ro) */
/* ??? */
int :32; int :32; int :32; int :32;
int :32; int :32; int :32; int :32;
int :32; int :32; int :32;
};
I'll have to re-run the build with out the patch, but this replaces just the
32bit bit fields with an int32.
Thanks. That's more or less what timex.h looks like on my system, but
I don't see the bitfields. GCC treats the :32 fields as int32, in
both 32-bit and 64-bit mode.
sorry, this was fixed with the patch for PR52557.
Re: [patch] libgo - fix build errors and add ARM bits
On Mon, Dec 3, 2012 at 11:38 PM, Matthias Klose d...@ubuntu.com wrote:
Am 04.12.2012 08:03, schrieb Ian Lance Taylor:
On Mon, Dec 3, 2012 at 10:47 PM, Matthias Klose d...@ubuntu.com wrote:
Am 04.12.2012 07:26, schrieb Ian Lance Taylor:
On Mon, Nov 19, 2012 at 8:27 AM, Matthias Klose d...@ubuntu.com wrote:
libgo-fix-arm.diff: Work around parse error of struct timex_ on ARM (both
trunk
and 4.7 branch).
libgo-hardening.diff: Avoid compiler warnings in libgo with
-D_FORTIFY_SOURCE=2,
which let the build fail with -Werror. first chunk for the trunk and 4.7,
second
chunk for trunk only.
libgo-mksysinfo.diff: Fix TIOCNOTTY and TIOCSCTTY definitions, afaicr
needed for
ARM as well. for trunk and 4.7.
Thanks. I committed the libgo-hardening and libgo-mksysinfo patches
to mainline and 4.7 branch.
Can you tell me more about the libgo-fix-arm patch? The patch adds
these lines to mksysinfo.sh:
+# ARM
+sed -i '/type _timex/s/INVALID-bit-field/int32/g;/type _timex/s,^//
,,' gen-sysinfo.go
I don't understand why there would an INVALID-bit-field on ARM. This
struct comes from the sys/timex.h, which as far as I can see should
be the same on every glibc system.
What does struct timex look like in your sys/timex.h file? What
does the line look like in gen-sysinfo.go before the sed script above
is run?
defined in bits/timex.h
struct timex
{
unsigned int modes; /* mode selector */
__syscall_slong_t offset; /* time offset (usec) */
__syscall_slong_t freq; /* frequency offset (scaled ppm) */
__syscall_slong_t maxerror; /* maximum error (usec) */
__syscall_slong_t esterror; /* estimated error (usec) */
int status; /* clock command/status */
__syscall_slong_t constant; /* pll time constant */
__syscall_slong_t precision; /* clock precision (usec) (ro) */
__syscall_slong_t tolerance; /* clock frequency tolerance (ppm) (ro) */
struct timeval time; /* (read only) */
__syscall_slong_t tick; /* (modified) usecs between clock ticks */
__syscall_slong_t ppsfreq;/* pps frequency (scaled ppm) (ro) */
__syscall_slong_t jitter; /* pps jitter (us) (ro) */
int shift;/* interval duration (s) (shift) (ro) */
__syscall_slong_t stabil; /* pps stability (scaled ppm) (ro) */
__syscall_slong_t jitcnt; /* jitter limit exceeded (ro) */
__syscall_slong_t calcnt; /* calibration intervals (ro) */
__syscall_slong_t errcnt; /* calibration errors (ro) */
__syscall_slong_t stbcnt; /* stability limit exceeded (ro) */
int tai; /* TAI offset (ro) */
/* ??? */
int :32; int :32; int :32; int :32;
int :32; int :32; int :32; int :32;
int :32; int :32; int :32;
};
I'll have to re-run the build with out the patch, but this replaces just the
32bit bit fields with an int32.
Thanks. That's more or less what timex.h looks like on my system, but
I don't see the bitfields. GCC treats the :32 fields as int32, in
both 32-bit and 64-bit mode.
sorry, this was fixed with the patch for PR52557.
Oh yeah. Thanks for looking into it.
Ian
Re: [Patch, Fortran] PR55475 - fix invalid reads with show_locus
I have now committed that patch as obvious, Rev. 194076. http://gcc.gnu.org/ml/fortran/2012-11/msg00084.html Tobias Tobias Burnus wrote: As found with -fsanitize=address by HJ, but it also shows up with valgrind. The fix for the PR is the change in scanner.c; I think the patch is rather obvious. The change in error.c is due to: if (c1 == c2) c2 += 1; which could lead to an out-of-bounds condition is c1 is already at the last character - then one exceeds the bound for c2. Build and tested on x86-64-linux with no new failures.* OK for the trunk? Tobias * I get: FAIL for gfortran.dg/lto/pr45586, gfortran.dg/realloc_on_assign_5.f03 and gfortran.dg/reassoc_4.f and XPASS for gfortran.dg/do_1.f90.
[PATCH] Fix PR52872
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
This fixes the tests for exported symbols and -rdynamic on systems with
exe extensions.
Kai, feel free to commit, if ok.
Cheers
Rainer
Patch against trunk:
Index: gcc/configure.ac
===
- --- gcc/configure.ac(revision 194074)
+++ gcc/configure.ac(working copy)
@@ -5156,14 +5156,14 @@ if test x$enable_plugin = xyes; then
if test x$export_sym_check != x; then
echo int main() {return 0;} int foobar() {return 0;} conftest.c
${CC} ${CFLAGS} ${LDFLAGS} conftest.c -o conftest /dev/null 21
- -if $export_sym_check conftest | grep foobar /dev/null; then
+if $export_sym_check conftest${exeext} | grep foobar /dev/null; then
: # No need to use a flag
AC_MSG_RESULT([yes])
else
AC_MSG_RESULT([yes])
AC_MSG_CHECKING([for -rdynamic])
${CC} ${CFLAGS} ${LDFLAGS} -rdynamic conftest.c -o conftest /dev/null
21
- - if $export_sym_check conftest | grep foobar /dev/null; then
+ if $export_sym_check conftest${exeext} | grep foobar /dev/null; then
plugin_rdynamic=yes
pluginlibs=-rdynamic
else
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with undefined - http://www.enigmail.net/
iQEcBAEBAgAGBQJQvGrRAAoJEB3HOsWs+KJb9zsH/izdk3gQ2yp+CG4p74UzaFh/
4UsKwqOFG487Xr169q6HVovfAy5/SKZloxkbgywYyqJoTq6L3I2utx8Q2bYTv6Hx
9j5UJoiSoQDjlz1MW7nQ/ZgzX6Lbo7KXuH9p6CVKFOqisN+260qujmUnivhMJmKw
BE4XpGcumK9SnUmnWwvECuENnhZTmLYl+K1AFI83wxO2/dmZdqlwwsUG1opHz7P7
8y6DwT7G9JQpkwNU8dYEgv+n6JPK7GWXqu2Nvcn8J52vtN/DC09nUWpRtIj+myUb
8nv8rTk78EvCd4hzDp23oDHfw+6IUD6R9XIVRPVgPwpCQYyjCJ7OOD+YtAr96PY=
=LuVL
-END PGP SIGNATURE-
Re: [PATCH] asan_test.cc from llvm
On Mon, Dec 03, 2012 at 11:06:48AM +0400, Konstantin Serebryany wrote:
This patch copies the asan tests almost, but not quite, verbatim from
upstream.
Since the patch is not in attachment (and gmail messes up with inlined
patches) I can't see the exact changes.
Sending patches inline rather than as attachments is the preferred way for
gcc-patches, you can always grab it from the gcc-patches ml archive:
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02557.html
(then click on Raw text).
I'm attaching the diff for asan_test.cc from llvm anyway.
I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
because I'd rather fix the test than disable it.
The test isn't disabled, just by default limited to 30 threads instead of
1000, because that really will ruin testing for everybody with ulimit -u
in 1024-ish range. Even 500 threads would be undesirable for that.
Can we commit the tests 100% verbatim, and then fix them as separate commits
(preferably, by fixing the tests upstream and doing the merge with
libsanitizer/merge.sh)?
I'd prefer delay committing the patch over causing random testsuite failures
elsewhere.
Jakub
--- /usr/src/llvm/projects/compiler-rt/lib/asan/tests/asan_test.cc
2012-11-30 14:23:48.525229790 +0100
+++ asan_test.cc2012-12-03 10:12:14.503366704 +0100
@@ -1,7 +1,5 @@
//===-- asan_test.cc
--===//
//
-// The LLVM Compiler Infrastructure
-//
// This file is distributed under the University of Illinois Open Source
// License. See LICENSE.TXT for details.
//
@@ -481,10 +479,16 @@ void *ManyThreadsWorker(void *a) {
}
TEST(AddressSanitizer, ManyThreadsTest) {
- const size_t kNumThreads = SANITIZER_WORDSIZE == 32 ? 30 : 1000;
- pthread_t t[kNumThreads];
+#ifdef ASAN_AVOID_EXPENSIVE_TESTS
+ const size_t kMaxThreads = 30;
+#else
+ const size_t kMaxThreads = SANITIZER_WORDSIZE == 32 ? 30 : 1000;
+#endif
+ pthread_t t[kMaxThreads];
+ size_t kNumThreads = kMaxThreads;
for (size_t i = 0; i kNumThreads; i++) {
-pthread_create(t[i], 0, (void* (*)(void *x))ManyThreadsWorker, (void*)i);
+if (pthread_create(t[i], 0, (void* (*)(void *x))ManyThreadsWorker,
(void*)i))
+ kNumThreads = i;
}
for (size_t i = 0; i kNumThreads; i++) {
pthread_join(t[i], 0);
Re: [PATCH] asan_test.cc from llvm
On Mon, Dec 3, 2012 at 1:18 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 03, 2012 at 11:06:48AM +0400, Konstantin Serebryany wrote:
This patch copies the asan tests almost, but not quite, verbatim from
upstream.
Since the patch is not in attachment (and gmail messes up with inlined
patches) I can't see the exact changes.
Sending patches inline rather than as attachments is the preferred way for
gcc-patches, you can always grab it from the gcc-patches ml archive:
http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02557.html
(then click on Raw text).
Got it, thanks.
I'm attaching the diff for asan_test.cc from llvm anyway.
I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
because I'd rather fix the test than disable it.
The test isn't disabled, just by default limited to 30 threads instead of
1000, because that really will ruin testing for everybody with ulimit -u
in 1024-ish range. Even 500 threads would be undesirable for that.
Which is the same as disabling it.
Unfortunately, we don't have a good automated way to test asan performance,
so this test is guarding us from performance degradation in the asan's
pthread wrappers.
If there is a bug there, we may not notice it on 30 threads, but may
(with a high probability)
on 1000 threads.
Anyway, does http://llvm.org/viewvc/llvm-project?rev=169118view=rev
solve the problem?
It adds ASAN_AVOID_EXPENSIVE_TESTS and checks the results of all
pthread_{create,join} calls in tests.
--kcc
Can we commit the tests 100% verbatim, and then fix them as separate commits
(preferably, by fixing the tests upstream and doing the merge with
libsanitizer/merge.sh)?
I'd prefer delay committing the patch over causing random testsuite failures
elsewhere.
Jakub
[patch] Small tweak to -Wuninitialized
Hi, if you compile the attached testcase on x86 with -O2 -Wuninitialized, you get: strip.adb: In function 'Strip': strip.adb:4:4: warning: 'Last' may be used uninitialized in this function [- Wuninitialized] strip.adb:26:44: warning: 'First' may be used uninitialized in this function [-Wmaybe-uninitialized] The same warning is apparently associated with 2 different options so, if you want to disable maybe warnings, -O2 -Wuninitialized -Wno-maybe-uninitialized only disables the second one: strip.adb: In function 'Strip': strip.adb:4:4: warning: 'Last' may be used uninitialized in this function [- Wuninitialized] Of course the story under the hood is a bit different, but this doesn't really matter to the user I think, so I propose using the -Wmaybe-uninitialized tag in both cases. Tested on x86-64-suse-linux, OK for the mainline? 2012-12-03 Eric Botcazou ebotca...@adacore.com * tree-ssa.c (warn_uninitialized_var): Use OPT_Wmaybe_uninitialized tag in the non-always executed case. -- Eric BotcazouIndex: tree-ssa.c === --- tree-ssa.c (revision 194049) +++ tree-ssa.c (working copy) @@ -1664,7 +1664,7 @@ warn_uninitialized_vars (bool warn_possi %qD is used uninitialized in this function, stmt); else if (warn_possibly_uninitialized) - warn_uninit (OPT_Wuninitialized, use, + warn_uninit (OPT_Wmaybe_uninitialized, use, SSA_NAME_VAR (use), SSA_NAME_VAR (use), %qD may be used uninitialized in this function, stmt); @@ -1696,13 +1696,13 @@ warn_uninitialized_vars (bool warn_possi continue; if (always_executed) - warn_uninit (OPT_Wuninitialized, use, gimple_assign_rhs1 (stmt), - base, + warn_uninit (OPT_Wuninitialized, use, + gimple_assign_rhs1 (stmt), base, %qE is used uninitialized in this function, stmt); else if (warn_possibly_uninitialized) - warn_uninit (OPT_Wuninitialized, use, gimple_assign_rhs1 (stmt), - base, + warn_uninit (OPT_Wmaybe_uninitialized, use, + gimple_assign_rhs1 (stmt), base, %qE may be used uninitialized in this function, stmt); } function Strip (Str : in String) return String is First : Natural; Last : Natural; Blank_Str : constant String (Str'Range) := (others = ' '); begin if Str = or Str = Blank_Str then return ; end if; for I in Str'Range loop if Str (I) /= ' ' and Str (I) /= Ascii.Ht then First := I; exit; end if; end loop; for I in reverse Str'Range loop if Str (I) /= ' ' and Str (I) /= Ascii.Ht then Last := I; exit; end if; end loop; declare Ret_Str : constant String (1 .. Last - First + 1) := Str (First .. Last); begin return Ret_Str; end; end Strip;
Re: [PATCH] Fix PR35634
On Sat, 1 Dec 2012, David Edelsohn wrote:
Richard,
The testcases assume default signed char and fail on systems with
different semantics. I believe that both testcases need to declare c
as signed char to consistently test the desired behavior, right?
Fixed as follows.
Richard.
2012-12-03 Richard Biener rguent...@suse.de
* gcc.dg/torture/pr35634.c: Use signed char.
* g++.dg/torture/pr35634.C: Likewise.
Index: gcc/testsuite/gcc.dg/torture/pr35634.c
===
--- gcc/testsuite/gcc.dg/torture/pr35634.c (revision 194077)
+++ gcc/testsuite/gcc.dg/torture/pr35634.c (working copy)
@@ -14,6 +14,6 @@ void foo (int i)
int main ()
{
-char c;
+signed char c;
for (c = 0; ; c++) foo (c);
}
Index: gcc/testsuite/g++.dg/torture/pr35634.C
===
--- gcc/testsuite/g++.dg/torture/pr35634.C (revision 194077)
+++ gcc/testsuite/g++.dg/torture/pr35634.C (working copy)
@@ -14,6 +14,6 @@ void foo (int i)
int main ()
{
-char c;
+signed char c;
for (c = 0; ; c++) foo (c);
}
Re: [PATCH] asan unit tests from llvm lit-test
Hi!
Mike, CCing you especially on the proposed lib/gcc-dg.exp dg-env-var
changes and I have one question about cleanup of files (file delete
vs. remote_file target (or is that host or build) delete).
But of course if you could eyeball the rest and comment, I'd be even happier.
On Fri, Nov 30, 2012 at 12:35:35PM -0800, Wei Mi wrote:
Thanks for the comments! Here is the second version patch. Please see
if it is ok.
(-Wno-attributes is kept or else we will get a warning because of
__attribute__((always_inline))).
--- gcc/testsuite/gcc.dg/asan/asan.exp(revision 194002)
+++ gcc/testsuite/gcc.dg/asan/asan.exp(working copy)
@@ -30,6 +30,10 @@ if ![check_effective_target_faddress_san
dg-init
asan_init
+# Set default torture options
+set default_asan_torture_options [list { -O0 } { -O1 } { -O2 } { -O3 }]
+set-torture-options $default_asan_torture_options
Why this? What is undesirable on the default torture options?
Do those tests fail with lto or similar?
--- gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/deep-stack-uaf-1.C (revision 0)
@@ -0,0 +1,33 @@
+// Check that we can store lots of stack frames if asked to.
+
+// { dg-do run }
+// { dg-env-var ASAN_OPTIONS malloc_context_size=120:redzone=512 }
+// { dg-shouldfail asan }
Can you please replace the two spaces after // with just one?
Dejagnu directives are often quite long, and thus it is IMHO better to make
the lines longer than necessary.
For this test, don't you need
// { dg-options -fno-optimize-sibling-calls }
and __attribute__((noinline)) on the free method? Otherwise I'd expect
that either at least at -O3 it could be all inlined, or if not inlined, then
at least tail call optimized (and thus not showing up in the backtrace
either).
+#include stdlib.h
+#include stdio.h
+
+template int depth
+struct DeepFree {
+ static void free(char *x) {
+DeepFreedepth - 1::free(x);
+ }
+};
+
+template
+struct DeepFree0 {
+ static void free(char *x) {
+::free(x);
+ }
+};
+
+int main() {
+ char *x = new char[10];
+ // deep_free(x);
+ DeepFree200::free(x);
+ return x[5];
+}
+
+// { dg-output ERROR: AddressSanitizer:? heap-use-after-free on
address.*(\n|\r\n|\r) }
+// { dg-output #37 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*36|\[(\]).*(\n|\r\n|\r) }
+// { dg-output #99 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*98|\[(\]).*(\n|\r\n|\r) }
+// { dg-output #116 0x\[0-9a-f\]+ (in
\[^\n\r]*DeepFree\[^\n\r]*115|\[(\])\[^\n\r]*(\n|\r\n|\r) }
--- gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/deep-tail-call-1.C (revision 0)
@@ -0,0 +1,20 @@
+// { dg-do run }
+// { dg-options -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer
-fno-optimize-sibling-calls }
-mno-omit-leaf-frame-pointer is i?86/x86_64 options, so you need to leave it
from dg-options and add
// { dg-additional-options -mno-omit-leaf-frame-pointer { target { i?86-*-*
x86_64-*-* } } }
--- gcc/testsuite/g++.dg/asan/asan.exp(revision 194002)
+++ gcc/testsuite/g++.dg/asan/asan.exp(working copy)
@@ -28,9 +28,15 @@ if ![check_effective_target_faddress_san
dg-init
asan_init
+# Set default torture options
+set default_asan_torture_options [list { -O0 } { -O1 } { -O2 } { -O3 }]
+set-torture-options $default_asan_torture_options
Again, like I asked earlier.
+
# Main loop.
gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C
$srcdir/c-c++-common/asan/*.c]]
+source $srcdir/$subdir/special/special.exp
Won't this cause double testing of the special tests? AFAIK dejagnu is
looking recursively for all *.exp files, so once you'd source it when
running asan.exp and again when dejagnu finds special.exp on its own.
If that is the case, then you shouldn't source it here, and rename
special.exp to say asan-special.exp, so that one can test all asan
tests with just RUNTESTFLAGS=--target_board=unix\{-m32,-m64\} asan.exp
asan-special.exp
but also make check will DTRT. Or perhaps name it also asan.exp, see if
RUNTESTFLAGS=--target_board=unix\{-m32,-m64\} asan.exp
then will DTRT and also make check?
--- gcc/testsuite/g++.dg/asan/interception-test-1.C (revision 0)
+++ gcc/testsuite/g++.dg/asan/interception-test-1.C (revision 0)
@@ -0,0 +1,22 @@
+// ASan interceptor can be accessed with __interceptor_ prefix.
+
+// { dg-do run }
+// { dg-shouldfail asan }
+
+#include stdlib.h
+#include stdio.h
+
+extern C long __interceptor_strtol(const char *nptr, char **endptr, int
base);
+extern C long strtol(const char *nptr, char **endptr, int base) {
+ fprintf(stderr, my_strtol_interceptor\n);
+ return __interceptor_strtol(nptr, endptr, base);
+}
+
+int main() {
+ char *x = (char*)malloc(10 * sizeof(char));
Ugh, why the * sizeof(char)? That is completely pointless...
--- gcc/testsuite/g++.dg/asan/large-func-test-1.C
Re: [patch] Rework RTL CFG graph dumping to dump DOT format
On Sat, Dec 1, 2012 at 6:40 PM, Steven Bosscher stevenb@gmail.com wrote:
On Sat, Dec 1, 2012 at 2:23 PM, Steven Bosscher wrote:
On Mon, Nov 26, 2012 at 4:46 PM, Richard Biener wrote:
Btw, I of course have my own CFG dumper (producing graphviz input)
in my local tree - attached for reference (I'm simply using it from
gdb sessions).
Here's my version of it. I still have to fix some minor fall-out of
not flushing the pretty-printers all over the (inappropriate) place,
but the graph dumps seem to work nicely so far. Perhaps you can try it
out and see if this is to your liking? :-)
Bootstrappedtested on {powerpc64,x86_64}-unknown-linux-gnu. As I
said: Still fixing some minor tree dump related fall-out.
I only need this fix on top of the patch:
diff -u tree-pretty-print.c tree-pretty-print.c
--- tree-pretty-print.c (working copy)
+++ tree-pretty-print.c (working copy)
@@ -161,6 +161,7 @@
{
maybe_init_pretty_print (file);
dump_generic_node (buffer, t, 0, flags, false);
+ pp_flush (buffer);
}
/* Dump the name of a _DECL node and its DECL_UID if TDF_UID is set
Bootstrappedtested on powerpc64-unknown-linux-gnu. OK for trunk?
Nice.
Ok!
Thanks,
Richard.
Ciao!
Steven
Re: [PATCH] asan_test.cc from llvm
On Mon, Dec 03, 2012 at 01:51:50PM +0400, Konstantin Serebryany wrote:
I'm attaching the diff for asan_test.cc from llvm anyway.
I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
because I'd rather fix the test than disable it.
The test isn't disabled, just by default limited to 30 threads instead of
1000, because that really will ruin testing for everybody with ulimit -u
in 1024-ish range. Even 500 threads would be undesirable for that.
Which is the same as disabling it.
Unfortunately, we don't have a good automated way to test asan performance,
so this test is guarding us from performance degradation in the asan's
pthread wrappers.
I understand that, that is why the test by default, when not run as part of
dejagnu, or even in dejagnu when requested expensive tests, runs 1000
threads instead of 30.
Anyway, does http://llvm.org/viewvc/llvm-project?rev=169118view=rev
solve the problem?
It adds ASAN_AVOID_EXPENSIVE_TESTS and checks the results of all
pthread_{create,join} calls in tests.
Yes, thanks. So, is the patch ok to commit to GCC with the imported tests
remerged
from upstream (or do I need to repost the patch for that)?
Jakub
Re: [patch] Small tweak to -Wuninitialized
On Mon, Dec 3, 2012 at 10:51 AM, Eric Botcazou ebotca...@adacore.com wrote: Hi, if you compile the attached testcase on x86 with -O2 -Wuninitialized, you get: strip.adb: In function 'Strip': strip.adb:4:4: warning: 'Last' may be used uninitialized in this function [- Wuninitialized] strip.adb:26:44: warning: 'First' may be used uninitialized in this function [-Wmaybe-uninitialized] The same warning is apparently associated with 2 different options so, if you want to disable maybe warnings, -O2 -Wuninitialized -Wno-maybe-uninitialized only disables the second one: strip.adb: In function 'Strip': strip.adb:4:4: warning: 'Last' may be used uninitialized in this function [- Wuninitialized] Of course the story under the hood is a bit different, but this doesn't really matter to the user I think, so I propose using the -Wmaybe-uninitialized tag in both cases. Tested on x86-64-suse-linux, OK for the mainline? Ok. Thanks, Richard. 2012-12-03 Eric Botcazou ebotca...@adacore.com * tree-ssa.c (warn_uninitialized_var): Use OPT_Wmaybe_uninitialized tag in the non-always executed case. -- Eric Botcazou
Re: [PATCH] asan_test.cc from llvm
On Mon, Dec 3, 2012 at 3:05 PM, Jakub Jelinek ja...@redhat.com wrote:
On Mon, Dec 03, 2012 at 01:51:50PM +0400, Konstantin Serebryany wrote:
I'm attaching the diff for asan_test.cc from llvm anyway.
I see #ifdef ASAN_AVOID_EXPENSIVE_TESTS, which I don't really like
because I'd rather fix the test than disable it.
The test isn't disabled, just by default limited to 30 threads instead of
1000, because that really will ruin testing for everybody with ulimit -u
in 1024-ish range. Even 500 threads would be undesirable for that.
Which is the same as disabling it.
Unfortunately, we don't have a good automated way to test asan performance,
so this test is guarding us from performance degradation in the asan's
pthread wrappers.
I understand that, that is why the test by default, when not run as part of
dejagnu, or even in dejagnu when requested expensive tests, runs 1000
threads instead of 30.
Anyway, does http://llvm.org/viewvc/llvm-project?rev=169118view=rev
solve the problem?
It adds ASAN_AVOID_EXPENSIVE_TESTS and checks the results of all
pthread_{create,join} calls in tests.
Yes, thanks. So, is the patch ok to commit to GCC with the imported tests
remerged
from upstream
Yes, ok to commit. Thanks!
--kcc
(or do I need to repost the patch for that)?
Jakub
Re: [PATCH] Add --with-build-config=bootstrap-asan support
Hi! On Thu, Nov 29, 2012 at 12:20:39PM -0800, H.J. Lu wrote: On Thu, Nov 29, 2012 at 11:06 AM, Jakub Jelinek ja...@redhat.com wrote: On Thu, Nov 29, 2012 at 07:24:38PM +0100, Paolo Carlini wrote: On 11/29/2012 06:36 PM, Tobias Burnus wrote: H.J. Lu wrote: This patch adds --with-build-config=bootstrap-asan support. Tested on Linux/x86-64. OK to install? I think that patch has broken bootstrap for me. If I do a normal bootstrap, Stage1 fails with: libtool: compile: unrecognized option `-D_GNU_SOURCE' libtool: compile: Try `libtool --help' for more information. make[4]: *** [interception_linux.lo] Error 1 make[4]: Leaving directory `/home/burnus/gcc/build/x86_64-unknown-linux-gnu/libsanitizer/interception' Likewise here. Would it be possible to revert the offending commit, in the meanwhile? Yes, H.J., please revert the patch, I thought you have tested it alone without any further patches. For the -I patch, I really would prefer if libsanitizer just had a dependency on libstdc++ at toplevel (configure-target-sanitizer depending on all-target-libstdc++-v3), then you can (and similarly for host variants if we need host sanitizer at all). Then you should be able to use scripts/testsuite_flags --build-includes just fine. I opened: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55533 to explain why scripts/testsuite_flags doesn't work when bootstrapping libsanitizer. CCing Paolo and Alex as build maintainers on this, using the script is just my preference and not sure whether it is feasible or not, though IMHO if there is a dependency on libstdc++-v3 being built before libsanitizer is configured (or perhaps just configure when it has been configured and build when it has been built), I don't see why it couldn't be used in theory. I'll defer this to them. Jakub
Re: Reduce complette unrolling peeling limits
here is updated patch. It should get the bounds safe enough to not have effect on codegen of complette unrolling. There is IMO no way to cut the walk of loop body w/o affecting codegen in unrolling for size mode. The condition on unroling to happen is unrolled_size * 2 / 3 original_size The patch makes the function walking body to stop after minimal number of duplicated insns is large (PARAM_MAX_COMPLETELY_PEELED_INSNS). The formula above allows unlimited duplication when loop body is large enough. This is more a bug than feature, so I think it is safe to alter it. Bootstrapped/regtested x86_64-linux, OK? Honza * tree-ssa-loop-ivcanon.c (tree_estimate_loop_size): Add UPPER_BOUND parameter. (try_unroll_loop_completely) Update. The patch hasn't been installed, has it? The test still takes 20s to compile at -O3 on a fast x86-64 box, so you can imagine what this yields on slower machines (and that's before the x4 because of the various dg-torture options). -- Eric Botcazou
Re: [IA-64] Fix dynamic allocation in leaf functions
2012-11-19 Eric Botcazou ebotca...@adacore.com * config/ia64/ia64.c (ia64_compute_frame_size): Allocate the scratch area if the function allocates dynamic stack space. (ia64_initial_elimination_offset): Adjust offsets to above change. Ping: http://gcc.gnu.org/ml/gcc-patches/2012-11/msg01617.html Thanks in advance. -- Eric Botcazou
[PATCH] Fix PR55570
We segfaulted on attached testcase, because we were accessing
-typed.type field via TREE_TYPE, but IDENTIFIER_NODEs don't contain
-typed element at all. Fixed by switching the expressions in
a condition, so we always first check that we're operating on
INTEGER_CST.
Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.7 branch?
2012-12-03 Marek Polacek pola...@redhat.com
PR c/55570
* c-common.c (check_user_alignment):
* gcc.dg/pr55570.c: New test.
--- gcc/c-family/c-common.c.mp 2012-12-03 13:07:58.233832299 +0100
+++ gcc/c-family/c-common.c 2012-12-03 13:08:55.395259841 +0100
@@ -7261,8 +7261,8 @@ check_user_alignment (const_tree align,
{
int i;
- if (!INTEGRAL_TYPE_P (TREE_TYPE (align))
- || TREE_CODE (align) != INTEGER_CST)
+ if (TREE_CODE (align) != INTEGER_CST
+ || !INTEGRAL_TYPE_P (TREE_TYPE (align)))
{
error (requested alignment is not an integer constant);
return -1;
--- gcc/testsuite/gcc.dg/pr55570.c.mp 2012-12-03 13:06:02.298558986 +0100
+++ gcc/testsuite/gcc.dg/pr55570.c 2012-12-03 13:05:39.200504521 +0100
@@ -0,0 +1,4 @@
+/* PR c/55570 */
+/* { dg-do compile } */
+
+char array[16] __attribute__((aligned (SOME_NOT_DEFINED_MACRO))); /* {
dg-error requested alignment is not an integer constant } */
Marek
Re: [PATCH] Fix PR55570
On Mon, Dec 03, 2012 at 01:36:28PM +0100, Marek Polacek wrote: We segfaulted on attached testcase, because we were accessing -typed.type field via TREE_TYPE, but IDENTIFIER_NODEs don't contain -typed element at all. Fixed by switching the expressions in a condition, so we always first check that we're operating on INTEGER_CST. Regtested/bootstrapped on x86_64-linux, ok for trunk and 4.7 branch? 2012-12-03 Marek Polacek pola...@redhat.com PR c/55570 * c-common.c (check_user_alignment): Missing descrption of the change above, I'd say Swap order of tests, check TREE_CODE first. or similar. Ok with that change. Jakub
Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack
Jason Merrill ja...@redhat.com writes:
It seems like your new code is a generalization of the old code for
handling substitution of a pack for itself (arg_from_parm_pack and
such) and the code for handling other packs with a single pack
expansion argument, and should replace those rather than adding on.
I guess that can of worms is now open. :-)
I tried to think about how to do this properly. What I came up with
in the patch below is to properly organize tsubst_pack_expansion in
two parts that are already screaming to emerge, IMHO. The first part
maps each parameter pack with its matching argument pack, and does
only that. The second part walks that map and builds the list of Ei
where each Ei is the result of substituting elements at index I in the
argument packs into the pattern of the expansion passed to
tsubst_pack_expansion.
It's only the latter part knows how to deal with special cases like:
- some parameter packs having no matching argument pack
- parameter packs having matching argument packs element that are
expansions
- etc
Is that what you had in mind?
The solution that if at a certain index all the packs have expansion
arguments then the substitution produces a pack expansion seems right
to me, but if one pack has an expansion and another pack has a normal
argument, we can't do the substitution and need to fall back on the
PACK_EXPANSION_EXTRA_ARGS mechanism.
Right, this is hopefully what this updated patch implements.
+set_arg_pack_select_index_for_pack_expansion (tree aps,
+ int i,
+ tree arg_pack)
+{
+ if (any_non_real_argument_pack_element_p (arg_pack))
I don't think we care if *any* element is an expansion (and please
talk about expansions rather than non-real elements). What we care
about is whether the i'th element is an expansion. And we need to
compare all the pack elements, so I think this needs to be handled in
the main function rather than encapsulated here.
Done.
+ TREE_VEC_ELT (args_vec, i) =
+ TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), i);
Aren't the LHS and RHS the same location here?
Yes, this is hopefully dropped now.
Bootstrapped and tested against trunk on x86_64-unknown-linux-gnu.
gcc/cp/
* pt.c (argument_pack_element_is_expansion_p)
(arg_pack_select_for_pack_expansion)
(set_arg_pack_select_index_for_pack_expansion, scan_parm_packs)
(gen_elem_of_pack_expansion_instantiation): New static functions.
(has_bare_parameter_packs): Factorized out of ...
(check_for_bare_parameter_packs): ... here.
(tsubst_pack_expansion): Now this function is clearly organized in
two parts: a part that maps each parameter pack with its matching
argument pack, and a part that walks that map to build the result
of the substituting each element of the argument packs into the
parameter packs. Use gen_elem_of_pack_expansion_instantiation for
the latter part.
gcc/testsuite/
* g++.dg/cpp0x/variadic139.C: New test.
* g++.dg/cpp0x/variadic140.C: Likewise.
* g++.dg/cpp0x/variadic141.C: Likewise.
---
gcc/cp/pt.c | 525 ++-
gcc/testsuite/g++.dg/cpp0x/variadic139.C | 14 +
gcc/testsuite/g++.dg/cpp0x/variadic140.C | 22 ++
gcc/testsuite/g++.dg/cpp0x/variadic141.C | 22 ++
4 files changed, 441 insertions(+), 142 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..b8f4294 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3308,6 +3308,29 @@ make_pack_expansion (tree arg)
return result;
}
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+ Return the TREE_LIST of unexpanded parameter packs otherwise. */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+ tree parameter_packs = NULL_TREE;
+ struct find_parameter_pack_data ppd;
+
+ if (!processing_template_decl || !t || t == error_mark_node)
+return NULL_TREE;
+
+ if (TREE_CODE (t) == TYPE_DECL)
+t = TREE_TYPE (t);
+
+ ppd.parameter_packs = parameter_packs;
+ ppd.visited = pointer_set_create ();
+ cp_walk_tree (t, find_parameter_packs_r, ppd, ppd.visited);
+ pointer_set_destroy (ppd.visited);
+
+ return parameter_packs;
+}
+
/* Checks T for any bare parameter packs, which have not yet been
expanded, and issues an error if any are found. This operation can
only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3348,7 @@ make_pack_expansion (tree arg)
bool
check_for_bare_parameter_packs (tree t)
{
- tree parameter_packs = NULL_TREE;
- struct find_parameter_pack_data ppd;
-
- if (!processing_template_decl || !t || t == error_mark_node)
-return false;
-
- if
Re: [cxx-conversion] graphite-related hash tables
On Sun, Dec 2, 2012 at 4:58 AM, Tobias Grosser tob...@grosser.es wrote: Assuming the test suite passes and this is just a no-behavior change intended commit, I am fine with this. No idea, who needs to sign off branch commits, though. The branch is under the usual maintainer rules (it's a regular development branch). If you think the patch is OK, then it certainly is. Diego.
Re: [asan] Another ICE fix
Jakub Jelinek ja...@redhat.com writes: Fixed thusly, ok for trunk? 2012-11-27 Jakub Jelinek ja...@redhat.com * asan.c (instrument_assignment): Instrument lhs only for gimple_store_p and rhs1 only for gimple_assign_load_p. This is OK, thanks. And sorry for the delay. -- Dodji
Re: [C++ Patch] PR 54170
OK, thanks. Jason
Re: [asan] Fix some asan ICEs
Ok for trunk? 2012-11-27 Jakub Jelinek ja...@redhat.com * asan.c (instrument_mem_region_access): Don't instrument if base doesn't have pointer type or len integral type. Add cast if len doesn't have size_t compatible type. (instrument_builtin_call): Don't instrument BUILT_IN_ATOMIC_LOAD, BUILT_IN_ATOMIC_TEST_AND_SET, BUILT_IN_ATOMIC_CLEAR, BUILT_IN_ATOMIC_EXCHANGE, BUILT_IN_ATOMIC_COMPARE_EXCHANGE and BUILT_IN_ATOMIC_STORE. This is OK. Thanks, and sorry for the delay. -- Dodji
[contrib] Fix buglet in validate_failures.py
An earlier patch had made the command line options a global variable _OPTIONS, but it had not renamed all the uses of the old options argument. * testsuite-management/validate_failures.py: Fix stale use of 'options'. Doug, if you're using validate_failures.py in some other branch, you may want to cherry pick this fix. diff --git a/contrib/testsuite-management/validate_failures.py b/contrib/testsuite-management/validate_failures.py index 483f466..d02b575 100755 --- a/contrib/testsuite-management/validate_failures.py +++ b/contrib/testsuite-management/validate_failures.py @@ -241,7 +241,7 @@ def GetNegativeResult(line): def ParseManifestWorker(result_set, manifest_path): Read manifest_path, adding the contents to result_set. - if options.verbosity = 1: + if _OPTIONS.verbosity = 1: print 'Parsing manifest file %s.' % manifest_path manifest_file = open(manifest_path) for line in manifest_file:
[committed] Fix up reassoc_4.f testcase (PR testsuite/55452)
Hi!
Honza changed recently max-completely-peeled-insns param default from 400 to
100, which broke the testcase assumptions.
Fixed by forcing the old default, everywhere, committed to trunk as obvious.
2012-12-03 Jakub Jelinek ja...@redhat.com
PR testsuite/55452
* gfortran.dg/reassoc_4.f: Use --param max-completely-peeled-insns=400
on all targets, not just s390*.
--- gcc/testsuite/gfortran.dg/reassoc_4.f.jj2012-01-30 00:09:59.0
+0100
+++ gcc/testsuite/gfortran.dg/reassoc_4.f 2012-12-03 16:14:56.429727381
+0100
@@ -1,7 +1,6 @@
! { dg-do compile }
-! { dg-options -O3 -ffast-math -fdump-tree-reassoc1 }
+! { dg-options -O3 -ffast-math -fdump-tree-reassoc1 --param
max-completely-peeled-insns=400 }
! { dg-additional-options --param max-completely-peel-times=16 { target
spu-*-* } }
-! { dg-additional-options --param max-completely-peeled-insns=400 { target
s390*-*-* } }
subroutine anisonl(w,vo,anisox,s,ii1,jj1,weight)
integer ii1,jj1,i1,iii1,j1,jjj1,k1,l1,m1,n1
real*8 w(3,3),vo(3,3),anisox(3,3,3,3),s(60,60),weight
Jakub
