D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 20495 - std.range.choose range is not safe when calling save
Summary: std.range.choose range is not safe when calling save
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: All All
: P1 normal
Assignee: No Owner
URL:
Keywords: pull, safe
Depends on:
Blocks:
 
Reported: 2020-01-10 01:02 UTC by Steven Schveighoffer
Modified: 2020-01-10 05:45 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Steven Schveighoffer 2020-01-10 01:02:44 UTC
Currently, the range passes a copy of the unused union member to the saved version. Although this might seem innocuous, it's possible the postblit or copy constructor uses the existing data, which is actually the contents of the other range.

Hard to come up with an example due to how most ranges work, but:

import std.range;

struct KillableRange
{
    @safe:
    int *item;
    ref int front() {return *item; }
    bool empty() { return *item > 10; }
    void popFront() { ++(*item); }
    this(this)
    {
        assert(item is null || cast(size_t)item > 1000);
        item = new int(*item);
    }
    KillableRange save() { return this; }
}

void main() @safe
{
    auto kr = KillableRange(new int(1));
    int[] x = [1,2,3,4,5]; // length is first
    
    auto chosen = choose(true, x, kr);
    auto chosen2 = chosen.save;
}

This will assert because x.length overlaps kr.item when save is called.

The correct mechanism is to use .init on the non-saved range.

Something with RefCounted as implementation would easily be a good use case to show problems here, but it's harder to test without segfaulting.
Comment 1 Dlang Bot 2020-01-10 01:22:42 UTC
@schveiguy created dlang/phobos pull request #7347 "Fix issue 20495 (choose copies unused union member, which is unsafe)" fixing this issue:

- Fix issue 20495. Fixes safety of save function by passing init value of
  unused union member to ctor. Also simplify the @safe inference.

https://github.com/dlang/phobos/pull/7347
Comment 2 Dlang Bot 2020-01-10 05:45:32 UTC
dlang/phobos pull request #7347 "Fix issue 20495 (choose copies unused union member, which is unsafe)" was merged into stable:

- 857543416dabea4c5b53867b908a1e103a4580a1 by Steven Schveighoffer:
  Fix issue 20495. Fixes safety of save function by passing init value of
  unused union member to ctor. Also simplify the @safe inference.

https://github.com/dlang/phobos/pull/7347