https://d.puremagic.com/issues/show_bug.cgi?id=11740

           Summary: [64-bit] Struct with constructor incorrectly passed on
                    stack
           Product: D
           Version: D2
          Platform: x86_64
        OS/Version: All
            Status: NEW
          Keywords: wrong-code
          Severity: critical
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: yebblies@gmail.com


--- Comment #0 from yebblies <yebblies@gmail.com> 2013-12-15 04:22:12 EST ---
In func, 's' is passed on the stack instead of in a register pair, only when the constructor is present.  This blocks DDMD on linux64.

extern(C++)
class C
{
extern(C++) static void func(S s, C c)
{
    assert(c);
    assert(s.filename);
    assert(s.linnum);
}

}

struct S
{
    const(char)* filename;
    uint linnum;
    this(const(char)* filename, uint linnum) {}
}

int main()
{
    C.func(S(null, 0), null);
    return 0;
}


0000000000000000 <_ZN1C4funcE1SPS_>:
   0:   55                      push   %rbp
   1:   48 8b ec                mov    %rsp,%rbp
   4:   48 83 ec 10             sub    $0x10,%rsp
   8:   48 89 7d f8             mov    %rdi,-0x8(%rbp) (should be in rdx, not
rdi)
   c:   48 83 7d f8 00          cmpq   $0x0,-0x8(%rbp)
  11:   75 0a                   jne    1d <_ZN1C4funcE1SPS_+0x1d>
  13:   bf 07 00 00 00          mov    $0x7,%edi
  18:   e8 00 00 00 00          callq  1d <_ZN1C4funcE1SPS_+0x1d>
                        19: R_X86_64_PC32       _D4test8__assertFiZv-0x4
  1d:   48 83 7d 10 00          cmpq   $0x0,0x10(%rbp)
  22:   75 0a                   jne    2e <_ZN1C4funcE1SPS_+0x2e>
  24:   bf 08 00 00 00          mov    $0x8,%edi
  29:   e8 00 00 00 00          callq  2e <_ZN1C4funcE1SPS_+0x2e>
                        2a: R_X86_64_PC32       _D4test8__assertFiZv-0x4
  2e:   83 7d 18 00             cmpl   $0x0,0x18(%rbp)
  32:   75 0a                   jne    3e <_ZN1C4funcE1SPS_+0x3e>
  34:   bf 09 00 00 00          mov    $0x9,%edi
  39:   e8 00 00 00 00          callq  3e <_ZN1C4funcE1SPS_+0x3e>
                        3a: R_X86_64_PC32       _D4test8__assertFiZv-0x4
  3e:   c9                      leaveq
  3f:   c3                      retq

It works correctly if StructDeclaration::isPOD is changed to not reject all structs with constructors, but there is a comment warning against that.

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740


Iain Buclaw <ibuclaw@ubuntu.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ibuclaw@ubuntu.com


--- Comment #1 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-12-15 13:48:44 PST ---
Errm...

This is working correctly as designed. Structs with constructors are considered non-POD, so they cannot be bit-copied in and out of registers.

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #2 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-12-15 14:14:12 PST ---
(In reply to comment #1)
> This is working correctly as designed. Structs with constructors are considered non-POD, so they cannot be bit-copied in and out of registers.


Maybe we should start considering whether structs are trivially copyable like C++ does.

eg:
StructDeclaration::isTriviallyCopyable()
if has no copy constructor
&& if has no postblit
&& if has a trivial destructor
&& if not nested inside another struct/class.

Which slightly differs from isPOD.

To compare behaviours, GDC should return from S ctor in memory, then pass via registers to C.func.  Call it a bug in GDC, but we only listen to isPOD for returning structs, not passing them. ;)

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #3 from yebblies <yebblies@gmail.com> 2013-12-16 15:52:29 EST ---
(In reply to comment #1)
> Errm...
> 
> This is working correctly as designed. Structs with constructors are considered non-POD, so they cannot be bit-copied in and out of registers.

No, it is not.  Constructors should not cause a struct to be considered non-POD
for extern(C++) functions.  I don't care what we do with extern(D) functions,
but we need to match the C++ compiler.

"If a C++ object has either a non-trivial copy constructor or a non-trivial destructor [11], it is passed by invisible reference (the object is replaced in the parameter list by a pointer that has class INTEGER)[12]"

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #4 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-12-16 01:38:55 PST ---
Exhibit A:
Constructors == non-POD in the frontend. This seems to be because of a
DMD-specific bug:

 * Note that D struct constructors can mean POD, since there is always default
 * construction with no ctor, but that interferes with OPstrpar which wants it
 * on the stack in memory, not in registers.

https://github.com/D-Programming-Language/dmd/blob/master/src/struct.c#L851


Exhibit B:
non-POD types are passed in memory according to toArgTypes:

https://github.com/D-Programming-Language/dmd/blob/master/src/argtypes.c#L286


Exhibit C:
core.stdc.stdarg handles this (condition at #L349 is false because of the
above) Though changing the behaviour should require no change to
core.stdc.stdarg:

https://github.com/D-Programming-Language/druntime/blob/master/src/core/stdc/stdarg.d#L443http://forum.dlang.org/post/513A33F5.6060205@digitalmars.com


Exhibit D:
<Walter> A non-POD cannot be in registers because its address gets taken for
constructors/destructors.

And he later asserts this in the thread.

http://forum.dlang.org/post/511FE6AB.60802@digitalmars.com


Exhibit E:
<Walter> If it lives in a register, then exception handling recovery won't work
on it.

http://forum.dlang.org/post/513A33F5.6060205@digitalmars.com



These considerations must be taken into account.

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740


yebblies <yebblies@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[64-bit] Struct with        |[64-bit] Struct with
                   |constructor incorrectly     |constructor incorrectly
                   |passed on stack             |passed on stack to
                   |                            |extern(C++) function


--- Comment #5 from yebblies <yebblies@gmail.com> 2013-12-16 22:35:56 EST ---
extern(C++) functions must do what the corresponding c++ compiler does.

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #6 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-12-16 06:01:42 PST ---
https://github.com/D-Programming-Language/dmd/pull/2975

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #7 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-12-21 08:11:07 PST ---
(In reply to comment #5)
> extern(C++) functions must do what the corresponding c++ compiler does.

The definition of POD:

A PODS type in C++ is defined as either a scalar type or a PODS class. A PODS class has no user-defined copy assignment operator, no user-defined destructor, and no non-static data members that are not themselves PODS. Moreover, a PODS class must be an aggregate, meaning it has no user-declared constructors, no private nor protected non-static data, no base classes and no virtual functions.



So if g++ treats structs as POD if there is a user defined constructor, then that would be a problem - as it should not be treated it as POD, empty or non-empty.

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #8 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-12-21 08:17:26 PST ---
(In reply to comment #7)
> (In reply to comment #5)
> > extern(C++) functions must do what the corresponding c++ compiler does.
> 
> The definition of POD:
> 

Also in written here: http://www.parashift.com/c++-faq-lite/pod-types.html

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

https://d.puremagic.com/issues/show_bug.cgi?id=11740



--- Comment #9 from yebblies <yebblies@gmail.com> 2013-12-22 03:23:12 EST ---
(In reply to comment #7)
> (In reply to comment #5)
> > extern(C++) functions must do what the corresponding c++ compiler does.
> 
> The definition of POD:
> 
> A PODS type in C++ is defined as either a scalar type or a PODS class. A PODS class has no user-defined copy assignment operator, no user-defined destructor, and no non-static data members that are not themselves PODS. Moreover, a PODS class must be an aggregate, meaning it has no user-declared constructors, no private nor protected non-static data, no base classes and no virtual functions.
> 
> 
> 
> So if g++ treats structs as POD if there is a user defined constructor, then that would be a problem - as it should not be treated it as POD, empty or non-empty.

It really doesn't matter if C++ thinks it's not a POD, or D thinks it's not a POD.  The System V ABI considers it a POD, and we need to match it.

Again, http://www.x86-64.org/documentation/abi.pdf
"If a C++ object has either a non-trivial copy constructor or a non-trivial
destructor [11], it is passed by invisible reference (the object is replaced in
the parameter list by a pointer that has class INTEGER)[12]"

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