[PATCH] D15380: [tsan] Update dispatch_group support to avoid using a disposed group object

[Kuba Brecka via llvm-commits]

kubabrecka added inline comments.

================
Comment at: lib/tsan/rtl/tsan_libdispatch_mac.cc:93
@@ -90,2 +92,3 @@
   user_free(thr, pc, context);
+  if (context->object_to_release) dispatch_release(context->object_to_release);
 }
----------------
dvyukov wrote:
> dvyukov wrote:
> > dvyukov wrote:
> > > Looks like use-after-free. You've just freed the context line above.
> > Please add a comment somewhere here explaining why we need this additional lifetime management.
> I don't understand the exact scenario that leads to the problem. The group is context->object_to_acquire, which we touch in the very beginning of this function. Then we call user callback orig_work. In your tests orig_work calls dispatch_group_leave on the group. So either (1) test code is also buggy, or (2) we don't have use-after-free in this function and don't need this change, or (3) I am missing something.
> 
> In your tests orig_work calls dispatch_group_leave
No, orig_work here is the empty block (or empty function) passed as argument to dispatch_group_notify (or dispatch_group_notify_f).  The issue is that the API doesn't guarantee that the group itself will still be alive, when the notification is invoked.

> Looks like use-after-free. You've just freed the context line above.
Oops, you're right.


http://reviews.llvm.org/D15380