http://d.puremagic.com/issues/show_bug.cgi?id=9099

           Summary: segfault while returning an atomicLoad value
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: regression
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: puneet@coverify.org


--- Comment #0 from Puneet Goel <puneet@coverify.org> 2012-11-30 01:44:50 PST ---
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();
}

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099



--- Comment #1 from Puneet Goel <puneet@coverify.org> 2012-11-30 06:48:25 PST ---
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();
}

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla@digitalmars.com
          Component|DMD                         |druntime


--- Comment #2 from Walter Bright <bugzilla@digitalmars.com> 2012-12-26 02:52:19 PST ---
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.

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099



--- Comment #3 from Walter Bright <bugzilla@digitalmars.com> 2012-12-26 02:58:38 PST ---
Unfortunately, there is no convenient isPOD trait detection, so I'm going to defer this for now.

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099


Dmitry Olshansky <dmitry.olsh@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dmitry.olsh@gmail.com


--- Comment #4 from Dmitry Olshansky <dmitry.olsh@gmail.com> 2012-12-27 04:39:38 PST ---
(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.

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099



--- Comment #5 from Walter Bright <bugzilla@digitalmars.com> 2012-12-28 16:10:42 PST ---
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.

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099



--- Comment #6 from github-bugzilla@puremagic.com 2012-12-29 23:22:22 PST ---
Commit pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/a15d228ed0c1a3bfb5bd05ec1d491802bb29aac0
fix
Issue 9099 - core.atomic.atomicLoad() cannot handle non-POD structs

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099



--- Comment #7 from github-bugzilla@puremagic.com 2012-12-29 23:31:44 PST ---
Commit pushed to staging at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/878a183a51825ea5dce25f4289a8ef001f6eeea1 fix Issue 9099 - core.atomic.atomicLoad() cannot handle non-POD structs

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

http://d.puremagic.com/issues/show_bug.cgi?id=9099


Walter Bright <bugzilla@digitalmars.com> changed:

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


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

http://d.puremagic.com/issues/show_bug.cgi?id=9099



--- Comment #8 from github-bugzilla@puremagic.com 2013-01-02 01:40:13 PST ---
Commit pushed to staging at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/a15d228ed0c1a3bfb5bd05ec1d491802bb29aac0 fix

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