Re: [PATCH] [asan] Fix for PR asan/56330
On Fri, Feb 15, 2013 at 06:01:05PM -0500, Jack Howarth wrote: > On Fri, Feb 15, 2013 at 09:54:19PM +0100, Dodji Seketeli wrote: > FAIL: c-c++-common/asan/no-redundant-instrumentation-7.c -O0 > scan-tree-dump-times asan0 "__builtin___asan_report_load" 6 > > at both -m32 and -m64. The no-redundant-instrumentation-7.s from the failing > -m64 > test case is attached, generated with... I think void foo (int *a, char *b, char *c) { __builtin_memcmp (s.a, e, 100); __builtin_memcmp (s.a, e, 200); } is problematic, for -O1 both calls would be definitely removed, because they are pure, and even at -O0 I guess they might be expanded into nothing. Perhaps int foo () { int i = __builtin_memcmp (s.a, e, 100); i += __builtin_memcmp (s.a, e, 200); return i; } or similar would work better. And for pr56330.c testcase, which is there to verify that we don't ICE on it and I believe there was important that both builtins are adjacent, perhaps it should be __builtin_memcpy instead. Jakub
Re: [PATCH] [asan] Fix for PR asan/56330
Jakub Jelinek writes: > On Fri, Feb 15, 2013 at 06:01:05PM -0500, Jack Howarth wrote: >> On Fri, Feb 15, 2013 at 09:54:19PM +0100, Dodji Seketeli wrote: >> FAIL: c-c++-common/asan/no-redundant-instrumentation-7.c -O0 >> scan-tree-dump-times asan0 "__builtin___asan_report_load" 6 >> >> at both -m32 and -m64. The no-redundant-instrumentation-7.s from the failing >> -m64 >> test case is attached, generated with... > > I think > void > foo (int *a, char *b, char *c) > { > __builtin_memcmp (s.a, e, 100); > __builtin_memcmp (s.a, e, 200); > } > is problematic, for -O1 both calls would be definitely removed, because they > are pure, and even at -O0 I guess they might be expanded into nothing. > Perhaps > int > foo () > { > int i = __builtin_memcmp (s.a, e, 100); > i += __builtin_memcmp (s.a, e, 200); > return i; > } > or similar would work better. And for pr56330.c testcase, which is there to > verify that we don't ICE on it and I believe there was important that both > builtins are adjacent, perhaps it should be __builtin_memcpy instead. Right. My first bootstrap actually caught this, I updated the patch locally to just modify that test and sent out the wrong one. Sorry for this. This is the patch that actually passed bootstrap. gcc/ * asan.c (get_mem_refs_of_builtin_call): White space and style cleanup. (instrument_mem_region_access): Do not forget to always put instrumentation of the of 'base' and 'base + len' in a "if (len != 0) statement, even for cases where either 'base' or 'base + len' are not instrumented -- because they have been previously instrumented. Simplify the logic by putting all the statements instrument 'base + len' inside a sequence, and then insert that sequence right before the current insertion point. Then, to instrument 'base + len', just get an iterator on that statement. And do not forget to update the pointer to iterator the function received as argument. gcc/testsuite/ * c-c++-common/asan/no-redundant-instrumentation-4.c: New test file. * c-c++-common/asan/no-redundant-instrumentation-5.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-6.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-7.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-8.c: Likewise. * c-c++-common/asan/pr56330.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-1.c (test1): Ensure the size argument of __builtin_memcpy is a constant. --- gcc/ChangeLog | 16 gcc/asan.c | 97 -- gcc/testsuite/ChangeLog| 12 +++ .../asan/no-redundant-instrumentation-1.c | 2 +- .../asan/no-redundant-instrumentation-4.c | 13 +++ .../asan/no-redundant-instrumentation-5.c | 13 +++ .../asan/no-redundant-instrumentation-6.c | 14 .../asan/no-redundant-instrumentation-7.c | 23 + .../asan/no-redundant-instrumentation-8.c | 14 gcc/testsuite/c-c++-common/asan/pr56330.c | 23 + 10 files changed, 182 insertions(+), 45 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c create mode 100644 gcc/testsuite/c-c++-common/asan/pr56330.c diff --git a/gcc/asan.c b/gcc/asan.c index a569479..67236a9 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -747,20 +747,17 @@ get_mem_refs_of_builtin_call (const gimple call, got_reference_p = true; } -else - { - if (dest) - { - dst->start = dest; - dst->access_size = access_size; - *dst_len = NULL_TREE; - *dst_is_store = is_store; - *dest_is_deref = true; - got_reference_p = true; - } - } + else if (dest) +{ + dst->start = dest; + dst->access_size = access_size; + *dst_len = NULL_TREE; + *dst_is_store = is_store; + *dest_is_deref = true; + got_reference_p = true; +} -return got_reference_p; + return got_reference_p; } /* Return true iff a given gimple statement has been instrumented. @@ -1535,8 +1532,15 @@ instrument_mem_region_access (tree base, tree len, /* If the beginning of the memory region has already been instrumented, do not instrument it. */ - if (has_mem_ref_been_instrumented (base, 1)) -goto after_first_instrumentation; + bool start_instrumented = has_mem_ref_been_instrumented (base, 1); + + /* If the end of
Re: [PATCH] [asan] Fix for PR asan/56330
On Sat, Feb 16, 2013 at 09:44:51AM +0100, Dodji Seketeli wrote: > Right. My first bootstrap actually caught this, I updated the patch > locally to just modify that test and sent out the wrong one. Sorry for > this. This is the patch that actually passed bootstrap. Ok. Actually, please change gcc/testsuite/c-c++-common/asan/pr56330.c to the same thing, I've just verified it ICEs the same way without the patch even when it is int foo (void) { int d = __builtin_memcmp (s.a, e, 100); d += __builtin_memcmp (s.a, e, 200); return d; } (while with __builtin_memcpy (s.a, e, 100); __builtin_memcpy (s.a, e, 200); it doesn't ICE without the patch). While pr56330.c test look to be redundant, it is at least tortured at all compilation options, so it tests something that the other tests don't. No need to rebootstrap/retest. Jakub
Re: [PATCH] [asan] Fix for PR asan/56330
Jakub Jelinek writes: > On Sat, Feb 16, 2013 at 09:44:51AM +0100, Dodji Seketeli wrote: >> Right. My first bootstrap actually caught this, I updated the patch >> locally to just modify that test and sent out the wrong one. Sorry for >> this. This is the patch that actually passed bootstrap. > > Ok. Actually, please change gcc/testsuite/c-c++-common/asan/pr56330.c > to the same thing, Oops, right. Done below. > I've just verified it ICEs the same way without the patch > even when it is > int > foo (void) > { > int d = __builtin_memcmp (s.a, e, 100); > d += __builtin_memcmp (s.a, e, 200); > return d; > } Yes, I have just verified this too, thanks. > > (while with > __builtin_memcpy (s.a, e, 100); > __builtin_memcpy (s.a, e, 200); > it doesn't ICE without the patch). Hmmh, interesting. > While pr56330.c test look to be redundant, it is at least tortured at all > compilation options, so it tests something that the other tests don't. Yes, that is why I kept it in the set of tests. > No need to rebootstrap/retest. I re-ran the tests locally w/o bootstrap though. I am going to commit this now. Thanks. gcc/ * asan.c (get_mem_refs_of_builtin_call): White space and style cleanup. (instrument_mem_region_access): Do not forget to always put instrumentation of the of 'base' and 'base + len' in a "if (len != 0) statement, even for cases where either 'base' or 'base + len' are not instrumented -- because they have been previously instrumented. Simplify the logic by putting all the statements instrument 'base + len' inside a sequence, and then insert that sequence right before the current insertion point. Then, to instrument 'base + len', just get an iterator on that statement. And do not forget to update the pointer to iterator the function received as argument. gcc/testsuite/ * c-c++-common/asan/no-redundant-instrumentation-4.c: New test file. * c-c++-common/asan/no-redundant-instrumentation-5.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-6.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-7.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-8.c: Likewise. * c-c++-common/asan/pr56330.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-1.c (test1): Ensure the size argument of __builtin_memcpy is a constant. --- gcc/ChangeLog | 16 gcc/asan.c | 97 -- gcc/testsuite/ChangeLog| 12 +++ .../asan/no-redundant-instrumentation-1.c | 2 +- .../asan/no-redundant-instrumentation-4.c | 13 +++ .../asan/no-redundant-instrumentation-5.c | 13 +++ .../asan/no-redundant-instrumentation-6.c | 14 .../asan/no-redundant-instrumentation-7.c | 23 + .../asan/no-redundant-instrumentation-8.c | 14 gcc/testsuite/c-c++-common/asan/pr56330.c | 24 ++ 10 files changed, 183 insertions(+), 45 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c create mode 100644 gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c create mode 100644 gcc/testsuite/c-c++-common/asan/pr56330.c diff --git a/gcc/asan.c b/gcc/asan.c index a569479..67236a9 100644 --- a/gcc/asan.c +++ b/gcc/asan.c @@ -747,20 +747,17 @@ get_mem_refs_of_builtin_call (const gimple call, got_reference_p = true; } -else - { - if (dest) - { - dst->start = dest; - dst->access_size = access_size; - *dst_len = NULL_TREE; - *dst_is_store = is_store; - *dest_is_deref = true; - got_reference_p = true; - } - } + else if (dest) +{ + dst->start = dest; + dst->access_size = access_size; + *dst_len = NULL_TREE; + *dst_is_store = is_store; + *dest_is_deref = true; + got_reference_p = true; +} -return got_reference_p; + return got_reference_p; } /* Return true iff a given gimple statement has been instrumented. @@ -1535,8 +1532,15 @@ instrument_mem_region_access (tree base, tree len, /* If the beginning of the memory region has already been instrumented, do not instrument it. */ - if (has_mem_ref_been_instrumented (base, 1)) -goto after_first_instrumentation; + bool start_instrumented = has_mem_ref_been_instrumented (base, 1); + + /* If the end of the memory region has already been instrumented, do + not instrument it. */ + tree end = asan_mem_ref_get_end (base, len
[PATCH] Add new debug_bb_range debugging function
Hello, In my first foray of debugging gimple stuff, I felt the need for a function function to dump a range of basic block from number N to P to stderr. I find this a bit more handy than calling debug_bb_n on each basic block instead. I understand this is material for 4.9, so if you agree I'll stage it in my queue of patches for when stage1 opens. Is this OK, or is there a function that does this already (or something else :))? Thanks. Tested on x86_64-unknown-linux-gnu against trunk. gcc/ * basic-block.h (debug_bb_range): Declare ... * cfg.c (debug_bb_range): ... new function. --- gcc/basic-block.h | 1 + gcc/cfg.c | 15 +++ 2 files changed, 16 insertions(+) diff --git a/gcc/basic-block.h b/gcc/basic-block.h index 90eb57b..8407c4a 100644 --- a/gcc/basic-block.h +++ b/gcc/basic-block.h @@ -752,6 +752,7 @@ extern bool predictable_edge_p (edge); extern void init_flow (struct function *); extern void debug_bb (basic_block); extern basic_block debug_bb_n (int); +extern basic_block debug_bb_range (int, int); extern void dump_flow_info (FILE *, int); extern void expunge_block (basic_block); extern void link_block (basic_block, basic_block); diff --git a/gcc/cfg.c b/gcc/cfg.c index 9e4380c..d34f27e 100644 --- a/gcc/cfg.c +++ b/gcc/cfg.c @@ -663,6 +663,21 @@ debug_bb_n (int n) return bb; } +/* Dumps cfg related information about basic blocks, from number 'S' + to number E, to stderr. */ + +DEBUG_FUNCTION basic_block +debug_bb_range (int s, int e) +{ + basic_block bb = NULL; + for (int i = s; i <= e; ++i) +{ + bb = BASIC_BLOCK (i); + debug_bb (bb); +} + return bb; +} + /* Dumps cfg related information about basic block BB to OUTF. If HEADER is true, dump things that appear before the instructions contained in BB. If FOOTER is true, dump things that appear after. -- Dodji
Re: [PATCH] Add new debug_bb_range debugging function
On Sat, Feb 16, 2013 at 10:40:43AM +0100, Dodji Seketeli wrote: > --- a/gcc/cfg.c > +++ b/gcc/cfg.c > @@ -663,6 +663,21 @@ debug_bb_n (int n) >return bb; > } > > +/* Dumps cfg related information about basic blocks, from number 'S' > + to number E, to stderr. */ > + > +DEBUG_FUNCTION basic_block > +debug_bb_range (int s, int e) > +{ > + basic_block bb = NULL; > + for (int i = s; i <= e; ++i) > +{ > + bb = BASIC_BLOCK (i); > + debug_bb (bb); At some points after cfg changes, but before cfg cleanup there might be gaps, so I think you want if (bb) debug_bb (bb);, otherwise it could crash in that case. Jakub
Re: [PATCH] Add new debug_bb_range debugging function
Jakub Jelinek writes: > On Sat, Feb 16, 2013 at 10:40:43AM +0100, Dodji Seketeli wrote: >> --- a/gcc/cfg.c >> +++ b/gcc/cfg.c >> @@ -663,6 +663,21 @@ debug_bb_n (int n) >>return bb; >> } >> >> +/* Dumps cfg related information about basic blocks, from number 'S' >> + to number E, to stderr. */ >> + >> +DEBUG_FUNCTION basic_block >> +debug_bb_range (int s, int e) >> +{ >> + basic_block bb = NULL; >> + for (int i = s; i <= e; ++i) >> +{ >> + bb = BASIC_BLOCK (i); >> + debug_bb (bb); > > At some points after cfg changes, but before cfg cleanup there might be > gaps, so I think you want if (bb) debug_bb (bb);, otherwise it could crash > in that case. Right. I have updated the patch below for that and also to ensure that we don't try to access a basic block with number >= last_basic_block. Thanks. gcc/ * basic-block.h (debug_bb_range): Declare ... * cfg.c (debug_bb_range): ... new function. --- gcc/basic-block.h | 1 + gcc/cfg.c | 16 2 files changed, 17 insertions(+) diff --git a/gcc/basic-block.h b/gcc/basic-block.h index 90eb57b..8407c4a 100644 --- a/gcc/basic-block.h +++ b/gcc/basic-block.h @@ -752,6 +752,7 @@ extern bool predictable_edge_p (edge); extern void init_flow (struct function *); extern void debug_bb (basic_block); extern basic_block debug_bb_n (int); +extern basic_block debug_bb_range (int, int); extern void dump_flow_info (FILE *, int); extern void expunge_block (basic_block); extern void link_block (basic_block, basic_block); diff --git a/gcc/cfg.c b/gcc/cfg.c index 9e4380c..d72e075 100644 --- a/gcc/cfg.c +++ b/gcc/cfg.c @@ -663,6 +663,22 @@ debug_bb_n (int n) return bb; } +/* Dumps cfg related information about basic blocks, from number 'S' + to number E, to stderr. */ + +DEBUG_FUNCTION basic_block +debug_bb_range (int s, int e) +{ + basic_block bb = NULL; + for (int i = s; i <= e && i < last_basic_block; ++i) +{ + bb = BASIC_BLOCK (i); + if (bb) + debug_bb (bb); +} + return bb; +} + /* Dumps cfg related information about basic block BB to OUTF. If HEADER is true, dump things that appear before the instructions contained in BB. If FOOTER is true, dump things that appear after. -- 1.8.1 -- Dodji
Re: PR target/52555: attribute optimize is overriding command line options
Aldy Hernandez writes: > diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c > index b203cdd..5e98485 100644 > --- a/gcc/config/mips/mips.c > +++ b/gcc/config/mips/mips.c > @@ -16313,7 +16313,26 @@ mips_set_mips16_mode (int mips16_p) >if (mips16_p) > { >if (!mips16_globals) > - mips16_globals = save_target_globals (); > + { > + if (optimization_current_node != optimization_default_node) > + { > + tree opts = optimization_current_node; > + /* Temporarily switch to the default optimization node, > + so that *this_target_optabs is set to the default, > + not reflecting whatever the first mips16 function > + uses for the optimize attribute. */ > + optimization_current_node = optimization_default_node; > + cl_optimization_restore > + (&global_options, > + TREE_OPTIMIZATION (optimization_default_node)); > + mips16_globals = save_target_globals (); > + optimization_current_node = opts; > + cl_optimization_restore (&global_options, > +TREE_OPTIMIZATION (opts)); > + } > + else > + mips16_globals = save_target_globals (); > + } I think this should go in target-globals.c, maybe as save_target_globals_default_opts. > diff --git a/gcc/function.c b/gcc/function.c > index 4ce2259..c5eea2e 100644 > --- a/gcc/function.c > +++ b/gcc/function.c > @@ -4400,6 +4400,31 @@ invoke_set_current_function_hook (tree fndecl) > } > >targetm.set_current_function (fndecl); > + > + if (opts == optimization_default_node) > + this_fn_optabs = this_target_optabs; > + else > + { > + struct function *fn = DECL_STRUCT_FUNCTION (fndecl); > + if (fn->optabs == NULL) > + { > + if (!SWITCHABLE_TARGET) > + fn->optabs = TREE_OPTIMIZATION_OPTABS (opts); > + else > + { > + if (this_target_optabs == &default_target_optabs) > + fn->optabs = TREE_OPTIMIZATION_OPTABS (opts); > + else > + { > + fn->optabs = (unsigned char *) > + ggc_alloc_atomic (sizeof (struct target_optabs)); > + init_all_optabs ((struct target_optabs *) fn->optabs); > + } > + } Following on from Jakub's: if (!SWITCHABLE_TARGET || this_target_optabs == &default_target_optabs) fn->optabs = TREE_OPTIMIZATION_OPTABS (opts); I'd prefer just: if (this_target_optabs == &default_target_optabs) fn->optabs = TREE_OPTIMIZATION_OPTABS (opts); Looks good to me otherwise, thanks. Sorry that SWITCHABLE_TARGETS has been so much hassle. TBH, like Jakub says in the PR, I was hoping things like the optimize attribute could use the target_globals stuff too. Today we just care about optabs, but e.g. arm.c has: if (TARGET_THUMB1 && optimize_size) { /* When optimizing for size on Thumb-1, it's better not to use the HI regs, because of the overhead of stacking them. */ for (regno = FIRST_HI_REGNUM; regno <= LAST_HI_REGNUM; ++regno) fixed_regs[regno] = call_used_regs[regno] = 1; } which affects other cached global state like IRA tables. The rtx_costs also often depend on optimize_size, and are cached in various places like expmed.c. E.g. for: int foo (int x, int y) { return x * 10; } the -O2 version on MIPS32 is: sll $2,$4,1 sll $4,$4,3 j $31 addu$2,$2,$4 and the -Os version is: li $2,10 j $31 mul $2,$4,$2 But even if an optimize attribute is added: int __attribute__((optimize(2))) foo (int x, int y) { return x * 10; } what you get depends on whether -Os or -O2 was passed on the command line. The attribute doesn't affect things either way. The same thing happens on x86_64. So in the end I think we'll end up trying solve the same problem that the SWITCHABLE_TARGETS stuff was trying to solve, but this time for __attribute__((optimize)). Richard
Re: PATCH: Correctly configure all big-endian ARM archs, not just arm*-*-linux-*.
On 15 February 2013 19:28:29 Richard Earnshaw wrote: On 15/02/13 18:20, Seth LaForge wrote: > Currently, for arm-* archs, TARGET_BIG_ENDIAN_DEFAULT is only set in > the case branch for arm*-*-linux-*, not for other arm architectures. > We're compiling for the TI TMS570, which is a big-endian processor, > with target set to armeb-unknown-eabi. The following patch moves the > big-endian check out of the big architecture case, so that > TARGET_BIG_ENDIAN_DEFAULT is consistently set for any armeb-*-* > target. > > We've been using this with good results for over a year at Google on > TMS570 processors. > > This fixes bug 52187 - armeb-unknown-eabi not recognized as big-endian. > > Seth LaForge > > > gcc/ > * config.gcc: Add TARGET_BIG_ENDIAN_DEFAULT=1 for all arm*b archs. > > diff -u -r gcc-4.8-20130210/gcc/config.gcc gcc-4.8-20130210.new/gcc/config.gcc > --- gcc-4.8-20130210/gcc/config.gcc2013-02-08 08:02:47.0 -0800 > +++ gcc-4.8-20130210.new/gcc/config.gcc2013-02-14 16:37:14.282107219 -0800 > @@ -809,6 +809,13 @@ > ;; > esac > > +# Handle big-endian ARM architectures. > +case ${target} in > +arm*b-*-*) > + tm_defines="${tm_defines} TARGET_BIG_ENDIAN_DEFAULT=1" > + ;; Not ok. This would mismatch on arm-blob-linux-gnueabi and cause it to be treated as big-endian. Sounds like a DUP of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16350 Is the missing hunk in by now (cannot look myself right now)? Thanks, R. Sent with AquaMail for Android http://www.aqua-mail.com
Re: GCC 4.8.0 Status Report (2013-02-14)
I updated the main page accordingly. Gerald Index: index.html === RCS file: /cvs/gcc/wwwdocs/htdocs/index.html,v retrieving revision 1.869 diff -u -3 -p -r1.869 index.html --- index.html 23 Jan 2013 20:02:00 - 1.869 +++ index.html 16 Feb 2013 15:42:38 - @@ -194,7 +194,7 @@ Any additions? Don't be shy, send them Status: - http://gcc.gnu.org/ml/gcc/2013-01/msg00071.html";>2013-01-08 + http://gcc.gnu.org/ml/gcc/2013-02/msg00177.html";>2013-02-14 (regression fixes and docs only).
Re: [Patch, microblaze]: Add support for nested functions
On 02/10/2013 10:37 PM, David Holsgrove wrote: Add MicroBlaze support for nested functions. Changelog 2013-02-11 Edgar E. Iglesias * config/microblaze/microblaze.c (microblaze_asm_trampoline_template): Replace with a microblaze version. (microblaze_trampoline_init): Adapt for microblaze. * config/microblaze/microblaze.h (TRAMPOLINE_SIZE): Adapt for microblaze. Committed revision 196103. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [Patch, microblaze]: Add TARGET_SUPPORTS_PIC check
On 02/10/2013 10:38 PM, David Holsgrove wrote: Add TARGET_SUPPORTS_PIC flag and check that the flag_pic = 2 Changelog 2013-02-11 Edgar E. Iglesias * config/microblaze/linux.h (TARGET_SUPPORTS_PIC): Define as 1. * config/microblaze/microblaze.h (TARGET_SUPPORTS_PIC): Define as 1. * config/microblaze/microblaze.c (microblaze_option_override): Bail out early for PIC modes when target does not support PIC. gcc/testsuite/Changelog 2013-02-11 Edgar E. Iglesias * gcc.dg/20020312-2.c: Define MicroBlaze PIC register Committed revision 196104. -- Michael Eagerea...@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
Re: [PATCH][www] Add loop case to non-bugs in bugs.html
On Thu, 7 Feb 2013, Richard Biener wrote: > As promised, a bugs.html entry. Looks good to me. Now, at the top of the page we have Before reporting that GCC compiles your code incorrectly, compile it with gcc -Wall -Wextra and see whether this shows anything wrong with your code. Similarly, if compiling with -fno-strict-aliasing -fwrapv makes a difference, your code probably is not correct. Should I add -fno-aggressive-loop-optimizations to the list in the second sentence? Gerald
Re: [PATCH][RFC] Add -fno-aggressive-loop-optimizations
On Thu, 31 Jan 2013, Richard Biener wrote: > +GCC now uses more a aggressive analysis to derive an upper bound for I think "a " should be omitted here. > +the number of iterations of loops using constraints imposed by language > +standards. This may cause non-conforming programs to no longer work as > +expected, such as SPEC CPU 2006 464.h264ref and 416.gamess. A new > +option, -fno-aggressive-loop-optimizations, was added > +to disable this aggressive analysis. Personally I would reduce aggressiveness and omit "aggressive" in the last line above. :-) Gerald
[PATCH] Fix PR50293 - LTO plugin with space in path
Mainline and 4.7 failed to use LTO when toolchain is installed to a path with space in it. Resulting in error message like: /ld.exe: c:/program: error loading plugin collect2.exe: error: ld returned 1 exit status Root cause is when GCC driver process specs, it doesn't handle plugin file name properly. Here is a patch fixing it. Bootstraped and make check on x86 and aarch32, no regression. OK to trunk and 4.7? ChangeLog: PR lto/50293 * gcc.c (convert_white_space): New function. (main): Handles white space in function name. Index: gcc/gcc.c === --- gcc/gcc.c (revision 195189) +++ gcc/gcc.c (working copy) @@ -265,6 +265,7 @@ static const char *compare_debug_auxbase_opt_spec_function (int, const char **); static const char *pass_through_libs_spec_func (int, const char **); static const char *replace_extension_spec_func (int, const char **); +static char * convert_white_space(char * orig); /* The Specs Language @@ -6595,6 +6596,7 @@ X_OK, false); if (lto_wrapper_file) { + lto_wrapper_file = convert_white_space(lto_wrapper_file); lto_wrapper_spec = lto_wrapper_file; obstack_init (&collect_obstack); obstack_grow (&collect_obstack, "COLLECT_LTO_WRAPPER=", @@ -7005,12 +7007,13 @@ + strlen (fuse_linker_plugin), 0)) #endif { - linker_plugin_file_spec = find_a_file (&exec_prefixes, + char * temp_spec = find_a_file (&exec_prefixes, LTOPLUGINSONAME, R_OK, false); - if (!linker_plugin_file_spec) + if (!temp_spec) fatal_error ("-fuse-linker-plugin, but %s not found", LTOPLUGINSONAME); + linker_plugin_file_spec = convert_white_space(temp_spec); } #endif lto_gcc_spec = argv[0]; @@ -8506,3 +8509,30 @@ free (name); return result; } + +/* Insert back slash before spaces in a string, to avoid path + that has space in it broken into multiple arguments. */ + +static char * convert_white_space(char * orig) +{ + int len, number_of_space = 0; + char *p = orig; + if (orig == NULL) return NULL; + + for (len=0; p[len]; len++) +if (p[len] == ' ' || p[len] == '\t') number_of_space ++; + + if (number_of_space) +{ + char * new_spec = (char *)xmalloc(len + number_of_space + 1); + int j,k; + for (j=0, k=0; j<=len; j++, k++) + { + if (p[j] == ' ' || p[j] == '\t') new_spec[k++]='\\'; + new_spec[k] = p[j]; + } + free(CONST_CAST(char *, orig)); + return new_spec; + } + else return orig; +}
Re: [PING^5] PR 54805: __gthread_tsd* in vxlib-tls.c
On 14/02/2013, at 10:18 AM, rbmj wrote: > On 18-Jan-13 20:35, Maxim Kuvyrkov wrote: >> On 19/01/2013, at 9:18 AM, rbmj wrote: >> > -150,7 +158,7 @@ static __gthread_once_t tls_init_guard = > need to read tls_keys.dtor[key] atomically. */ > > static void > -tls_delete_hook (void *tcb ATTRIBUTE_UNUSED) > +tls_delete_hook (void *tcb) Don't remove ATTRIBUTE_UNUSED. TCB was and will remain unused #ifdef __RTP__. >>> >>> And #ifndef __RTP__ ? >> >> No, simply leave that line as is. ATTRIBUTE_UNUSED tells the compiler that >> a variable can be unused, but not necessarily is unused. It's fine to have >> this attribute set on variables that are used under certain preprocessor >> configurations. >> > > Seems like I kept this email in drafts and never sent it out... > > Sorry about that. > > Here's the updated, (trivial) patch. Thanks. I'll apply this once 4.8 branches and trunk is back into development mode. -- Maxim Kuvyrkov