15054 – [3.4 Regression] Bad code due to overlapping stack temporaries (original) (raw)

Description Ulrich Weigand 2004-04-21 19:13:18 UTC

The testcase appended below triggers the abort, because two temporaries that are concurrently used get assigned the same stack slot; thus the constructor of the 'pointer' temporary built inside the 'element' destructor will clobber the 'canary' member of the current 'element'.

To reproduce, build the file with -O (both -O0 and -O2 work correctly).

extern "C" void abort (void);

struct pointer { void* ptr;

pointer(void* x = 0) : ptr(x) {} pointer(const pointer& x) : ptr(x.ptr) {} };

struct element { int canary;

element() : canary(123) { } ~element() { pointer(); if (canary != 123) abort (); } };

inline pointer insert(const element& x) { return pointer(new element(x)); }

int main (void) { insert(element()); return 0; }

Comment 1 Drea Pinski 2004-04-21 19:27:53 UTC

Confirmed. A regression from 3.0.4, it does not work in 3.2.3 either.

Comment 2 Drea Pinski 2004-04-21 20:30:57 UTC

Note it is opimized down to the abort.

(insn 29 28 30 (parallel [ (set (reg/v/f:SI 71 [ this ]) (plus:SI (reg/f:SI 54 virtual-stack-vars) (const_int -32 [0xffffffe0]))) (clobber (reg:CC 17 flags)) ]) -1 (nil) (nil)) (insn 34 33 35 (set (mem/s/j:SI (reg/v/f:SI 71 [ this ]) [0 .canary+0 S4 A32]) (const_int 123 [0x7b])) -1 (nil) (nil))

(insn 41 40 42 (parallel [ (set (reg/v/f:SI 70 [ x ]) (plus:SI (reg/f:SI 54 virtual-stack-vars) (const_int -32 [0xffffffe0]))) (clobber (reg:CC 17 flags)) ]) -1 (nil) (nil))

(insn 51 50 52 (parallel [ (set (reg/v/f:SI 72 [ this ]) (plus:SI (reg/f:SI 54 virtual-stack-vars) (const_int -16 [0xfffffff0]))) (clobber (reg:CC 17 flags)) ]) -1 (nil) (nil)) (insn 52 51 53 (set (mem/i:SI (reg/f:SI 56 virtual-outgoing-args) [0 S4 A32]) (const_int 4 [0x4])) -1 (nil) (nil))

Comment 3 Ulrich Weigand 2004-04-22 13:46:00 UTC

Why did you change the component to 'middle-end'? The bug is already present on the RTL level right after expand (this is on i386):

(insn 144 142 145 (parallel[ (set (reg/v/f:SI 74) (plus:SI (reg/f:SI 54 virtual-stack-vars) (const_int -32 [0xffffffe0]))) (clobber (reg:CC 17 flags)) ] ) -1 (nil) (nil))

(insn 156 153 158 (parallel[ (set (reg/v/f:SI 75) (plus:SI (reg/f:SI 54 virtual-stack-vars) (const_int -32 [0xffffffe0]))) (clobber (reg:CC 17 flags)) ] ) -1 (nil) (nil))

(insn 158 156 159 (set (reg/v/f:SI 76) (const_int 0 [0x0])) -1 (nil) (nil))

(insn 164 162 165 (set (mem/s/j:SI (reg/v/f:SI 75) [0 .ptr+0 S4 A32]) (reg/v/f:SI 76)) -1 (nil) (nil))

(insn 169 167 170 (set (reg:CCZ 17 flags) (compare:CCZ (mem/s/j:SI (reg/v/f:SI 74) [0 .canary+0 S4 A32]) (const_int 123 [0x7b]))) -1 (nil) (nil))

See how .ptr and .canary share the same stack slot?

That this is subsequently optimized away is only the logical conclusion, and not a middle-end bug. The bug is the overlapping stack slot assignment.

Comment 4 Drea Pinski 2004-04-22 14:51:35 UTC

Because middle-end is where the conversion between trees to RTL happens. "GCC's middle end; folks, expand_*, etc. Target dependent parts and optimization passes have their own component" See how expand is part of it.

Comment 6 Drea Pinski 2004-04-30 17:06:51 UTC

Note this is fixed on the tree-ssa as it is optimized before getting to the expand.

Comment 8 Drea Pinski 2004-05-01 12:23:22 UTC

Fixed on the mainline so far.

Comment 11 Gabriel Dos Reis 2004-05-16 20:30:51 UTC

Fixed for 3.3.4. Now only 3.4.x regression.

(I've just been bitten by this bug in my own code :-()

Comment 12 Gabriel Dos Reis 2004-05-17 15:03:13 UTC

Subject: Re: [3.4 Regression] Bad code due to overlapping stack temporaries

Ulrich Weigand <Ulrich.Weigand@de.ibm.com> writes:

| Hello Gaby, | | sorry for not committing this earlier; I was waiting for Mark's approval to | apply to both branches simultaneously ...

Hey Ulrich,

There is no need to sorry! I came to appreciate the patch when my own code got screwed :-)

-- Gaby

Comment 13 Wolfgang Bangerth 2004-05-17 15:40:56 UTC

Mark, what is your word on this for 3.4.1?

W.

Comment 14 Mark Mitchell 2004-05-18 07:35:04 UTC

This patch is OK for 3.4.1. Thanks!

Comment 16 Drea Pinski 2004-05-18 11:49:28 UTC

Fixed.