dovecot-2.2: lib: add identifying markers to data-stack frames

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.2/rev/6ea0584e3861
changeset: 17635:6ea0584e3861
user:      Phil Carmody <phil at dovecot.fi>
date:      Mon Jul 28 16:45:33 2014 +0300
description:
lib: add identifying markers to data-stack frames
Add a string parameter to t_push() so that in DEBUG mode,
misbehaviour inside a stack level can be blamed on someone.

Default the T_BEGIN macro to automatigally use __FUNCTION__ or
__FILE__:__LINE__ as that identifier, therefore no clients of
those macros need to change.

ioloop used t_push() directly as it wanted customised diagnostic
strings. To preserve this friendliness, also introduce a t_push_named()
which takes a format string with paramters.

Apart from the unused paramter, a non-DEBUG build should see no
changes.

Signed-off-by: Phil Carmody <phil at dovecot.fi>

diffstat:

 src/lib/data-stack.c |  29 ++++++++++++++++++++++++++---
 src/lib/data-stack.h |  22 ++++++++++++++++++----
 src/lib/ioloop.c     |   6 ++++--
 3 files changed, 48 insertions(+), 9 deletions(-)

diffs (137 lines):

diff -r 098725138469 -r 6ea0584e3861 src/lib/data-stack.c
--- a/src/lib/data-stack.c	Mon Jul 28 16:40:25 2014 +0300
+++ b/src/lib/data-stack.c	Mon Jul 28 16:45:33 2014 +0300
@@ -54,6 +54,9 @@
 	struct stack_block *block[BLOCK_FRAME_COUNT];
         size_t block_space_used[BLOCK_FRAME_COUNT];
 	size_t last_alloc_size[BLOCK_FRAME_COUNT];
+#ifdef DEBUG
+	const char *marker[BLOCK_FRAME_COUNT];
+#endif
 };
 
 unsigned int data_stack_frame = 0;
@@ -112,7 +115,7 @@
 	}
 }
 
-unsigned int t_push(void)
+unsigned int t_push(const char *marker)
 {
         struct stack_frame_block *frame_block;
 
@@ -123,7 +126,7 @@
 			/* kludgy, but allow this before initialization */
 			frame_pos = 0;
 			data_stack_init();
-			return t_push();
+			return t_push(marker);
 		}
 
 		frame_pos = 0;
@@ -153,10 +156,30 @@
 	current_frame_block->block[frame_pos] = current_block;
 	current_frame_block->block_space_used[frame_pos] = current_block->left;
         current_frame_block->last_alloc_size[frame_pos] = 0;
+#ifdef DEBUG
+	current_frame_block->marker[frame_pos] = marker;
+#else
+	(void)marker; /* only used for debugging */
+#endif
 
         return data_stack_frame++;
 }
 
+unsigned int t_push_named(const char *format, ...)
+{
+	unsigned int ret = t_push(NULL);
+#ifdef DEBUG
+	va_list args;
+	va_start(args, format);
+	current_frame_block->marker[frame_pos] = p_strdup_vprintf(unsafe_data_stack_pool, format, args);
+	va_end(args);
+#else
+	(void)format; /* unused in non-DEBUG builds */
+#endif
+
+	return ret;
+}
+
 static void free_blocks(struct stack_block *block)
 {
 	struct stack_block *next;
@@ -523,7 +546,7 @@
 	last_buffer_block = NULL;
 	last_buffer_size = 0;
 
-	(void)t_push();
+	(void)t_push("data_stack_init");
 }
 
 void data_stack_deinit(void)
diff -r 098725138469 -r 6ea0584e3861 src/lib/data-stack.h
--- a/src/lib/data-stack.h	Mon Jul 28 16:40:25 2014 +0300
+++ b/src/lib/data-stack.h	Mon Jul 28 16:45:33 2014 +0300
@@ -33,21 +33,35 @@
 
 extern unsigned int data_stack_frame;
 
-/* All t_..() allocations between t_push() and t_pop() are freed after t_pop()
+/* All t_..() allocations between t_push*() and t_pop() are freed after t_pop()
    is called. Returns the current stack frame number, which can be used
    to detect missing t_pop() calls:
 
-   x = t_push(); .. if (t_pop() != x) abort();
+   x = t_push(__func__); .. if (t_pop() != x) abort();
+
+   In DEBUG mode, t_push_named() makes a temporary allocation for the name,
+   but is safe to call in a loop as it performs the allocation within its own
+   frame. However, you should always prefer to use T_BEGIN { ... } T_END below.
 */
-unsigned int t_push(void) ATTR_HOT;
+unsigned int t_push(const char *marker) ATTR_HOT;
+unsigned int t_push_named(const char *format, ...) ATTR_HOT ATTR_FORMAT(1, 2);
 unsigned int t_pop(void) ATTR_HOT;
 /* Simplifies the if (t_pop() != x) check by comparing it internally and
    panicking if it doesn't match. */
 void t_pop_check(unsigned int *id) ATTR_HOT;
 
 /* Usage: T_BEGIN { code } T_END */
+#ifndef DEBUG
 #define T_BEGIN \
-	STMT_START { unsigned int _data_stack_cur_id = t_push();
+	STMT_START { unsigned int _data_stack_cur_id = t_push(NULL);
+#elif defined (__GNUC__) && !defined (__STRICT_ANSI__)
+#define T_BEGIN \
+	STMT_START { unsigned int _data_stack_cur_id = t_push(__FUNCTION__);
+#else
+#define T_CAT2(a,b) (a ":" #b)
+#define T_BEGIN \
+	STMT_START { unsigned int _data_stack_cur_id = t_push(T_CAT2(__FILE__,__LINE__));
+#endif
 #define T_END \
 	t_pop_check(&_data_stack_cur_id); } STMT_END
 
diff -r 098725138469 -r 6ea0584e3861 src/lib/ioloop.c
--- a/src/lib/ioloop.c	Mon Jul 28 16:40:25 2014 +0300
+++ b/src/lib/ioloop.c	Mon Jul 28 16:45:33 2014 +0300
@@ -406,7 +406,8 @@
 
 		if (timeout->ctx != NULL)
 			io_loop_context_activate(timeout->ctx);
-                t_id = t_push();
+		t_id = t_push_named("ioloop timeout handler %p",
+				    (void *)timeout->callback);
 		timeout->callback(timeout->context);
 		if (t_pop() != t_id) {
 			i_panic("Leaked a t_pop() call in timeout handler %p",
@@ -437,7 +438,8 @@
 
 	if (io->ctx != NULL)
 		io_loop_context_activate(io->ctx);
-	t_id = t_push();
+	t_id = t_push_named("ioloop handler %p",
+			    (void *)io->callback);
 	io->callback(io->context);
 	if (t_pop() != t_id) {
 		i_panic("Leaked a t_pop() call in I/O handler %p",