sociomantic-tsunami / dhtproto

Distributed Hash Table protocol definition, client, fake node, and tests
Boost Software License 1.0
5 stars 22 forks source link

Swarm neo requests use an escaping reference to a stack variable #226

Closed don-clugston-sociomantic closed 4 years ago

don-clugston-sociomantic commented 4 years ago

When compiled with -dip25, dmd give an alarming warning:

./submodules/swarm/src/swarm/neo/connection/RequestOnConnBase.d(416): Deprecation: copying (cast(const(void*))&elem)[0..4] into allocated memory escapes a reference to parameter variable elem

The code is:

            public void add ( T ) ( ref T elem )
            {
                static assert(!hasIndirections!(T));
                this.outer.outer.send_payload ~= (cast(const(void*))&elem)[0..T.sizeof];
            }

And actually I'm horrified that such a nasty bit of code has no comments. Now look at what this is doing. Suppose T is an int. This creates a void[] slice of the raw bytes of elem, and appends them to send_payload. That's a hacky way of serializing but OK.

But here's the problem. send_payload is not an array of bytes! It is actually:

         private const(void)[][] send_payload;

So though it looks as though it appends the bytes, that's not what it's doing. It's appending the slices. Ie it is saving pointers to stack variables.

Now it looks as though in practice, the send_payload array is used immediately after being set, and then the array is cleared. However, in most cases, the parent class, and hence the array itself, has a lifetime which is longer than the stack variables which it points to. It's possible that all extant uses are correct in the sense that they avoid memory corruption. But it all seems rather fragile.

I am not sure how to deal with this.

don-clugston-sociomantic commented 4 years ago

Oops, meant to open this as a swarm issue.