gdbsupport: fix scoped_debug_start_end's move constructor

I spotted a problem with scoped_debug_start_end's move constructor.
When constructing a scoped_debug_start_end through it, it doesn't
disable the moved-from object, meaning there are now two objects that
will do the side-effects of decrementing the debug_print_depth global
and printing the "end" message.  Decrementing the debug_print_depth
global twice is actually problematic, because the increments and
decrements get out of sync, meaning we should hit this assertion, in
theory:

    gdb_assert (debug_print_depth > 0);

However, in practice, we don't see that.  This is because despite the
move constructor being required for this to compile:

    template<typename PT>
    static inline scoped_debug_start_end<PT &> ATTRIBUTE_NULL_PRINTF (6, 7)
    make_scoped_debug_start_end (PT &&pred, const char *module, const char *func,
    			     const char *start_prefix,
    			     const char *end_prefix, const char *fmt, ...)
    {
      va_list args;
      va_start (args, fmt);
      auto res = scoped_debug_start_end<PT &> (pred, module, func, start_prefix,
    					   end_prefix, fmt, args);
      va_end (args);

      return res;
    }

... it is never actually called, because compilers elide the move
constructors all the way (the scoped_debug_start_end gets constructed
directly in the instance of the top-level caller).  To confirm this, I
built GDB with -fno-elide-constructors, and now I see it:

    /home/simark/src/binutils-gdb/gdb/../gdbsupport/common-debug.h:147: internal-error: ~scoped_debug_start_end: Assertion `debug_print_depth > 0' failed.

    #9  0x00005614ba5f17c3 in internal_error_loc (file=0x5614b8749960 "/home/simark/src/binutils-gdb/gdb/../gdbsupport/common-debug.h", line=147, fmt=0x5614b8733fa0 "%s: Assertion `%s' failed.") at /home/simark/src/binutils-gdb/gdbsupport/errors.cc:58
    #10 0x00005614b8e1b2e5 in scoped_debug_start_end<bool&>::~scoped_debug_start_end (this=0x7ffc6c5e7b40, __in_chrg=<optimized out>) at /home/simark/src/binutils-gdb/gdb/../gdbsupport/common-debug.h:147
    #11 0x00005614b96dbe34 in make_scoped_debug_start_end<bool&> (pred=@0x5614baad7200: true, module=0x5614b891d840 "infrun", func=0x5614b891d800 "infrun_debug_show_threads", start_prefix=0x5614b891d7c0 "enter", end_prefix=0x5614b891d780 "exit", fmt=0x0) at /home/simark/src/binutils-gdb/gdb/../gdbsupport/common-debug.h:235

Fix this by adding an m_disabled field to scoped_debug_start_end, and
setting it in the move constructor.

Change-Id: Ie5213269c584837f751d2d11de831f45ae4a899f

1 files changed, 20 insertions, 1 deletions

diff --git a/gdbsupport/common-debug.h b/gdbsupport/common-debug.h
index ec36d88..33b15a0 100644
--- a/gdbsupport/common-debug.h
+++ b/gdbsupport/common-debug.h
@@ -138,10 +138,25 @@ struct scoped_debug_start_end
 
   DISABLE_COPY_AND_ASSIGN (scoped_debug_start_end);
 
-  scoped_debug_start_end (scoped_debug_start_end &&other) = default;
+  scoped_debug_start_end (scoped_debug_start_end &&other)
+    : m_debug_enabled (other.m_debug_enabled),
+      m_module (other.m_module),
+      m_func (other.m_func),
+      m_end_prefix (other.m_end_prefix),
+      m_msg (other.m_msg),
+      m_with_format (other.m_with_format),
+      m_must_decrement_print_depth (other.m_must_decrement_print_depth),
+      m_disabled (other.m_disabled)
+  {
+    /* Avoid the moved-from object doing the side-effects in its destructor.  */
+    other.m_disabled = true;
+  }
 
   ~scoped_debug_start_end ()
   {
+    if (m_disabled)
+      return;
+
     if (m_must_decrement_print_depth)
       {
 	gdb_assert (debug_print_depth > 0);
@@ -194,6 +209,10 @@ private:
      construction but not during destruction, or vice-versa.  We want to make
      sure there are as many increments are there are decrements.  */
   bool m_must_decrement_print_depth = false;
+
+  /* True if this object was moved from, and the destructor behavior must be
+     inhibited.  */
+  bool m_disabled = false;
 };
 
 /* Implementation of is_debug_enabled when PT is an invokable type.  */