From 6ecdcca3de0dea694cdfb252160c7939f7dc2ef1 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 7 Nov 2019 18:18:06 -0500 Subject: [PATCH] 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 --- src/lib/assert-post.h | 29 +++++++++++++++++++++++++++++ src/lib/graph/component.c | 1 + src/lib/graph/graph.c | 4 ++++ src/lib/graph/graph.h | 1 + src/lib/graph/iterator.c | 8 ++++++++ src/lib/graph/mip.c | 1 + src/lib/graph/query-executor.c | 1 + src/lib/trace-ir/trace-class.c | 1 + src/lib/trace-ir/trace.c | 1 + 9 files changed, 47 insertions(+) diff --git a/src/lib/assert-post.h b/src/lib/assert-post.h index 2cc359b4..b06eb742 100644 --- 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 d9f233ee..b5591c9e 100644 --- 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 869a71d6..0f30ec46 100644 --- 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 4e8daa27..ac431d2a 100644 --- 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 40acd76d..6dc3ba98 100644 --- 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 46347652..2e0aac35 100644 --- 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 60f85659..c798cfe5 100644 --- 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 d838fdcd..3e6a872c 100644 --- 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 1ad61b64..3bf26b7b 100644 --- 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(); } /* -- 2.34.1