www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 12095] New: Wrong code with -O -inline and final functions

reply d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095

           Summary: Wrong code with -O -inline and final functions
           Product: D
           Version: D1 & D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: critical
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: clugdbug yahoo.com.au


--- Comment #0 from Don <clugdbug yahoo.com.au> 2014-02-07 00:23:30 PST ---
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);
}

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla digitalmars.com


--- Comment #1 from Walter Bright <bugzilla digitalmars.com> 2014-02-07
01:16:07 PST ---
I can repro on Win32. I agree we must fix this.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #2 from Don <clugdbug yahoo.com.au> 2014-02-07 03:00:33 PST ---
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);
}

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #3 from Don <clugdbug yahoo.com.au> 2014-02-07 03:05:15 PST ---
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);
}

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |yebblies gmail.com


--- Comment #4 from yebblies <yebblies gmail.com> 2014-02-08 00:27:04 EST ---
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.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095


yebblies <yebblies gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull


--- Comment #5 from yebblies <yebblies gmail.com> 2014-02-08 00:39:37 EST ---
https://github.com/D-Programming-Language/dmd/pull/3231

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #6 from yebblies <yebblies gmail.com> 2014-02-08 01:51:43 EST ---
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.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 07 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #7 from Walter Bright <bugzilla digitalmars.com> 2014-02-08
15:49:25 PST ---
Nice work tracking it down!

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 08 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #8 from github-bugzilla puremagic.com 2014-02-09 00:29:13 PST ---
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

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 09 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #9 from yebblies <yebblies gmail.com> 2014-02-09 19:49:46 EST ---
Fixed for D2

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 09 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #10 from github-bugzilla puremagic.com 2014-02-09 00:56:34 PST ---
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

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 09 2014
prev sibling next sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 09 2014
prev sibling parent d-bugmail puremagic.com writes:
https://d.puremagic.com/issues/show_bug.cgi?id=12095



--- Comment #11 from github-bugzilla puremagic.com 2014-02-12 14:37:18 PST ---
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

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 12 2014