Skip to content
Snippets Groups Projects
  • Simon Marchi's avatar
    7e0bd9ea
    gdbsupport: fix scoped_debug_start_end's move constructor · 7e0bd9ea
    Simon Marchi authored
    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
    7e0bd9ea
    History
    gdbsupport: fix scoped_debug_start_end's move constructor
    Simon Marchi authored
    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