Skip to content

Commit 7e0bd9e

Browse files
committed
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 parent 1a8605a commit 7e0bd9e

File tree

1 file changed

+20
-1
lines changed

1 file changed

+20
-1
lines changed

gdbsupport/common-debug.h

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,25 @@ struct scoped_debug_start_end
138138

139139
DISABLE_COPY_AND_ASSIGN (scoped_debug_start_end);
140140

141-
scoped_debug_start_end (scoped_debug_start_end &&other) = default;
141+
scoped_debug_start_end (scoped_debug_start_end &&other)
142+
: m_debug_enabled (other.m_debug_enabled),
143+
m_module (other.m_module),
144+
m_func (other.m_func),
145+
m_end_prefix (other.m_end_prefix),
146+
m_msg (other.m_msg),
147+
m_with_format (other.m_with_format),
148+
m_must_decrement_print_depth (other.m_must_decrement_print_depth),
149+
m_disabled (other.m_disabled)
150+
{
151+
/* Avoid the moved-from object doing the side-effects in its destructor. */
152+
other.m_disabled = true;
153+
}
142154

143155
~scoped_debug_start_end ()
144156
{
157+
if (m_disabled)
158+
return;
159+
145160
if (m_must_decrement_print_depth)
146161
{
147162
gdb_assert (debug_print_depth > 0);
@@ -194,6 +209,10 @@ struct scoped_debug_start_end
194209
construction but not during destruction, or vice-versa. We want to make
195210
sure there are as many increments are there are decrements. */
196211
bool m_must_decrement_print_depth = false;
212+
213+
/* True if this object was moved from, and the destructor behavior must be
214+
inhibited. */
215+
bool m_disabled = false;
197216
};
198217

199218
/* Implementation of is_debug_enabled when PT is an invokable type. */

0 commit comments

Comments
 (0)