tavianator added inline comments.

================
Comment at: src/cxa_thread_atexit.cpp:70
@@ +69,3 @@
+    while (auto head = dtors) {
+      dtors = head->next;
+      head->dtor(head->obj);
----------------
EricWF wrote:
> tavianator wrote:
> > EricWF wrote:
> > > There is a bug here. If `head->next == nullptr` and if 
> > > `head->dtor(head->obj))` creates a TL variable in the destructor then 
> > > that destructor will not be invoked.
> > > 
> > > Here's an updated test case which catches the bug: 
> > > https://gist.github.com/EricWF/3bb50d4f28b91aa28d2adefea0e94a0e
> > I can't reproduce that failure here, your exact test case passes (even with 
> > `#undef HAVE___CXA_THREAD_ATEXIT_IMPL` and the weak symbol test commented 
> > out).
> > 
> > Tracing the implementation logic, it seems correct.  If `head->next == 
> > nullptr` then this line does `dtors = nullptr`.  Then if 
> > `head->dtor(head->obj)` registers a new `thread_local`, 
> > `__cxa_thread_atexit()` does `head = malloc(...); ... dtors = head;`.  Then 
> > the next iteration of the loop `while (auto head = dtors) {` picks up that 
> > new node.
> > 
> > Have I missed something?
> I can't reproduce this morning either, I must have been doing something 
> funny. I'll look at this with a fresh head tomorrow. If I can't find anything 
> this will be good to go. Thanks for working on this. 
No problem!  I can integrate your updated test case anyway if you want.

https://reviews.llvm.org/D21803



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to