lib: add post condition assertions for current thread error after user 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>

diff --git a/src/lib/assert-post.h b/src/lib/assert-post.h
index 2cc359b4078445adaad91722641d039fb1c8c716..b06eb7425fb03da0bf64382420cf6652c5331396 100644 (file)
--- a/src/lib/assert-post.h
+++ b/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 */
 

diff --git a/src/lib/graph/component.c b/src/lib/graph/component.c
index d9f233eec4c0d3714f0e8cad4a6d0943aa03e9a1..b5591c9eb4f48152eed3bb76e6ed80022d109a98 100644 (file)
--- a/src/lib/graph/component.c
+++ b/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;

diff --git a/src/lib/graph/graph.c b/src/lib/graph/graph.c
index 869a71d6c82df716838c7da6429a8c1885620919..0f30ec46a529a217c451810b13347492cfb560af 100644 (file)
--- a/src/lib/graph/graph.c
+++ b/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(

diff --git a/src/lib/graph/graph.h b/src/lib/graph/graph.h
index 4e8daa27deb0c1f278ed41c5cc39817562260be5..ac431d2a3635132882c50be765d72faaec87fe60 100644 (file)
--- a/src/lib/graph/graph.h
+++ b/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(

diff --git a/src/lib/graph/iterator.c b/src/lib/graph/iterator.c
index 40acd76dec671af0546b41db2f5517fce99a0bb1..6dc3ba98266d37fe88f94d4e617b1a3fc36113bf 100644 (file)
--- a/src/lib/graph/iterator.c
+++ b/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: "

diff --git a/src/lib/graph/mip.c b/src/lib/graph/mip.c
index 46347652cdb93ada1b12556aadd463f470d47aec..2e0aac35d65a8d5f03f555c81a4ca32e22957611 100644 (file)
--- a/src/lib/graph/mip.c
+++ b/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: "

diff --git a/src/lib/graph/query-executor.c b/src/lib/graph/query-executor.c
index 60f85659f757281f6d3f42f7910543aaecaef5a4..c798cfe5b7f94713e65c24833a9e3a94dafdf865 100644 (file)
--- a/src/lib/graph/query-executor.c
+++ b/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) {

diff --git a/src/lib/trace-ir/trace-class.c b/src/lib/trace-ir/trace-class.c
index d838fdcd0699610a9d5f9f22381fc2144eaf2a47..3e6a872c1666d46a369af24c67438abe4f34ee8e 100644 (file)
--- a/src/lib/trace-ir/trace-class.c
+++ b/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();
                        }
 
                        /*

diff --git a/src/lib/trace-ir/trace.c b/src/lib/trace-ir/trace.c
index 1ad61b64df5f2d76dcfe00d51a09e22465d4c300..3bf26b7bcdb8b009a32311ff9dd29e07a3c81582 100644 (file)
--- a/src/lib/trace-ir/trace.c
+++ b/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();
                        }
 
                        /*