<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Sep 13, 2013, at 3:59 PM, Filip Pizlo <<a href="mailto:fpizlo@apple.com">fpizlo@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Author: fpizlo<br>Date: Fri Sep 13 17:59:47 2013<br>New Revision: 190730<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=190730&view=rev">http://llvm.org/viewvc/llvm-project?rev=190730&view=rev</a><br>Log:<br>Make PrettyStackTraceEntry use ManagedStatic for its ThreadLocal.<br><br>This was somewhat tricky because ~PrettyStackTraceEntry() may run after<span class="Apple-converted-space"> </span><br>llvm_shutdown() has been called. This is rare and only happens for a common idiom<span class="Apple-converted-space"> </span><br>used in the main() functions of command-line tools. This works around the idiom by<span class="Apple-converted-space"> </span><br>skipping the stack clean-up if the PrettyStackTraceHead ManagedStatic is not<span class="Apple-converted-space"> </span><br>constructed (i.e. llvm_shutdown() has been called).<br><br><br>Modified:<br>   llvm/trunk/include/llvm/Support/ManagedStatic.h<br>   llvm/trunk/lib/Support/PrettyStackTrace.cpp<br><br>Modified: llvm/trunk/include/llvm/Support/ManagedStatic.h<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ManagedStatic.h?rev=190730&r1=190729&r2=190730&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ManagedStatic.h?rev=190730&r1=190729&r2=190730&view=diff</a><br>==============================================================================<br>--- llvm/trunk/include/llvm/Support/ManagedStatic.h (original)<br>+++ llvm/trunk/include/llvm/Support/ManagedStatic.h Fri Sep 13 17:59:47 2013<br>@@ -99,7 +99,6 @@ public:<br>/// llvm_shutdown - Deallocate and destroy all ManagedStatic variables.<br>void llvm_shutdown();<br><br>-<br></div></blockquote><div dir="auto"><br></div><div dir="auto">Eek!  Sorry about that. :-/</div><div dir="auto"><br></div><div dir="auto">Should I revert this change in a separate changeset or should I assume that the damage is already done and be more careful next time?</div><div dir="auto"><br></div><div dir="auto">-Filip</div><div dir="auto"><br></div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">/// llvm_shutdown_obj - This is a simple helper class that calls<br>/// llvm_shutdown() when it is destroyed.<br>struct llvm_shutdown_obj {<br><br>Modified: llvm/trunk/lib/Support/PrettyStackTrace.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=190730&r1=190729&r2=190730&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=190730&r1=190729&r2=190730&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Support/PrettyStackTrace.cpp (original)<br>+++ llvm/trunk/lib/Support/PrettyStackTrace.cpp Fri Sep 13 17:59:47 2013<br>@@ -15,6 +15,7 @@<br>#include "llvm/Support/PrettyStackTrace.h"<br>#include "llvm/ADT/SmallString.h"<br>#include "llvm/Config/config.h"     // Get autoconf configuration settings<br>+#include "llvm/Support/ManagedStatic.h"<br>#include "llvm/Support/Signals.h"<br>#include "llvm/Support/ThreadLocal.h"<br>#include "llvm/Support/Watchdog.h"<br>@@ -30,8 +31,7 @@ namespace llvm {<br>  bool DisablePrettyStackTrace = false;<br>}<br><br>-// FIXME: This should be thread local when llvm supports threads.<br>-static sys::ThreadLocal<const PrettyStackTraceEntry> PrettyStackTraceHead;<br>+static ManagedStatic<sys::ThreadLocal<const PrettyStackTraceEntry> > PrettyStackTraceHead;<br><br>static unsigned PrintStack(const PrettyStackTraceEntry *Entry, raw_ostream &OS){<br>  unsigned NextID = 0;<br>@@ -49,12 +49,12 @@ static unsigned PrintStack(const PrettyS<br>/// PrintCurStackTrace - Print the current stack trace to the specified stream.<br>static void PrintCurStackTrace(raw_ostream &OS) {<br>  // Don't print an empty trace.<br>-  if (PrettyStackTraceHead.get() == 0) return;<br>+  if (PrettyStackTraceHead->get() == 0) return;<br><br>  // If there are pretty stack frames registered, walk and emit them.<br>  OS << "Stack dump:\n";<br><br>-  PrintStack(PrettyStackTraceHead.get(), OS);<br>+  PrintStack(PrettyStackTraceHead->get(), OS);<br>  OS.flush();<br>}<br><br>@@ -114,14 +114,26 @@ PrettyStackTraceEntry::PrettyStackTraceE<br>  (void)HandlerRegistered;<br><br>  // Link ourselves.<br>-  NextEntry = PrettyStackTraceHead.get();<br>-  PrettyStackTraceHead.set(this);<br>+  NextEntry = PrettyStackTraceHead->get();<br>+  PrettyStackTraceHead->set(this);<br>}<br><br>PrettyStackTraceEntry::~PrettyStackTraceEntry() {<br>-  assert(PrettyStackTraceHead.get() == this &&<br>+  // Do nothing if PrettyStackTraceHead is uninitialized. This can only happen<br>+  // if a shutdown occurred after we created the PrettyStackTraceEntry. That<br>+  // does occur in the following idiom:<br>+  //<br>+  // PrettyStackTraceProgram X(...);<br>+  // llvm_shutdown_obj Y;<br>+  //<br>+  // Without this check, we may end up removing ourselves from the stack trace<br>+  // after PrettyStackTraceHead has already been destroyed.<br>+  if (!PrettyStackTraceHead.isConstructed())<br>+    return;<br>+  <br>+  assert(PrettyStackTraceHead->get() == this &&<br>         "Pretty stack trace entry destruction is out of order");<br>-  PrettyStackTraceHead.set(getNextEntry());<br>+  PrettyStackTraceHead->set(getNextEntry());<br>}<br><br>void PrettyStackTraceString::print(raw_ostream &OS) const {<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div><br></body></html>