Message ID | 201009021301.49046.ebotcazou@adacore.com |
---|---|
State | New |
Headers | show |
On Thu, Sep 02, 2010 at 01:01:48PM +0200, Eric Botcazou wrote: > > > It is the gcc.c-torture/execute/builtins/sprintf-chk.c compilation which is > > exhibiting three behaviors... > > > > 1) consistent code > > 2) random differences in code > > 3) compiler ICE > > > > My main concern is if this bug will spill over into production code > > generation. > > Sure, but I don't really see the relationship with my patch. It has been used > for more than 4 years in AdaCore compilers based on 3 different GCC versions > on a wide range of architectures and OSes (including Darwin) so I've a hard > time believing it could be responsible for random failures like this one, > especially if you don't pass -fstack-usage to the compiler. > Eric, Looking at the patch, it seems to me that I could strip out everything wrappered in if flag_stack_usage statements and still end up with very significant code changes in the compiler execution even without -fstack-usage being used. For example, the calls to allocate_dynamic_stack_space() in calls.c and changes like... @@ -1223,13 +1255,28 @@ insns. Since this is an extremely rare event, we have no reliable way of knowing which systems have this problem. So we avoid even momentarily mis-aligning the stack. */ + if (!known_align_valid || known_align % PREFERRED_STACK_BOUNDARY != 0) + { + size = round_push (size); - /* If we added a variable amount to SIZE, - we can no longer assume it is aligned. */ -#if !defined (SETJMP_VIA_SAVE_AREA) - if (MUST_ALIGN || known_align % PREFERRED_STACK_BOUNDARY != 0) -#endif - size = round_push (size); + if (flag_stack_usage) + { + int align = PREFERRED_STACK_BOUNDARY / BITS_PER_UNIT; + stack_usage_size = (stack_usage_size + align - 1) / align * align; + } + } + + /* The size is supposed to be fully adjusted at this point so record it + if stack usage info is requested. */ + if (flag_stack_usage) + { + current_function_dynamic_stack_size += stack_usage_size; + + /* ??? This is gross but the only safe stance in the absence + of stack usage oriented flow analysis. */ + if (!cannot_accumulate) + current_function_has_unbounded_dynamic_stack_size = 1; + } do_pending_stack_adjust (); It seems like a leap of faith to say this code executes identically to the previous in the absence of the -fstack-usage flag. Jack
> It seems like a leap of faith to say this code executes identically > to the previous in the absence of the -fstack-usage flag. No, it isn't, just look at both codes and you'll be convinced as well. The only (possible) changes are for SETJMP_VIA_SAVE_AREA targets, i.e. SPARC.
Index: gcc.target/i386/stack-usage-realign.c =================================================================== --- gcc.target/i386/stack-usage-realign.c (revision 163745) +++ gcc.target/i386/stack-usage-realign.c (working copy) @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-require-effective-target ilp32 } */ +/* { dg-skip-if "no stack realignment" { *-*-darwin* } { "*" } { "" } } */ /* { dg-options "-fstack-usage -msse2 -mforce-drap" } */ typedef int __attribute__((vector_size(16))) vec; Index: gcc.dg/stack-usage-1.c =================================================================== --- gcc.dg/stack-usage-1.c (revision 163745) +++ gcc.dg/stack-usage-1.c (working copy) @@ -8,7 +8,11 @@ Then check that this is the actual stack usage in the assembly file. */ #if defined(__i386__) -# define SIZE 248 +# if defined (__MACH__) +# define SIZE 232 +# else +# define SIZE 248 +# endif #elif defined(__x86_64__) # define SIZE 356 #elif defined (__sparc__)