Issue 19877 - [dip1000] std.container.rbtree is unsafely accessing private data
Summary: [dip1000] std.container.rbtree is unsafely accessing private data
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: druntime (show other issues)
Version: D2
Hardware: All All
: P1 major
Assignee: No Owner
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2019-05-15 18:12 UTC by hsteoh
Modified: 2022-04-03 08:33 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description hsteoh 2019-05-15 18:12:42 UTC
Code:
------
import std.container.rbtree;

alias Grid = RedBlackTree!(GridPoint);

struct GridPoint
{
    private string _srcStr;
    int opCmp(in GridPoint p) const { return 0; }
}
------

Compiler output (with -preview=dip1000):
------
/usr/src/d/phobos/std/container/rbtree.d(1111): Error: @safe function std.container.rbtree.RedBlackTree!(GridPoint, "a < b", false).RedBlackTree.toHash cannot call @system function core.internal.hash.hashOf!(GridPoint).hashOf
/usr/src/d/druntime/import/core/internal/hash.d(510):        core.internal.hash.hashOf!(GridPoint).hashOf is declared here
numid.d(3): Error: template instance `std.container.rbtree.RedBlackTree!(GridPoint, "a < b", false)` error instantiating
------

Compiling without -preview=dip1000 works.

The problem appears to be the 'private': removing 'private' from _srcStr makes the problem go away.

Why 'private' makes toHash un-@safe is anybody's guess.
Comment 1 hsteoh 2019-05-16 00:54:32 UTC
Here's a much more reduced case:
------
struct S {
        private int _x;
}
struct RedBlackTree
{
    size_t toHash() nothrow @safe
    {
        return .hashOf(S.init);
    }
}
void main() { }
------
Comment 2 Walter Bright 2020-03-12 09:31:34 UTC
The cause of this is core.internal.hash.hashOf() is getting its attributes inferred. Having it access private members of a struct causes it to be inferred as @system, and @safe RedBlackTree.toHash() is not allowed to call an @system function.

The problem is in std.container.rbtree, where the call to .toHash() should be put in an @trusted block.
Comment 3 Ate Eskola 2021-06-07 10:40:12 UTC
I'd argue it behaves correctly now. Private state should be private. If `.hashOf`, or any other external funtion, needs to access it directly, it ought to fail. The correct thing to do is to add `toHash` member function to the element `struct`.

Current `-dip1000` prevents errors like this:

```
struct Ternary
{ private ubyte state;
  @safe auto opEquals(const Ternary rhs) const
  { if (state >= 2) return rhs.state >= 2;
    else return state == rhs.state;
  }
}

@safe void main()
{ Ternary a = void, b = void;
  //fails with high likelihood
  if(a == b) assert(.hashOf(a) == .hashOf(b), "WAT?");
}
```

Perhaps the error message should suggest adding the `toHash` function, though.
Comment 4 Dennis 2022-04-03 08:33:55 UTC
Fixed by https://github.com/dlang/dmd/pull/13522