www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 11740] New: [64-bit] Struct with constructor incorrectly passed on stack

reply d-bugmail puremagic.com writes:
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: -------
Dec 14 2013
next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 15 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 15 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 15 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 16 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 16 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 16 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 21 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 21 2013
prev sibling parent d-bugmail puremagic.com writes:
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: -------
Dec 21 2013