D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 15399 - unaligned pointers are not @safe
Summary: unaligned pointers are not @safe
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 major
Assignee: No Owner
URL:
Keywords: accepts-invalid, safe
Depends on:
Blocks:
 
Reported: 2015-12-03 00:21 UTC by Vladimir Panteleev
Modified: 2018-03-12 10:07 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Vladimir Panteleev 2015-12-03 00:21:32 UTC
This @safe program produces dangling GC pointers by storing the only references to them in an unaligned struct field:

//////////////////////////////// test.d ////////////////////////////////
@safe:

struct Victim
{
    bool alive = true;
    ~this() { alive = false; }
}

align(1)
struct Unaligned
{
align(1):
    ubyte filler;
    Victim* p;
}

pragma(msg, Unaligned.sizeof);

void main()
{
    enum N = 100;

    Unaligned[N] hosts;

    foreach (n; 0..N)
    {
        hosts[n].p = new Victim;
        assert(hosts[n].p.alive);
    }

    // Unaligned.p is invisible to the GC due to alignment

    void trustedCollect() @trusted { import core.memory; GC.collect(); }
    trustedCollect();

    foreach (n; 0..N)
        assert(hosts[n].p.alive); // Dangling pointer!
}
////////////////////////////////////////////////////////////////////////
Comment 1 yebblies 2015-12-15 02:07:14 UTC
One option is to ban unaligned pointer members altogether.  It could still be done by casting to size_t, so I don't think we'd be losing much.
Comment 2 Walter Bright 2016-06-08 09:36:39 UTC
https://github.com/dlang/dmd/pull/5852
Comment 3 github-bugzilla 2016-06-18 07:59:48 UTC
Commits pushed to master at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/a294c6e774fb2d0b40b453f2ff9896b7f99e027f
fix Issue 15399 - unaligned pointers are not @safe

https://github.com/dlang/dmd/commit/077a2d729149945ca7fc704423bdb3c9fca3ab50
Merge pull request #5852 from WalterBright/fix15399

fix Issue 15399 - unaligned pointers are not @safe
Comment 4 github-bugzilla 2016-10-01 11:47:51 UTC
Commits pushed to stable at https://github.com/dlang/dmd

https://github.com/dlang/dmd/commit/a294c6e774fb2d0b40b453f2ff9896b7f99e027f
fix Issue 15399 - unaligned pointers are not @safe

https://github.com/dlang/dmd/commit/077a2d729149945ca7fc704423bdb3c9fca3ab50
Merge pull request #5852 from WalterBright/fix15399
Comment 5 ag0aep6g 2017-09-05 15:46:35 UTC
Reopening because these very slight variations are still accepted:

1) hosts[n] = Unaligned(0, new Victim);
2) Unaligned u = { p: new Victim }; hosts[n] = u;
Comment 6 Walter Bright 2018-03-04 02:00:16 UTC
(In reply to ag0aep6g from comment #5)
> Reopening because these very slight variations are still accepted:
> 
> 1) hosts[n] = Unaligned(0, new Victim);
> 2) Unaligned u = { p: new Victim }; hosts[n] = u;

I don't know what that means. Can you please post a complete example?
Comment 7 ag0aep6g 2018-03-04 11:40:46 UTC
(In reply to Walter Bright from comment #6)
> (In reply to ag0aep6g from comment #5)
> > Reopening because these very slight variations are still accepted:
> > 
> > 1) hosts[n] = Unaligned(0, new Victim);
> > 2) Unaligned u = { p: new Victim }; hosts[n] = u;
> 
> I don't know what that means. Can you please post a complete example?


//////////////////////////////// test.d ////////////////////////////////
@safe:

struct Victim
{
    bool alive = true;
    ~this() { alive = false; }
}

align(1)
struct Unaligned
{
align(1):
    ubyte filler;
    Victim* p;
}

pragma(msg, Unaligned.sizeof);

void main()
{
    enum N = 100;

    Unaligned[N] hosts;

    foreach (n; 0..N)
    {
        version (original) hosts[n].p = new Victim;
        else version (variation1) hosts[n] = Unaligned(0, new Victim);
        else version (variation2)
        {
            Unaligned u = { p: new Victim };
            hosts[n] = u;
        }
        else static assert(false);
        assert(hosts[n].p.alive);
    }

    // Unaligned.p is invisible to the GC due to alignment

    void trustedCollect() @trusted { import core.memory; GC.collect(); }
    trustedCollect();

    foreach (n; 0..N)
        assert(hosts[n].p.alive); // Dangling pointer!
}
////////////////////////////////////////////////////////////////////////

These should all fail with the same error:

    dmd -version=original test.d
    dmd -version=variation1 test.d
    dmd -version=variation2 test.d

The different versions effectively do the same thing, just with different syntax.
Comment 8 Walter Bright 2018-03-12 09:55:21 UTC
(In reply to ag0aep6g from comment #5)
> Reopening because these very slight variations are still accepted:

Since the original bug was fixed, I'd prefer opening new bugs for new problems, not reopening fixed bugs with variations.
Comment 9 Walter Bright 2018-03-12 10:01:02 UTC
Closed and new bug created for variations:

 https://issues.dlang.org/show_bug.cgi?id=18597