[PATCH] Implement dbgs()

Here's the patch to provide dbgs(). By default it works just like errs().
When -debug-buffer-size=N (N > 0) is set, it buffers output sent to it and
dumps it at program termination via a signal handler.

Please review. Thanks!

                                  -Dave

Index: include/llvm/Support/Debug.h

Here's the patch to provide dbgs(). By default it works just like errs().
When -debug-buffer-size=N (N > 0) is set, it buffers output sent to it and
dumps it at program termination via a signal handler.

Please review. Thanks!

                                 -Dave

Index: include/llvm/Support/Debug.h

--- include/llvm/Support/Debug.h (revision 91557)

Ok.

+++ lib/Support/Debug.cpp (working copy)
@@ -25,6 +25,9 @@

#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/circular_raw_ostream.h"
+#include "llvm/System/Signals.h"
+
using namespace llvm;

// All Debug.h functionality is a no-op in NDEBUG mode.
@@ -37,6 +40,15 @@
Debug("debug", cl::desc("Enable debug output"), cl::Hidden,
      cl::location(DebugFlag));

+// -debug-buffer-size - This is a command line op0tion to set the size
+// of the debug stream circular buffer. The value is the number of
+// characters to save.
+static cl::opt<unsigned>
+DebugBufferSize("debug-buffer-size",
+ llvm::cl::desc("Save last N characters of debug output "

How about "Buffer the last N characters of debug output until program termination". This should probably also be cl::Hidden.

+// Signal handlers - dump debug output on termination.
+static void debug_user_sig_handler(void *Cookie)
+{
+ llvm::circular_raw_ostream *logout =
+ dynamic_cast<llvm::circular_raw_ostream *>(&llvm::dbgs());

Please do not use dynamic_cast, we're trying to eliminate the last RTTI use in the compiler.

+/// dbgs - Return a circular-buffered debug stream.
+raw_ostream &llvm::dbgs() {
+ static circular_raw_ostream strm(errs(), DebugBufferSize);
+
+ static bool initialized = false;
+ if (!initialized) {
+ initialized = true;

This isn't thread safe. A way to fix this is to do something like:

static struct dbgstream {
   circular_raw_ostream strm;

   dbgstream() : strm(errs(), DebugBufferSize) {
     your one time init stuff here.
   }
} thestrm;

return thestrm.strm;

Please commit with the appropriate fixes when we settle on the stream design.

-Chris

> +// -debug-buffer-size - This is a command line op0tion to set the
> size
> +// of the debug stream circular buffer. The value is the number of
> +// characters to save.
> +static cl::opt<unsigned>
> +DebugBufferSize("debug-buffer-size",
> + llvm::cl::desc("Save last N characters of debug
> output "

How about "Buffer the last N characters of debug output until program
termination". This should probably also be cl::Hidden.

Ok.

> +// Signal handlers - dump debug output on termination.
> +static void debug_user_sig_handler(void *Cookie)
> +{
> + llvm::circular_raw_ostream *logout =
> + dynamic_cast<llvm::circular_raw_ostream *>(&llvm::dbgs());

Please do not use dynamic_cast, we're trying to eliminate the last
RTTI use in the compiler.

Do you want me to use dyn_cast? That means I'll have to add some more
stuff to raw_ostream and circular_raw_ostream.

Or I think I can just assume (Yikes!) that if the signal handler is
invoked it will really be a circular_raw_ostream since the handler
should (!) only be set up in debug mode.

That scares me a bit, though.

> +/// dbgs - Return a circular-buffered debug stream.
> +raw_ostream &llvm::dbgs() {
> + static circular_raw_ostream strm(errs(), DebugBufferSize);
> +
> + static bool initialized = false;
> + if (!initialized) {
> + initialized = true;

This isn't thread safe. A way to fix this is to do something like:

static struct dbgstream {
   circular_raw_ostream strm;

   dbgstream() : strm(errs(), DebugBufferSize) {
     your one time init stuff here.
   }
} thestrm;

return thestrm.strm;

Ok.

Please commit with the appropriate fixes when we settle on the stream
design.

All right.

                                      -Dave

+// Signal handlers - dump debug output on termination.
+static void debug_user_sig_handler(void *Cookie)
+{
+ llvm::circular_raw_ostream *logout =
+ dynamic_cast<llvm::circular_raw_ostream *>(&llvm::dbgs());

Please do not use dynamic_cast, we're trying to eliminate the last
RTTI use in the compiler.

Do you want me to use dyn_cast? That means I'll have to add some more
stuff to raw_ostream and circular_raw_ostream.

No, dyn_cast is only really suitable for static class hierarchies.

Or I think I can just assume (Yikes!) that if the signal handler is
invoked it will really be a circular_raw_ostream since the handler
should (!) only be set up in debug mode.

That scares me a bit, though.

Why don't you just check #ifndef NDEBUG like the code that sets it up?

-Chris

It's under #ifndef NDEBUG right now. My worry is that this kind of thing
is fragile.

                               -Dave

Next iteration.

                                -Dave
Index: include/llvm/Support/Debug.h

Will this setup a signal handler in a Release+Asserts build?
That is not very nice, a library shouldn't setup signal handlers, except
in debug mode.

Isn't there a better macro to differentiate between Release and Debug
builds?

Or to allow the library's client to specify whether it wants the signal
handler or not?
That would be similar to how the PrettyStacktrace stuff works: client
software can skip it completely when in release mode,
and still have assertions enabled.

Best regards,
--Edwin

Here's a revision. It only adds the handler if DisableDebugBuffering is
false, DebugFlag is true and the debug buffer size is non-zero.

                                  -Dave

Index: include/llvm/Support/Debug.h

Please reverse the sense of this, forcing clients to opt into dbgs() buffering, and update the tools to set the flag. Looks good otherwise,

-Chris

Done. Will commit once the circular_raw_ostream is in.

                            -Dave