www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 3736] New: corrupted struct returned by function with optimizations (-O)

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

           Summary: corrupted struct returned by function with
                    optimizations (-O)
           Product: D
           Version: 2.030
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Keywords: wrong-code
          Severity: regression
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: spam extrawurst.org


--- Comment #0 from Stephan Dilly <spam extrawurst.org> 2010-01-23 10:02:04 PST
---
building the following code with -O corrupts the return value of getFoo.
sorry could not reduce it more:

import std.stdio;

struct Foo
{
    float x=0;
}

struct Bar
{
    Foo p0;
    Foo p1;
}

Bar a = {Foo(0),Foo(400)};

Foo getFoo(Foo _pos)
{
    Bar stillok = a;

    float d0 = (a.p0.x - _pos.x);
    float d1 = (a.p0.x - _pos.x);

    if( d0 > d1 )
    {
        return stillok.p0;
    }
    else
    {
        return stillok.p1;
    }
}

void main()
{
    writefln("%s", getFoo( Foo(100) ).x );
}

// prints 6.91062e-39 under dmd2039
// prints 3.2372e-39 under dmd2030

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 23 2010
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3736


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au
            Version|2.030                       |1.020
           Severity|regression                  |critical


--- Comment #1 from Don <clugdbug yahoo.com.au> 2010-01-24 23:53:23 PST ---
This isn't actually a regression. It fails on D2.00, and D1.020, 1.041, 1.055
as well. 

Reduced test case:
---------------
struct Foo
{
    int x;
}

Foo getFoo(Foo irrelevant)
{
    Foo p = Foo(400);
    if ( p.x > p.x )
        return irrelevant;
    else
        return p;        
}

void main()
{
   assert(getFoo( Foo(0) ).x == 400);
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jan 24 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3736



--- Comment #2 from Don <clugdbug yahoo.com.au> 2010-02-07 13:47:44 PST ---
The problem has something to do with the transformation from 
if(cond) return xxx; else return yyy;  into return cond ? xxx : yyy;

If I comment out blockopt.c brcombine() lines 734-764, the test case doesn't
fail. That doesn't necessarily mean that the problem lies there, though.

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


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch


--- Comment #3 from Don <clugdbug yahoo.com.au> 2010-02-08 21:02:14 PST ---
This seems to be a problem with transformations which copy the type 'Ety'
without
also copying the size 'Enumbytes', but you need the struct size to be retained.
when the struct size is zero, blocks get dropped and bad codegen results.
This particular bug is fixed by adding:
in optelem(), cgelem.c line 4376 (this is called with op==OPcond).

  if (!OTrtol(op) && op != OPparam && op != OPcolon && op != OPcolon2 &&
          e1->Eoper == OPcomma)
      {    // Convert ((a,b) op c) to (a,(b op c))
        e1->Ety = e->Ety;
+        e1->Enumbytes = e->Enumbytes;
        e->E1 = e1->E2;
        e1->E2 = e;
        e = e1;
        goto beg;
      }      

blockopt.c, brcombine() line 755:

            if (EOP(b3->Belem))
                continue;
            ty = (bc2 == BCretexp) ? b2->Belem->Ety : TYvoid;
            e = el_bin(OPcolon2,ty,b2->Belem,b3->Belem);
            b->Belem = el_bin(OPcond,ty,b->Belem,e);
            }
+            b->Belem->Enumbytes = b2->Belem->Enumbytes;
            b->BC = bc2;


A dozen lines further down the same function are two other calls to 
el_bin(OPcond,...) or el_bin(OPcolon2,...) in brcombine in blockopt.c.

So I suspect line 797 should also be added:

                e = el_bin(OPcolon2,b2->Belem->Ety,
                    b2->Belem,b3->Belem);
                e = el_bin(OPcond,e->Ety,b->Belem,e);
+                e->Enumbytes=b2->Belem->Enumbytes;
                }

But this is just speculation, I don't have a test case which fails there.
There are no other instances of el_bin(OPcond,...) or el_bin(OPcolon2,...)
anywhere else in the backend.
I suspect there are other cases in the backend where types are transferred
but Enumbytes is lost. (there are several in el_comma in cgelem.c).
But mostly this probably just results in lost optimisation opportunities.

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



--- Comment #4 from Don <clugdbug yahoo.com.au> 2010-02-10 00:58:30 PST ---
Here is a test case which fails in DMC 8.42n. It hits the breakpoint when
compiled with -O.
----------
struct Foo {
    int x;
};

Foo getFoo(Foo irrelevant) {
    Foo p;
    p.x=400;
    if ( p.x > p.x )
        return irrelevant;
    else
        return p;        
}

void main() {
   Foo z;
   z.x=0;
   int y = getFoo( z ).x;
   if (y!=400) _asm int 3;
}
----------

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 10 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3736


Walter Bright <bugzilla digitalmars.com> changed:

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


--- Comment #5 from Walter Bright <bugzilla digitalmars.com> 2010-02-12
01:36:21 PST ---
Changeset 380

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Feb 12 2010
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3736


Kosmonaut <Kosmonaut tempinbox.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Kosmonaut tempinbox.com


--- Comment #6 from Kosmonaut <Kosmonaut tempinbox.com> 2010-02-12 11:48:07 PST
---
(In reply to comment #5)
 Changeset 380
http://www.dsource.org/projects/dmd/changeset/380 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Feb 12 2010
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3736


Walter Bright <bugzilla digitalmars.com> changed:

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


--- Comment #7 from Walter Bright <bugzilla digitalmars.com> 2010-03-08
22:22:18 PST ---
Fixed dmd 1.057 and 2.041

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Mar 08 2010