Works with dmd-2.059. Does not work with the current github snapshot and with dmd-2.060.
Strangely, if the "this" function is not defined for the Foo struct. The code works fine -- no segfault. Alternatively if I store the value returned by alomicLoad in a local variable before returning it from the function foo, the code still runs fine.
// minimal code that gives segfault
import core.atomic;
struct Foo {
private ulong _s = 0;
public this (ulong s) { // no segfault if this function is commented out
this._s = s;
}
}
auto foo () {
shared static Foo bar;
// shared frop = atomicLoad(bar);
// return frop; // no segfault
return atomicLoad(bar); // segfaults here
}
void main() {
foo();
}
Comment #1 by puneet — 2012-11-30T06:48:25Z
This is related. The result printed out by the code on my Ubuntu amd64 machine is
42
4294967296
Upto version dmd-2.059, it correctly prints 42 and 42.
When the "this" constructor for Foo is commented out again the results are correctly printed out as 42 and 42.
//
import core.atomic;
struct Foo {
public ulong _s = 0;
// atomicLoad gives out wrong value when this (constructor) is defined
public this (ulong s) {
this._s = s;
}
}
auto foo () {
import std.stdio;
shared static Foo bar;
bar._s = 42;
writeln(bar._s);
atomicStore(bar, bar);
shared Foo frop = atomicLoad(bar);
writeln(frop._s);
return frop;
}
void main() {
foo();
}
Comment #2 by bugzilla — 2012-12-26T02:52:19Z
The problem is that atomicLoad() only works with POD (Plain Old Data) types. Structs with constructors are not POD. Non-POD structs are returned with a different calling convention than POD, hence the crash.
The fix is for atomicLoad() to reject attempts to instantiate it with non-POD types.
The test case here should fail to compile.
Comment #3 by bugzilla — 2012-12-26T02:58:38Z
Unfortunately, there is no convenient isPOD trait detection, so I'm going to defer this for now.
Comment #4 by dmitry.olsh — 2012-12-27T04:39:38Z
(In reply to comment #2)
> The problem is that atomicLoad() only works with POD (Plain Old Data) types.
> Structs with constructors are not POD.
It used to work.
> Non-POD structs are returned with a
> different calling convention than POD, hence the crash.
>
Then the definition of POD changed?
And besides what's the difference for the atomic load? AFAICT in D all structs are PODs unless there is a postblit. There is no virtual table nor inheritance. Even C++11 extends POD definition to include member functions. Why should we go the opposite direction?
> The fix is for atomicLoad() to reject attempts to instantiate it with non-POD
> types.
>
Even if there was a clear definition of it in D...
> The test case here should fail to compile.
... I expect atomicLoad to work with anything suitably sized w/o postblits and opAssign.
Comment #5 by bugzilla — 2012-12-28T16:10:42Z
atomicLoad()'s implementation assumes that 8 byte structs are returned in EDX:EAX. This is not correct for non-POD structs, which are always returned via a hidden pointer to the return result on the caller's stack.
There is no way for atomicLoad() to detect this.
See Issue 9237.
Comment #6 by github-bugzilla — 2012-12-29T23:22:22Z