On Tuesday, 27 August 2013 at 16:57:36 UTC, H. S. Teoh wrote:
> On Tue, Aug 27, 2013 at 05:46:40PM +0200, deadalnix wrote:
>> On Tuesday, 27 August 2013 at 14:26:53 UTC, H. S. Teoh wrote:
>> >What if move(r2) throws? Then res1 won't get cleaned up, because r1
>> >has already been nulled by the move.
>> >
>> 
>> I don't think move can throw.
>
> Well, it's not marked nothrow, so I wouldn't count on that.
>
> Also, the fact that move() uses memcpy is a bit worrying; Adam Ruppe &
> myself ran into a nasty bug involving closures over struct members when
> the struct may get moved upon return from a function. For example:
>
> 	struct S {
> 		int id;
> 		void delegate()[] cleanups;
>
> 		this() {
> 			id = acquireResource();
> 			cleanups ~= {
> 				// N.B.: closure over this.id
> 				releaseResource(id);
> 			};
> 		}
> 	}
>
> 	S makeS() {
> 		// Problem: S.cleanups[0] is a closure over the struct
> 		// instance on this function's stack, but once S is
> 		// returned, it gets memcpy'd into the caller's stack
> 		// frame. This invalidates the delegate's context
> 		// pointer.
> 		return S(1);
> 	}
>
> 	void main() {
> 		auto s = makeS();
> 		// Problem: s.cleanups[0] now has an invalid context
> 		// pointer. If the stack is reused after this point, the
> 		// dtor of s will get a garbage value for s.id.
> 	}
>
> Using move() to move a resource from a local variable into a member
> looks like it might be vulnerable to this bug as well -- if the resource
> has closures over member variables it might trigger this problem.
>
>
> T

Funny, I ran into this twice this week XD

struct are movable by definition, so the compiler should reject this unless the delegate is scope.

On Wed, Aug 28, 2013 at 02:13:39AM +0200, deadalnix wrote:
> On Tuesday, 27 August 2013 at 16:57:36 UTC, H. S. Teoh wrote:
[...]
> >Also, the fact that move() uses memcpy is a bit worrying; Adam Ruppe & myself ran into a nasty bug involving closures over struct members when the struct may get moved upon return from a function. For example:
> >
> >	struct S {
> >		int id;
> >		void delegate()[] cleanups;
> >
> >		this() {
> >			id = acquireResource();
> >			cleanups ~= {
> >				// N.B.: closure over this.id
> >				releaseResource(id);
> >			};
> >		}
> >	}
> >
> >	S makeS() {
> >		// Problem: S.cleanups[0] is a closure over the struct
> >		// instance on this function's stack, but once S is
> >		// returned, it gets memcpy'd into the caller's stack
> >		// frame. This invalidates the delegate's context
> >		// pointer.
> >		return S(1);
> >	}
> >
> >	void main() {
> >		auto s = makeS();
> >		// Problem: s.cleanups[0] now has an invalid context
> >		// pointer. If the stack is reused after this point, the
> >		// dtor of s will get a garbage value for s.id.
> >	}
> >
> >Using move() to move a resource from a local variable into a member looks like it might be vulnerable to this bug as well -- if the resource has closures over member variables it might trigger this problem.
> >
> >
> >T
> 
> Funny, I ran into this twice this week XD
> 
> struct are movable by definition, so the compiler should reject this unless the delegate is scope.

Is scope even implemented right now? :-/ It's one of those things that are really, really nice to have, but so far haven't materialized yet.


T

-- 
Guns don't kill people. Bullets do.