This came up in discussion of an earlier patch.
I'm in two minds as to whether it's a good idea or not - the underlying
issue being that libubsan does not yet (AFAICT) have the concept of a
coroutine, so that the diagnostics are not very specific and might appear
strange (i.e. "execution reached the end of a value-returning function
without returning a value" which is a bit of an odd diagnostic for
a missing return_void ()).
OTOH one might argue that some diagnostic is better than silent UB .. but
I do not have cycles to address improving this in upstream libsanitizer ...
The logic used here is intended to match cp_maybe_instrument_return ()
although it's not 100% clear that that is doing exactly what the comment
says - since it does not distinguish between -fno-sanitize=return and
the case that the user simply did not specify it. So that
-fsanitize=unreachable is ignored for both fno-sanitize=return and the
unset case.
tested on x86_64-darwin and powerpc64le-linux,
apply to trunk? Opinions?
thanks
Iain
--- 8< ---
[dcl.fct.def.coroutine] / 6 Note 1:
"If return_void is found, flowing off the end of a coroutine is equivalent
to a co_return with no operand. Otherwise, flowing off the end of a
coroutine results in undefined behavior."
Here we implement this as a check for sanitized returns and call the ubsan
instrumentation; if that is not enabled we mark this as unreachable (which
might trap depending on the target settings).
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::wrap_original_function_body): Instrument
the case where control flows off the end of a coroutine and the
user promise has no return_void entry.
Signed-off-by: Iain Sandoe <[email protected]>
---
gcc/cp/coroutines.cc | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index e709d02b5f7..b67f4e3ef88 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -33,6 +33,9 @@ along with GCC; see the file COPYING3. If not see
#include "gcc-rich-location.h"
#include "hash-map.h"
#include "coroutines.h"
+#include "c-family/c-ubsan.h"
+#include "attribs.h" /* lookup_attribute */
+#include "asan.h" /* sanitize_flags_p */
static bool coro_promise_type_found_p (tree, location_t);
@@ -4335,8 +4338,17 @@ cp_coroutine_transform::wrap_original_function_body ()
finish_expr_stmt (initial_await);
/* Append the original function body. */
add_stmt (coroutine_body);
+ /* Flowing off the end of a coroutine is equivalent to calling
+ promise.return_void () or is UB if the promise does not contain
+ that. Do not add an unreachable unless the user has asked for
+ checking of such cases. */
if (return_void)
add_stmt (return_void);
+ else if (sanitize_flags_p (SANITIZE_RETURN, orig_fn_decl))
+ add_stmt (ubsan_instrument_return (fn_start));
+ else if (flag_unreachable_traps
+ && !sanitize_flags_p (SANITIZE_UNREACHABLE, orig_fn_decl))
+ add_stmt (build_builtin_unreachable (fn_start));
TRY_STMTS (tcb) = pop_stmt_list (TRY_STMTS (tcb));
TRY_HANDLERS (tcb) = push_stmt_list ();
/* Mimic what the parser does for the catch. */
@@ -4393,8 +4405,14 @@ cp_coroutine_transform::wrap_original_function_body ()
finish_expr_stmt (initial_await);
/* Append the original function body. */
add_stmt (coroutine_body);
+ /* See comment above. */
if (return_void)
add_stmt (return_void);
+ else if (sanitize_flags_p (SANITIZE_RETURN, orig_fn_decl))
+ add_stmt (ubsan_instrument_return (fn_start));
+ else if (flag_unreachable_traps
+ && !sanitize_flags_p (SANITIZE_UNREACHABLE, orig_fn_decl))
+ add_stmt (build_builtin_unreachable (fn_start));
}
/* co_return branches to the final_suspend label, so declare that now. */
--
2.39.2 (Apple Git-143)
