Hi David,<br><br>Thanks a lot for the review, your feedback is very valuable! We will apply it and resubmit the patch as soon as possible.<div><br></div><div>Two of your points seem to be more challenging to me to address, given my limited experience with the libc internals:</div><div><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">================<br>
Comment at: lib/safestack/safestack.cc:75<br>
@@ +74,3 @@<br>
+<br>
+// The following locations are platform-specific<br>
+# define __GET_UNSAFE_STACK_PTR()         (void*) __THREAD_GETMEM_L(0x280)<br>
----------------<br>
There are magic numbers here again.  Please reference either an ABI document or equivalent that indicates that these are safe locations to use.<br></blockquote><div><br></div><div>Do you know of any ABI documents that might be relevant to this? On FreeBSD, we simply added corresponding fields to the tcb struct in libc (in lib/libthr/arch/amd64/include/pthread_md.h). Do you think it would be safer to use thread-local variables on Linux for now ? It would impact the performance though. Perhaps in the future the glibc can be modified to add the required fields to its tcb structure as well.</div>







<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: lib/safestack/safestack.cc:255<br>
@@ +254,3 @@<br>
+void thread_cleanup_handler(void* _iter) {<br>
+  // We want to free the unsafe stack only after all other destructors<br>
+  // have already run. We force this function to be called multiple times.<br>
----------------<br>
This seems somewhat fragile.  It doesn't guarantee that the other destructors won't be run after this, only that they get several opportunities to run before.<br></blockquote><div><br></div><div>Yes, I was worried about this as well. It worked in our tests, but can certainly break in some cases. The clean solution is to perform this cleanup in the pthread code itself, which is what we did on FreeBSD. We hope it will eventually be added to glibc as well. Do you have any better suggestions on how to handle this in the meantime?</div><div><br></div><div>Thanks!</div><div>- Vova</div></div>