[llvm] r190730 - Make PrettyStackTraceEntry use ManagedStatic for its ThreadLocal.

[Filip Pizlo]

On Sep 13, 2013, at 3:59 PM, Filip Pizlo <fpizlo at apple.com> wrote:

> Author: fpizlo
> Date: Fri Sep 13 17:59:47 2013
> New Revision: 190730
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=190730&view=rev
> Log:
> Make PrettyStackTraceEntry use ManagedStatic for its ThreadLocal.
> 
> This was somewhat tricky because ~PrettyStackTraceEntry() may run after 
> llvm_shutdown() has been called. This is rare and only happens for a common idiom 
> used in the main() functions of command-line tools. This works around the idiom by 
> skipping the stack clean-up if the PrettyStackTraceHead ManagedStatic is not 
> constructed (i.e. llvm_shutdown() has been called).
> 
> 
> Modified:
>    llvm/trunk/include/llvm/Support/ManagedStatic.h
>    llvm/trunk/lib/Support/PrettyStackTrace.cpp
> 
> Modified: llvm/trunk/include/llvm/Support/ManagedStatic.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/ManagedStatic.h?rev=190730&r1=190729&r2=190730&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/ManagedStatic.h (original)
> +++ llvm/trunk/include/llvm/Support/ManagedStatic.h Fri Sep 13 17:59:47 2013
> @@ -99,7 +99,6 @@ public:
> /// llvm_shutdown - Deallocate and destroy all ManagedStatic variables.
> void llvm_shutdown();
> 
> -

Eek!  Sorry about that. :-/

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?

-Filip


> /// llvm_shutdown_obj - This is a simple helper class that calls
> /// llvm_shutdown() when it is destroyed.
> struct llvm_shutdown_obj {
> 
> Modified: llvm/trunk/lib/Support/PrettyStackTrace.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=190730&r1=190729&r2=190730&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Support/PrettyStackTrace.cpp (original)
> +++ llvm/trunk/lib/Support/PrettyStackTrace.cpp Fri Sep 13 17:59:47 2013
> @@ -15,6 +15,7 @@
> #include "llvm/Support/PrettyStackTrace.h"
> #include "llvm/ADT/SmallString.h"
> #include "llvm/Config/config.h"     // Get autoconf configuration settings
> +#include "llvm/Support/ManagedStatic.h"
> #include "llvm/Support/Signals.h"
> #include "llvm/Support/ThreadLocal.h"
> #include "llvm/Support/Watchdog.h"
> @@ -30,8 +31,7 @@ namespace llvm {
>   bool DisablePrettyStackTrace = false;
> }
> 
> -// FIXME: This should be thread local when llvm supports threads.
> -static sys::ThreadLocal<const PrettyStackTraceEntry> PrettyStackTraceHead;
> +static ManagedStatic<sys::ThreadLocal<const PrettyStackTraceEntry> > PrettyStackTraceHead;
> 
> static unsigned PrintStack(const PrettyStackTraceEntry *Entry, raw_ostream &OS){
>   unsigned NextID = 0;
> @@ -49,12 +49,12 @@ static unsigned PrintStack(const PrettyS
> /// PrintCurStackTrace - Print the current stack trace to the specified stream.
> static void PrintCurStackTrace(raw_ostream &OS) {
>   // Don't print an empty trace.
> -  if (PrettyStackTraceHead.get() == 0) return;
> +  if (PrettyStackTraceHead->get() == 0) return;
> 
>   // If there are pretty stack frames registered, walk and emit them.
>   OS << "Stack dump:\n";
> 
> -  PrintStack(PrettyStackTraceHead.get(), OS);
> +  PrintStack(PrettyStackTraceHead->get(), OS);
>   OS.flush();
> }
> 
> @@ -114,14 +114,26 @@ PrettyStackTraceEntry::PrettyStackTraceE
>   (void)HandlerRegistered;
> 
>   // Link ourselves.
> -  NextEntry = PrettyStackTraceHead.get();
> -  PrettyStackTraceHead.set(this);
> +  NextEntry = PrettyStackTraceHead->get();
> +  PrettyStackTraceHead->set(this);
> }
> 
> PrettyStackTraceEntry::~PrettyStackTraceEntry() {
> -  assert(PrettyStackTraceHead.get() == this &&
> +  // Do nothing if PrettyStackTraceHead is uninitialized. This can only happen
> +  // if a shutdown occurred after we created the PrettyStackTraceEntry. That
> +  // does occur in the following idiom:
> +  //
> +  // PrettyStackTraceProgram X(...);
> +  // llvm_shutdown_obj Y;
> +  //
> +  // Without this check, we may end up removing ourselves from the stack trace
> +  // after PrettyStackTraceHead has already been destroyed.
> +  if (!PrettyStackTraceHead.isConstructed())
> +    return;
> +  
> +  assert(PrettyStackTraceHead->get() == this &&
>          "Pretty stack trace entry destruction is out of order");
> -  PrettyStackTraceHead.set(getNextEntry());
> +  PrettyStackTraceHead->set(getNextEntry());
> }
> 
> void PrettyStackTraceString::print(raw_ostream &OS) const {
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130913/872e91fa/attachment.html>