[llvm] r270903 - Don't use recursion to print out the PrettyStackTrace after a crash. If the

[Sean Silva via llvm-commits]

On Thu, May 26, 2016 at 1:21 PM, Richard Smith via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rsmith
> Date: Thu May 26 15:21:55 2016
> New Revision: 270903
>
> URL: http://llvm.org/viewvc/llvm-project?rev=270903&view=rev
> Log:
> Don't use recursion to print out the PrettyStackTrace after a crash. If the
> crash was due to a stack overflow, chances are good that this would also
> cause
> a stack overflow.
>
> Modified:
>     llvm/trunk/include/llvm/Support/PrettyStackTrace.h
>     llvm/trunk/lib/Support/PrettyStackTrace.cpp
>
> Modified: llvm/trunk/include/llvm/Support/PrettyStackTrace.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/PrettyStackTrace.h?rev=270903&r1=270902&r2=270903&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Support/PrettyStackTrace.h (original)
> +++ llvm/trunk/include/llvm/Support/PrettyStackTrace.h Thu May 26 15:21:55
> 2016
> @@ -29,9 +29,11 @@ namespace llvm {
>    /// constructed and destructed, they will add their symbolic frames to a
>    /// virtual stack trace.  This gets dumped out if the program crashes.
>    class PrettyStackTraceEntry {
> -    const PrettyStackTraceEntry *NextEntry;
> +    friend PrettyStackTraceEntry *ReverseStackTrace(PrettyStackTraceEntry
> *);
> +
> +    PrettyStackTraceEntry *NextEntry;
>      PrettyStackTraceEntry(const PrettyStackTraceEntry &) = delete;
> -    void operator=(const PrettyStackTraceEntry&) = delete;
> +    void operator=(const PrettyStackTraceEntry &) = delete;
>    public:
>      PrettyStackTraceEntry();
>      virtual ~PrettyStackTraceEntry();
> @@ -67,7 +69,7 @@ namespace llvm {
>    };
>
>    /// Returns the topmost element of the "pretty" stack state.
> -  const void* SavePrettyStackState();
> +  const void *SavePrettyStackState();
>
>    /// Restores the topmost element of the "pretty" stack state to State,
> which
>    /// should come from a previous call to SavePrettyStackState().  This is
> @@ -76,7 +78,7 @@ namespace llvm {
>    /// happens after a crash that's been recovered by CrashRecoveryContext
>    /// doesn't have frames on it that were added in code unwound by the
>    /// CrashRecoveryContext.
> -  void RestorePrettyStackState(const void* State);
> +  void RestorePrettyStackState(const void *State);
>
>  } // end namespace llvm
>
>
> Modified: llvm/trunk/lib/Support/PrettyStackTrace.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=270903&r1=270902&r2=270903&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Support/PrettyStackTrace.cpp (original)
> +++ llvm/trunk/lib/Support/PrettyStackTrace.cpp Thu May 26 15:21:55 2016
> @@ -21,6 +21,8 @@
>  #include "llvm/Support/Watchdog.h"
>  #include "llvm/Support/raw_ostream.h"
>
> +#include <tuple>
> +
>  #ifdef HAVE_CRASHREPORTERCLIENT_H
>  #include <CrashReporterClient.h>
>  #endif
> @@ -36,20 +38,32 @@ using namespace llvm;
>  // objects, but we *really* cannot tolerate destructors running and do
> not want
>  // to pay any overhead of synchronizing. As a consequence, we use a raw
>  // thread-local variable.
> -static LLVM_THREAD_LOCAL const PrettyStackTraceEntry
> *PrettyStackTraceHead =
> -    nullptr;
> +static LLVM_THREAD_LOCAL PrettyStackTraceEntry *PrettyStackTraceHead =
> nullptr;
>
> -static unsigned PrintStack(const PrettyStackTraceEntry *Entry,
> raw_ostream &OS){
> -  unsigned NextID = 0;
> -  if (Entry->getNextEntry())
> -    NextID = PrintStack(Entry->getNextEntry(), OS);
> -  OS << NextID << ".\t";
> -  {
> +namespace llvm {
> +PrettyStackTraceEntry *ReverseStackTrace(PrettyStackTraceEntry *Head) {
> +  PrettyStackTraceEntry *Prev = nullptr;
> +  while (Head)
> +    std::tie(Prev, Head, Head->NextEntry) =
> +        std::make_tuple(Head, Head->NextEntry, Prev);
>

Neat trick! Reminds me of the `std::tie(LHS_A,LHS_B,...) <
std::tie(RHS_A,RHS_B,...)` trick for lexicographical comparisons.

-- Sean Silva


> +  return Prev;
> +}
> +}
> +
> +static void PrintStack(raw_ostream &OS) {
> +  // Print out the stack in reverse order. To avoid recursion (which is
> likely
> +  // to fail if we crashed due to stack overflow), we do an up-front pass
> to
> +  // reverse the stack, then print it, then reverse it again.
> +  unsigned ID = 0;
> +  PrettyStackTraceEntry *ReversedStack =
> +      llvm::ReverseStackTrace(PrettyStackTraceHead);
> +  for (const PrettyStackTraceEntry *Entry = ReversedStack; Entry;
> +       Entry = Entry->getNextEntry()) {
> +    OS << ID++ << ".\t";
>      sys::Watchdog W(5);
>      Entry->print(OS);
>    }
> -
> -  return NextID+1;
> +  llvm::ReverseStackTrace(ReversedStack);
>  }
>
>  /// PrintCurStackTrace - Print the current stack trace to the specified
> stream.
> @@ -60,7 +74,7 @@ static void PrintCurStackTrace(raw_ostre
>    // If there are pretty stack frames registered, walk and emit them.
>    OS << "Stack dump:\n";
>
> -  PrintStack(PrettyStackTraceHead, OS);
> +  PrintStack(OS);
>    OS.flush();
>  }
>
> @@ -123,7 +137,7 @@ PrettyStackTraceEntry::~PrettyStackTrace
>  #if defined(HAVE_BACKTRACE) && defined(ENABLE_BACKTRACES)
>    assert(PrettyStackTraceHead == this &&
>           "Pretty stack trace entry destruction is out of order");
> -  PrettyStackTraceHead = getNextEntry();
> +  PrettyStackTraceHead = NextEntry;
>  #endif
>  }
>
> @@ -154,7 +168,7 @@ void llvm::EnablePrettyStackTrace() {
>  #endif
>  }
>
> -const void* llvm::SavePrettyStackState() {
> +const void *llvm::SavePrettyStackState() {
>  #if defined(HAVE_BACKTRACE) && defined(ENABLE_BACKTRACES)
>    return PrettyStackTraceHead;
>  #else
> @@ -162,9 +176,9 @@ const void* llvm::SavePrettyStackState()
>  #endif
>  }
>
> -void llvm::RestorePrettyStackState(const void* Top) {
> +void llvm::RestorePrettyStackState(const void *Top) {
>  #if defined(HAVE_BACKTRACE) && defined(ENABLE_BACKTRACES)
> -  PrettyStackTraceHead = (const PrettyStackTraceEntry*)Top;
> +  PrettyStackTraceHead = (PrettyStackTraceEntry*)Top;
>  #endif
>  }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160527/72086418/attachment.html>