lib: add post condition assertions for current thread error after use…

…r functions

It is a logic error and a post condition breach for a user function to
return a non-error status code (>= 0) but leave an error set on the
current thread.  Add some assertions after each point where we call a
user function to catch this kind of mistake.

The macro BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS is meant to be
used in the very fast path (consume/next callbacks).  The macro
BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS is used elsewhere.

The macros are written in such a way that if they cause an abort, the
error will still be present in thread_error, so accessible to a
debugger.

Change-Id: I4c6127c0b0258dac5be1e3bae63c2d9e6baa2d1f
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>

src/lib/assert-post.h

@@ -101,6 +101,29 @@
 		}							\
 	} while (0)
 
+/*
+ * Asserts that if there's an error on the current thread, an error status code
+ * was returned.  Puts back the error in place (if there is one) such that if
+ * the assertion hits, it will be possible to inspect it with a debugger.
+ */
+#define BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(_status)			\
+	do {									\
+		const struct bt_error *err = bt_current_thread_take_error();	\
+		if (err) {							\
+			bt_current_thread_move_error(err);			\
+		}								\
+		BT_ASSERT_POST(_status < 0 || !err,				\
+			"Current thread has an error, but user function "	\
+			"returned a non-error status: status=%s",		\
+			bt_common_func_status_string(_status));			\
+	} while (0)
+
+/*
+ * Asserts that the current thread has no error.
+ */
+#define BT_ASSERT_POST_NO_ERROR() \
+	BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(0)
+
 /*
  * Marks a function as being only used within a BT_ASSERT_POST()
  * context.
@@ -116,11 +139,17 @@
 # define BT_ASSERT_POST_DEV(_cond, _fmt, ...)				\
 	BT_ASSERT_POST((_cond), _fmt, ##__VA_ARGS__)
 
+/* Developer mode version of BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(). */
+# define BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS(_status) \
+	BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(_status)
+
 /* Developer mode version of `BT_ASSERT_POST_FUNC`. */
 # define BT_ASSERT_POST_DEV_FUNC BT_ASSERT_POST_FUNC
 #else
 # define BT_ASSERT_POST_DEV_MSG(_fmt, ...)
 # define BT_ASSERT_POST_DEV(_cond, _fmt, ...)	((void) sizeof((void) (_cond), 0))
+# define BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS(_status) \
+	((void) sizeof((void) (_status), 0))
 # define BT_ASSERT_POST_DEV_FUNC	__attribute__((unused))
 #endif /* BT_DEV_MODE */
 

src/lib/graph/component.c

@@ -553,6 +553,7 @@ bt_component_port_connected(
 			status == BT_FUNC_STATUS_MEMORY_ERROR,
 			"Unexpected returned component status: status=%s",
 			bt_common_func_status_string(status));
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
 	}
 
 	return status;

src/lib/graph/graph.c

@@ -588,6 +588,7 @@ int consume_graph_sink(struct bt_component_sink *comp)
 		consume_status == BT_FUNC_STATUS_MEMORY_ERROR,
 		"Invalid component status returned by consuming method: "
 		"status=%s", bt_common_func_status_string(consume_status));
+	BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS(consume_status);
 	if (consume_status) {
 		if (consume_status < 0) {
 			BT_LIB_LOGW_APPEND_CAUSE(
@@ -1144,6 +1145,7 @@ enum bt_graph_listener_func_status bt_graph_notify_port_added(
 
 		BT_ASSERT(listener->func);
 		status = listener->func(comp, port, listener->base.data);
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
 		if (status != BT_FUNC_STATUS_OK) {
 			goto end;
 		}
@@ -1222,6 +1224,7 @@ enum bt_graph_listener_func_status bt_graph_notify_ports_connected(
 		BT_ASSERT(listener->func);
 		status = listener->func(upstream_comp, downstream_comp,
 			upstream_port, downstream_port, listener->base.data);
+		BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS(status);
 		if (status != BT_FUNC_STATUS_OK) {
 			goto end;
 		}
@@ -1338,6 +1341,7 @@ int add_component_with_init_method_data(
 		init_status = init_method(component, NULL, params, init_method_data);
 		BT_LOGD("User method returned: status=%s",
 			bt_common_func_status_string(init_status));
+		BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS(init_status);
 		if (init_status != BT_FUNC_STATUS_OK) {
 			if (init_status < 0) {
 				BT_LIB_LOGW_APPEND_CAUSE(

src/lib/graph/graph.h

@@ -257,6 +257,7 @@ int bt_graph_configure(struct bt_graph *graph)
 				comp_status == BT_FUNC_STATUS_MEMORY_ERROR,
 				"Unexpected returned status: status=%s",
 				bt_common_func_status_string(comp_status));
+			BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(comp_status);
 			if (comp_status != BT_FUNC_STATUS_OK) {
 				if (comp_status < 0) {
 					BT_LIB_LOGW_APPEND_CAUSE(

src/lib/graph/iterator.c

@@ -477,6 +477,7 @@ int create_self_component_input_port_message_iterator(
 			upstream_port);
 		BT_LOGD("User method returned: status=%s",
 			bt_common_func_status_string(iter_status));
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(iter_status);
 		if (iter_status != BT_FUNC_STATUS_OK) {
 			BT_LIB_LOGW_APPEND_CAUSE(
 				"Component input port message iterator initialization method failed: "
@@ -869,6 +870,8 @@ call_iterator_next_method(
 			"Clock snapshots are not monotonic");
 	}
 
+	BT_ASSERT_POST_DEV_NO_ERROR_IF_NO_ERROR_STATUS(status);
+
 	return status;
 }
 
@@ -1007,6 +1010,8 @@ bt_self_component_port_input_message_iterator_can_seek_ns_from_origin(
 		status = (int) iterator->methods.can_seek_ns_from_origin(iterator,
 			ns_from_origin, can_seek);
 
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
+
 		if (status != BT_FUNC_STATUS_OK) {
 			BT_LIB_LOGW_APPEND_CAUSE(
 				"Component input port message iterator's \"can seek nanoseconds from origin\" method failed: "
@@ -1077,6 +1082,7 @@ bt_self_component_port_input_message_iterator_can_seek_beginning(
 				*can_seek == BT_FALSE,
 			"Unexpected boolean value returned from user's \"can seek beginning\" method: val=%d, %![iter-]+i",
 			*can_seek, iterator);
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
 	} else {
 		*can_seek = BT_FALSE;
 		status = BT_FUNC_STATUS_OK;
@@ -1172,6 +1178,7 @@ bt_self_component_port_input_message_iterator_seek_beginning(
 		status == BT_FUNC_STATUS_AGAIN,
 		"Unexpected status: %![iter-]+i, status=%s",
 		iterator, bt_common_func_status_string(status));
+	BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
 	if (status < 0) {
 		BT_LIB_LOGW_APPEND_CAUSE(
 			"Component input port message iterator's \"seek beginning\" method failed: "
@@ -1759,6 +1766,7 @@ bt_self_component_port_input_message_iterator_seek_ns_from_origin(
 			status == BT_FUNC_STATUS_AGAIN,
 			"Unexpected status: %![iter-]+i, status=%s",
 			iterator, bt_common_func_status_string(status));
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(status);
 		if (status < 0) {
 			BT_LIB_LOGW_APPEND_CAUSE(
 				"Component input port message iterator's \"seek nanoseconds from origin\" method failed: "

src/lib/graph/mip.c

@@ -146,6 +146,7 @@ int validate_operative_mip_version_in_array(GPtrArray *descriptors,
 			range_set->ranges->len > 0,
 			"User method returned `BT_FUNC_STATUS_OK` without "
 			"adding a range to the supported MIP version range set.");
+		BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(method_status);
 		if (method_status < 0) {
 			BT_LIB_LOGW_APPEND_CAUSE(
 				"Component class's \"get supported MIP versions\" method failed: "

src/lib/graph/query-executor.c

@@ -234,6 +234,7 @@ enum bt_query_executor_query_status bt_query_executor_query(
 		bt_common_func_status_string(query_status), *user_result);
 	BT_ASSERT_POST(query_status != BT_FUNC_STATUS_OK || *user_result,
 		"User method returned `BT_FUNC_STATUS_OK` without a result.");
+	BT_ASSERT_POST_NO_ERROR_IF_NO_ERROR_STATUS(query_status);
 	status = (int) query_status;
 
 	if (status < 0) {

src/lib/trace-ir/trace-class.c

@@ -102,6 +102,7 @@ void destroy_trace_class(struct bt_object *obj)
 
 			if (elem.func) {
 				elem.func(tc, elem.data);
+				BT_ASSERT_POST_NO_ERROR();
 			}
 
 			/*

src/lib/trace-ir/trace.c

@@ -104,6 +104,7 @@ void destroy_trace(struct bt_object *obj)
 
 			if (elem.func) {
 				elem.func(trace, elem.data);
+				BT_ASSERT_POST_NO_ERROR();
 			}
 
 			/*