Bug 12095 – Wrong code with -O -inline

Status
RESOLVED
Resolution
FIXED
Severity
critical
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-02-07T00:23:00Z
Last change time
2015-06-09T05:11:47Z
Keywords
pull, wrong-code
Assigned to
nobody
Creator
clugdbug

Comments

Comment #0 by clugdbug — 2014-02-07T00:23:30Z
Applies to both D1 and D2. Requires -O -inline. The "if (e) " branch is taken, even though e is null. ---- int* func12095() { return null; } class Bug12095 { final void get(int a, int b) { auto e = func12095(); if (e) assert (a, "fail"); else assert (!b); } void fun(int k) { get(k, 0); } } void main() { auto yy = new Bug12095; yy.fun(0); }
Comment #1 by bugzilla — 2014-02-07T01:16:07Z
I can repro on Win32. I agree we must fix this.
Comment #2 by clugdbug — 2014-02-07T03:00:33Z
Reduced, shows it is not related to final: --- void bug12905( int a, int b ) { int * e = null; if (e) assert (a); else assert (b); } class Bug12095 { void fun(int k) { bug12905(k, 1); } } void main() { Bug12095 yy = new Bug12095; yy.fun(0); }
Comment #3 by clugdbug — 2014-02-07T03:05:15Z
In fact it doesn't even need a pointer. Looks pretty fundamental. void bug12905( int a, int b ) { int e = 0; if (e) assert (a); else assert (b); } class Bug12095 { void fun(int k) { bug12905(k, 1); } } void main() { Bug12095 yy = new Bug12095; yy.fun(0); }
Comment #4 by yebblies — 2014-02-07T05:27:04Z
The optimizer is turning fun into this: push ecx ; 0000 _ 51 mov eax, 5 ; 0001 _ B8, 00000005 call _D5testx8__assertFiZv ; 0006 _ E8, 00000000(rel) add esp, 4 ; 000B _ 83. C4, 04 ret 4 ; 000E _ C2, 0004 The optimizer sees el:00362EB4 cnt=0 ? TYvoid 00362344 00362E7C el:00362344 cnt=0 const TYint 0L el:00362E7C cnt=0 colon TYvoid 00362D2C 00362E44 el:00362D2C cnt=0 || TYvoid 0036237C 00361F20 el:0036237C cnt=0 var TYint a el:00361F20 cnt=0 call TYvoid 00361F58 0036230C el:00361F58 cnt=0 var TYD func _D5testx8__assertFiZv el:0036230C cnt=0 const TYint 5L el:00362E44 cnt=0 || TYvoid 00362D64 00362E0C el:00362D64 cnt=0 const TYint 1L el:00362E0C cnt=0 call TYvoid 00362DD4 00362D9C el:00362DD4 cnt=0 var TYD func _D5testx8__assertFiZv el:00362D9C cnt=0 const TYint 7L And it brilliantly deduces that as e2 is `1 || assert, void` -> `void` it can replace it with null. Then, seeing e2 is null, it reduces OPcolon down to just `a || assert` Then the code in elcond goes 'hey, the cond is 0 so I'll just take e2'. So then we're left with just assert.
Comment #5 by yebblies — 2014-02-07T05:39:37Z
Comment #6 by yebblies — 2014-02-07T06:51:43Z
Don and Walter, since you're both here: I would really like a green light on refactoring inside interpret.c (and here's the comment I forgot to post half an hour ago) Reduced, this just requires -O void vfunc(); void fun(int k) { int e = 0; e ? k || assert(0) : !e || vfunc(); } void main() { fun(0); } Any expression of this form will trigger it: (something that the optimizer can find is 0, but the constfolder can't) ? (binary expression that can't be reduced) : (expression that gets reduced to NULL by the optimizer) e1 and e2 can be swapped too, with appropriate changes to the condition eg e ? k || assert(0) : e && vfunc(); !e ? !e || vfunc() : k || assert(0); I've fixed this by returning when OPcolon has NULL children, and allowing el_selecte1/e2 to pick a child that is NULL.
Comment #7 by bugzilla — 2014-02-08T15:49:25Z
Nice work tracking it down!
Comment #8 by github-bugzilla — 2014-02-09T00:29:13Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/97efeeae685f18a33b2090e08792e5f64d33b211 Fix Issue 12095 - Wrong code with -O -inline https://github.com/D-Programming-Language/dmd/commit/292a4b96346983cd67ca9398b54a3a143fad756b Merge pull request #3231 from yebblies/issue12095 Issue 12095 - Wrong code with -O -inline
Comment #9 by yebblies — 2014-02-09T00:49:46Z
Fixed for D2
Comment #10 by github-bugzilla — 2014-02-09T00:56:34Z
Commit pushed to dmd-1.x at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/079d0415ae639e339c2c876020314f9b56f805ad Merge pull request #3231 from yebblies/issue12095 Issue 12095 - Wrong code with -O -inline
Comment #11 by github-bugzilla — 2014-02-12T14:37:18Z
Commit pushed to 2.065 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/fb6110265d786bd1d0d33f28161605383c0227aa Merge pull request #3231 from yebblies/issue12095 Issue 12095 - Wrong code with -O -inline