Bug 19179 – extern(C++) small-struct by-val uses wrong ABI

Status
RESOLVED
Resolution
INVALID
Severity
blocker
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
Windows
Creation time
2018-08-19T18:51:51Z
Last change time
2020-11-10T09:11:17Z
Keywords
backend, C++, industry, pull, wrong-code
Assigned to
No Owner
Creator
Manu
See also
https://issues.dlang.org/show_bug.cgi?id=5570

Comments

Comment #0 by turkeyman — 2018-08-19T18:51:51Z
test.d ------ import core.stdc.stdio; extern(C++) struct SmallStruct { int x = 10, y = 20; } SmallStruct test_small(SmallStruct s) { printf("%d %d\n", s.x, s.y); // prints: invalid memory return s; } void test_small_noret(SmallStruct s) { printf("%d %d\n", s.x, s.y); // prints: 10 20 } test.cpp -------- struct SmallStruct { int x = 10, y = 20; }; SmallStruct test_small(SmallStruct); void test_small_noret(SmallStruct); void main() { test_small(SmallStruct()); test_small_noret(SmallStruct()); } Tested with VS2015 - x86 and x64 Note: if you add one more `int` to the struct, this code works as expected.
Comment #1 by turkeyman — 2018-08-19T18:57:31Z
Oops, my cut&paste removed the `extern(C++):` at the top of the D source. Those 2 test functions should be extern(C++)!
Comment #2 by look.at.me.pee.please — 2018-09-10T02:50:59Z
Can't reproduce with your example, I've come across something similar in the past though. If you swap your struct with this one: struct SmallStruct { int x, y; SmallStruct() { x = 10; y = 20; } }; static_assert(!std::is_pod<SmallStruct>::value, ""); // not a POD, your other struct is a POD though the D version will be POD though. But since you can't have a default constructor in D, you have to use another method to make it not POD. That is give it a destructor "~this()" or a copy constructor "this(this)". Windows treats structs that aren't POD differently if they are less than 8 bytes. That's the root of the problem. A solution would be to add an attribute "@NotPOD struct blah { }" or something, rather than having to put an empty constructor. Not exactly something that is easy to notify the user of if this inconsistency exists.
Comment #3 by turkeyman — 2019-03-04T03:25:02Z
I think what should probably happen here, is that while the type in D is POD, the counterpart in C++ is not POD because default initialisation generates a constructor. Perhaps the proper solution is to make the D struct emit a default constructor (this just assign's init), and also force the type to use the non-POD ABI? This way semantics will match C++, and if the C++ class externs to the default constructor; it will link as they expect.
Comment #4 by dlang-bot — 2020-09-06T08:46:53Z
@WalterBright created dlang/dmd pull request #11698 "fix Issue 19179 - extern(C++) small-struct by-val uses wrong ABI" fixing this issue: - fix Issue 19179 - extern(C++) small-struct by-val uses wrong ABI https://github.com/dlang/dmd/pull/11698
Comment #5 by dlang-bot — 2020-09-08T00:39:42Z
dlang/dmd pull request #11698 "fix Issue 19179 - extern(C++) small-struct by-val uses wrong ABI" was merged into master: - b6f570491c6972aa865d1f2e6e19535127a58a06 by Walter Bright: fix Issue 19179 - extern(C++) small-struct by-val uses wrong ABI https://github.com/dlang/dmd/pull/11698
Comment #6 by look.at.me.pee.please — 2020-09-20T13:57:22Z
Please read the comments. If you had, you'd have known that the test Manu gave didn't contain the bug.
Comment #7 by bugzilla — 2020-11-10T09:11:17Z
Default initialization in C++ doesn't make it non-POD. And I added the `extern(C++)` to the test cases that Manu had omitted. Is the actual problem trying to create a non-POD struct in D that is compatible with a non-POD struct in C++? As to how to make a POD in D without a default constructor, just add a trivial postblit: this(this) { } and it will be not POD. If there's still a problem, please file a new report explaining what that is rather than reopening this as a different issue.