When passing three uint64_t arguments to a function, AVR-GCC generates completely wrong code. The first two uint64_t arguments are passed in registers, and thus not affected. The caller then allocates 8 bytes on the stack for the fourth argument, but writes to 8 bytes beyond the allocated area (destroying the current function's stack). The callee tries to fetch the values from the locations on the stack where they ought to be (but actually aren't). This bug appears in all tested versions of GCC (3.4.6, 4.1.0, SVN trunk).
Created attachment 11358 [details] Testcase demonstrating the faulty code generated.
Created attachment 11359 [details] Faulty code generated by test case. The faulty code is in lines 136...160. First, 8 bytes of space are allocated on the stack, but the access uses Z+8 through Z+15 instead of Z+1 through Z+8.
Is this one related to PR21834?
(In reply to comment #3) > Is this one related to PR21834? Possible, yes. The symptoms are similar. Sorry for missing that one at my prior search. Anyway, my report has a preprocessed source file attached, so it might be more useful to non-AVR aware GCC developers.
(In reply to comment #4) > > Is this one related to PR21834? > Anyway, my report has a preprocessed source file attached, so it > might be more useful to non-AVR aware GCC developers. Perhaps Richard (Cc'ed) can add an xref to this bug in 21834 (and correct the target triplet to "avr-*-*"), then I'll resolve this bug as a dup.
(Sorry, I hit <enter> a bit too fast.)
Forwarding this comment on behalf of Bjoern Haase: Preliminary analysis of the RTL generated without optimization shows that the problem is present already directly after expand. Maybe the problem is triggered by the fact that the avr port does not have move patterns for DI, however, there seems to be a general problem in the mid-end.: Excerpt of test.c.01.sibling. Personal comments marked with. # ;; Function main (note 2 0 5 NOTE_INSN_DELETED) (note 5 2 3 0 [bb 0] NOTE_INSN_BASIC_BLOCK) (note 3 5 6 0 NOTE_INSN_FUNCTION_BEG) (note 6 3 8 1 [bb 1] NOTE_INSN_BASIC_BLOCK) (insn 8 6 9 1 (set (reg:HI 44) (const_int 42 [0x2a])) -1 (nil) (nil)) #push int parameter on stack (last parameter of function call) (insn 9 8 10 1 (set (mem/i:HI (post_dec:HI (reg/f:HI 32 __SPL__)) [0 S2 A8]) (reg:HI 44)) -1 (nil) (nil)) #allocate 8 Bytes on the stack for the uint64_t var, i.e. stack -= 8 (insn 10 9 11 1 (set (reg/f:HI 32 __SPL__) (plus:HI (reg/f:HI 32 __SPL__) (const_int -8 [0xfffffff8]))) -1 (nil) (nil)) #load first data byte in reg (insn 11 10 12 1 (set (reg:QI 45) (const_int -16 [0xfffffff0])) -1 (nil) (nil)) #write first data byte to stack + 8. instead of stack + 0 !!!! BUG !!!! (insn 12 11 13 1 (set (mem/i:QI (plus:HI (reg/f:HI 32 __SPL__) (const_int 8 [0x8])) [0 S1 A8]) (reg:QI 45)) -1 (nil) (nil)) #load second data byte in reg (insn 13 12 14 1 (set (reg:QI 46) (const_int -34 [0xffffffde])) -1 (nil) (nil)) #write second data byte to stack + 9. instead of stack + 1 : !!!! BUG !!!! (insn 14 13 15 1 (set (mem/i:QI (plus:HI (reg/f:HI 32 __SPL__) (const_int 9 [0x9])) [0 S1 A8]) (reg:QI 46)) -1 (nil) (nil))
Bugmasters: Please mark this bug as NEW. It is a valid bug.
5 months later and this bug still needs to be marked as NEW. Will a bug master please do this?
Test case fails for 4.3-20070525.
*** Bug 21834 has been marked as a duplicate of this bug. ***
The AVR is unusual in having #define STACK_PUSH_CODE POST_DEC #define STACK_GROWS_DOWNWARD where most targets would have PRE_DEC when the stack grows downward. The middle-end tries to synthesize the missing pushdi pattern, but while the offsets seem to be wrong in any case, the instruction order would be correct for PRE_DEC. In expr.c/emit_single_push_insn(), there is this comment: #ifdef STACK_GROWS_DOWNWARD /* ??? This seems wrong if STACK_PUSH_CODE == POST_DEC. */ dest_addr = gen_rtx_PLUS (Pmode, stack_pointer_rtx, GEN_INT (-(HOST_WIDE_INT) rounded_size)); #else Someone needs to set a breakpoint on emit_push_insn() and friends to find out exactly where the problem is.
expr.c appears all messed up on emit_single_push_insn. This bad code gets executed when there is no push instruction available. As well as getting address of the mem created completely wrong, it does not account for any offset between SP and Top/Bottom of Stack aka STACK_POINTER_OFFSET Any comment before I try and fix this mess? First example, ironically without the warning mentioned in latter code. else if (FUNCTION_ARG_PADDING (mode, type) == downward) { unsigned padding_size = rounded_size - GET_MODE_SIZE (mode); HOST_WIDE_INT offset; emit_move_insn (stack_pointer_rtx, expand_binop (Pmode, #ifdef STACK_GROWS_DOWNWARD sub_optab, #else add_optab, #endif stack_pointer_rtx, GEN_INT (rounded_size), NULL_RTX, 0, OPTAB_LIB_WIDEN)); offset = (HOST_WIDE_INT) padding_size; #ifdef STACK_GROWS_DOWNWARD if (STACK_PUSH_CODE == POST_DEC) /* We have already decremented the stack pointer, so get the previous value. */ ///NEXT LINE IS WRONG We are pointing just below value so we need SP + STACK_POINTER_OFFSET offset += (HOST_WIDE_INT) rounded_size; //For PRE_DEC we already point directly to mem so code OK #else if (STACK_PUSH_CODE == POST_INC) /* We have already incremented the stack pointer, so get the previous value. */ //NEXT LINE IS CORRECT offset -= (HOST_WIDE_INT) rounded_size; //For PRE_INC we now add STACK_POINTER_OFFSET or SP will be one lower than mem address #endif dest_addr = gen_rtx_PLUS (Pmode, stack_pointer_rtx, GEN_INT (offset)); } else The rest of code is even worse!
It appears emit_single_push_insn() is BROKEN for targets with: a)STACK_GROWS_DOWNWARDS+POST_DEC push b)Upwards+POST_INC push. So if any target has this combo and #define PUSH_ROUNDING - it is broken. Fortunately for AVR the whole mess goes away when we #undef PUSH_ROUNDING- which appears so far to be uneccesary. It also cleared some compat test failures! For reference here what I did sort out for emit_single_push_insn There are two problems. 1) For downwards padding, MISTAKE in original (H8) change that for (a) adds size of slot to SP to get address - that would be highest byte of mem! This crashes AVR. /* We have already decremented the stack pointer, so get the previous value. */ offset += (HOST_WIDE_INT) rounded_size; I AM VERY WRONG POST_DEC leaves stack pointer at BOS-1. We must add smallest addressable unit of stack (byte,word?) to get address of MEM. Or perhaps use STACK_POINTER_OFFSET. So above becomes. offset += (HOST_WIDE_INT) STACK_POINTER_OFFSET; 2) For Upwards padding both POST_INC and POST_DEC have PRE_MODIFY sequences created for upwards padding. This is questioned in code and clearly incorrect. I assume no target has this combo. One possible way to fix issue is to use POST_MODIFY. However, that would assume final instructions would not be split and cause stack corruption during interrupt. This matter I have not checked. Someone might consider adding some asserts here!
Subject: Bug 27386 Author: hutchinsonandy Date: Wed Jun 4 22:02:57 2008 New Revision: 136377 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=136377 Log: PR target/27386 * config/avr/avr.h: (PUSH_ROUNDING): Remove. Modified: trunk/gcc/ChangeLog trunk/gcc/config/avr/avr.h
Fixed 4.4
Subject: Bug 27386 Author: hutchinsonandy Date: Sat Jun 7 15:48:25 2008 New Revision: 136531 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=136531 Log: Backports from 4.4 PR target/27386 * config/avr/avr.h: (PUSH_ROUNDING): Remove. PR target/30243 * builtins.c (expand_builtin_signbit): Don't take lowpart when register is already smaller or equal to required mode. PR target/34932 * config/avr/avr.md (*addhi3_zero_extend2): Remove. * config/avr/avr.h (MAX_OFILE_ALIGNMENT): Define. Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/builtins.c branches/gcc-4_3-branch/gcc/config/avr/avr.h branches/gcc-4_3-branch/gcc/config/avr/avr.md