Hi!
The following patch changes the way destructor calls are sanitized. Currently,
they are sanitized just like any other member calls, transforming `a::~a(this)'
to `a::~a(.UBSAN_NULL(SAVE_EXPR(this)), SAVE_EXPR(this))'. However, this is a
problem for coroutines. In some cases, a destructor call is cloned to another
location, so then the saved expression is not evaluated before the second call
to the destructor.
The patch gets rid of SAVE_EXPRs by creating a temporary variable, so the calls
are transformed to something resembling:
{
void *ptr;
ptr = this;
.UBSAN_NULL(ptr);
a::~a(ptr);
}
Unfortunately, the implementation is not so straight-forward. Firstly, we do not
want to sanitize again already sanitized call. I accomplish this by creating a
hash set of instrumented expressions on the way to the tree's root. Secondly,
the affected calls are cloned, so I need to explicitly copy the tree node to
change it's argument. I am not sure if the proposed solution is optimal and will
not cause the creation of an exponential number of nodes.
The patch survives bootstrapping and regression testing on x86_64-pc-linux-gnu.
If approved, I would like to note that I do not have write access to the
repository and this is my first contribution to GCC.
2021-08-30 Dan Klishch <daklis...@gmail.com>
PR c++/101355
gcc/testsuite/
* g++.dg/coroutines/pr101355.C: New test.
gcc/c-family/
* c-ubsan.h (ubsan_maybe_instrument_member_call): Change prototype.
* c-ubsan.c: Add new include.
(ubsan_should_instrument_reference_or_call): Add new function.
(ubsan_maybe_instrument_reference_or_call): Add possibility to convert
call-expr to bind-expr, so it would not use save-expr.
(ubsan_maybe_instrument_member_call): Add flag to allow using new
functionality of the previous.
(ubsan_maybe_instrument_reference): Fix arguments of call to
ubsan_maybe_instrument_reference_or_call.
gcc/cp/
* cp-gimplify.c (struct cp_genericize_data): Add instrumented_dtor_set
field.
(cp_genericize_r): Tell ubsan to instrument destructors without using
save-expr.
(cp_genericize_tree): Add initialization of instrumented_dtor_set.
diff --git a/gcc/c-family/c-ubsan.c b/gcc/c-family/c-ubsan.c
index 12a7bca5c32..593836984a5 100644
--- a/gcc/c-family/c-ubsan.c
+++ b/gcc/c-family/c-ubsan.c
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3. If not see
#include "coretypes.h"
#include "tm.h"
#include "c-family/c-common.h"
+#include "tree-iterator.h"
#include "ubsan.h"
#include "c-family/c-ubsan.h"
#include "stor-layout.h"
@@ -398,22 +399,15 @@ ubsan_maybe_instrument_array_ref (tree *expr_p, bool
ignore_off_by_one)
}
}
-static tree
-ubsan_maybe_instrument_reference_or_call (location_t loc, tree op, tree ptype,
- enum ubsan_null_ckind ckind)
+static bool
+ubsan_should_instrument_reference_or_call (tree op, tree ptype,
+ unsigned int &mina)
{
- if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL)
- || current_function_decl == NULL_TREE)
- return NULL_TREE;
-
- tree type = TREE_TYPE (ptype);
- tree orig_op = op;
bool instrument = false;
- unsigned int mina = 0;
if (sanitize_flags_p (SANITIZE_ALIGNMENT))
{
- mina = min_align_of_type (type);
+ mina = min_align_of_type (TREE_TYPE (ptype));
if (mina <= 1)
mina = 0;
}
@@ -454,19 +448,68 @@ ubsan_maybe_instrument_reference_or_call (location_t loc,
tree op, tree ptype,
instrument = true;
}
}
- if (!instrument)
+ return instrument;
+}
+
+static tree
+ubsan_maybe_instrument_reference_or_call (location_t loc, tree op, tree ptype,
+ enum ubsan_null_ckind ckind,
+ bool use_save_expr, tree *stmt_p,
+ tree *copied_call)
+{
+ if (!sanitize_flags_p (SANITIZE_ALIGNMENT | SANITIZE_NULL)
+ || current_function_decl == NULL_TREE)
return NULL_TREE;
- op = save_expr (orig_op);
+
+ unsigned int mina = 0;
+ if (!ubsan_should_instrument_reference_or_call (op, ptype, mina))
+ return NULL_TREE;
+
gcc_assert (POINTER_TYPE_P (ptype));
if (TREE_CODE (ptype) == REFERENCE_TYPE)
ptype = build_pointer_type (TREE_TYPE (ptype));
tree kind = build_int_cst (ptype, ckind);
tree align = build_int_cst (pointer_sized_int_node, mina);
- tree call
- = build_call_expr_internal_loc (loc, IFN_UBSAN_NULL, void_type_node,
- 3, op, kind, align);
- TREE_SIDE_EFFECTS (call) = 1;
- return fold_build2 (COMPOUND_EXPR, TREE_TYPE (op), call, op);
+
+ if (use_save_expr)
+ {
+ op = save_expr (op);
+ tree call = build_call_expr_internal_loc (
+ loc, IFN_UBSAN_NULL, void_type_node, 3, op, kind, align);
+ TREE_SIDE_EFFECTS (call) = 1;
+ return fold_build2 (COMPOUND_EXPR, TREE_TYPE (op), call, op);
+ }
+ else
+ {
+ tree stmt = *stmt_p;
+ tree var
+ = build_decl (EXPR_LOCATION (stmt), VAR_DECL, NULL, ptr_type_node);
+ DECL_ARTIFICIAL (var) = 1;
+ DECL_IGNORED_P (var) = 1;
+ DECL_NAMELESS (var) = 1;
+ TREE_READONLY (var) = 0;
+ DECL_EXTERNAL (var) = 0;
+ TREE_STATIC (var) = 0;
+
+ tree exprs = NULL_TREE;
+
+ tree modify_expr
+ = build2 (MODIFY_EXPR, ptr_type_node, var, CALL_EXPR_ARG (stmt, 0));
+ append_to_statement_list (modify_expr, &exprs);
+
+ tree sanitize_expr = build_call_expr_internal_loc (
+ loc, IFN_UBSAN_NULL, void_type_node, 3, var, kind, align);
+ append_to_statement_list (sanitize_expr, &exprs);
+
+ tree call_expr = copy_node (stmt);
+ CALL_EXPR_ARG (call_expr, 0) = var;
+ *copied_call = call_expr;
+ append_to_statement_list (call_expr, &exprs);
+
+ tree bind = build3 (BIND_EXPR, void_type_node, var, exprs, NULL);
+ TREE_SIDE_EFFECTS (bind) = 1;
+ return bind;
+ }
}
/* Instrument a NOP_EXPR to REFERENCE_TYPE or INTEGER_CST with REFERENCE_TYPE
@@ -481,7 +524,8 @@ ubsan_maybe_instrument_reference (tree *stmt_p)
op = TREE_OPERAND (stmt, 0);
op = ubsan_maybe_instrument_reference_or_call (EXPR_LOCATION (stmt), op,
TREE_TYPE (stmt),
- UBSAN_REF_BINDING);
+ UBSAN_REF_BINDING, true,
+ stmt_p, NULL);
if (op)
{
if (TREE_CODE (stmt) == NOP_EXPR)
@@ -493,19 +537,28 @@ ubsan_maybe_instrument_reference (tree *stmt_p)
/* Instrument a CALL_EXPR to a method if needed. */
-void
-ubsan_maybe_instrument_member_call (tree stmt, bool is_ctor)
+tree
+ubsan_maybe_instrument_member_call (tree *stmt_p, bool is_ctor, bool is_dtor)
{
+ tree stmt = *stmt_p;
if (call_expr_nargs (stmt) == 0)
- return;
+ return NULL_TREE;
tree op = CALL_EXPR_ARG (stmt, 0);
- if (op == error_mark_node
- || !POINTER_TYPE_P (TREE_TYPE (op)))
- return;
- op = ubsan_maybe_instrument_reference_or_call (EXPR_LOCATION (stmt), op,
- TREE_TYPE (op),
- is_ctor ? UBSAN_CTOR_CALL
- : UBSAN_MEMBER_CALL);
+ if (op == error_mark_node || !POINTER_TYPE_P (TREE_TYPE (op)))
+ return NULL_TREE;
+ tree node;
+ op = ubsan_maybe_instrument_reference_or_call (
+ EXPR_LOCATION (stmt), op, TREE_TYPE (op),
+ is_ctor ? UBSAN_CTOR_CALL : UBSAN_MEMBER_CALL, !is_dtor, stmt_p, &node);
if (op)
- CALL_EXPR_ARG (stmt, 0) = op;
+ {
+ if (!is_dtor)
+ CALL_EXPR_ARG (stmt, 0) = op;
+ else
+ {
+ *stmt_p = op;
+ return node;
+ }
+ }
+ return NULL_TREE;
}
diff --git a/gcc/c-family/c-ubsan.h b/gcc/c-family/c-ubsan.h
index 19c3b4464ae..b5cf0b4131d 100644
--- a/gcc/c-family/c-ubsan.h
+++ b/gcc/c-family/c-ubsan.h
@@ -29,6 +29,6 @@ extern tree ubsan_instrument_bounds (location_t, tree, tree
*, bool);
extern bool ubsan_array_ref_instrumented_p (const_tree);
extern void ubsan_maybe_instrument_array_ref (tree *, bool);
extern void ubsan_maybe_instrument_reference (tree *);
-extern void ubsan_maybe_instrument_member_call (tree, bool);
+extern tree ubsan_maybe_instrument_member_call (tree *, bool, bool);
#endif /* GCC_C_UBSAN_H */
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index bf928a82ce9..4fb6a9cfd04 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -824,6 +824,7 @@ omp_cxx_notice_variable (struct cp_genericize_omp_taskreg
*omp_ctx, tree decl)
struct cp_genericize_data
{
hash_set<tree> *p_set;
+ hash_set<tree> *instrumented_dtor_set;
auto_vec<tree> bind_expr_stack;
struct cp_genericize_omp_taskreg *omp_ctx;
tree try_block;
@@ -1435,12 +1436,25 @@ cp_genericize_r (tree *stmt_p, int *walk_subtrees, void
*data)
&& INDIRECT_TYPE_P (TREE_TYPE (fn))
&& TREE_CODE (TREE_TYPE (TREE_TYPE (fn))) == METHOD_TYPE)
{
+ bool is_func
+ = TREE_CODE (fn) == ADDR_EXPR
+ && TREE_CODE (TREE_OPERAND (fn, 0)) == FUNCTION_DECL;
bool is_ctor
- = TREE_CODE (fn) == ADDR_EXPR
- && TREE_CODE (TREE_OPERAND (fn, 0)) == FUNCTION_DECL
- && DECL_CONSTRUCTOR_P (TREE_OPERAND (fn, 0));
- if (sanitize_flags_p (SANITIZE_NULL | SANITIZE_ALIGNMENT))
- ubsan_maybe_instrument_member_call (stmt, is_ctor);
+ = is_func && DECL_CONSTRUCTOR_P (TREE_OPERAND (fn, 0));
+ bool is_dtor
+ = is_func && DECL_DESTRUCTOR_P (TREE_OPERAND (fn, 0));
+ tree node;
+ if (!wtd->instrumented_dtor_set->contains (stmt)
+ && sanitize_flags_p (SANITIZE_NULL | SANITIZE_ALIGNMENT)
+ && (node = ubsan_maybe_instrument_member_call (stmt_p,
+ is_ctor,
+ is_dtor)))
+ {
+ wtd->instrumented_dtor_set->add (node);
+ tree result = cp_genericize_r (stmt_p, walk_subtrees, data);
+ wtd->instrumented_dtor_set->remove (node);
+ return result;
+ }
if (sanitize_flags_p (SANITIZE_VPTR) && !is_ctor)
cp_ubsan_maybe_instrument_member_call (stmt);
}
@@ -1592,6 +1606,7 @@ cp_genericize_tree (tree* t_p, bool
handle_invisiref_parm_p)
struct cp_genericize_data wtd;
wtd.p_set = new hash_set<tree>;
+ wtd.instrumented_dtor_set = new hash_set<tree>;
wtd.bind_expr_stack.create (0);
wtd.omp_ctx = NULL;
wtd.try_block = NULL_TREE;
@@ -1599,6 +1614,7 @@ cp_genericize_tree (tree* t_p, bool
handle_invisiref_parm_p)
wtd.handle_invisiref_parm_p = handle_invisiref_parm_p;
cp_walk_tree (t_p, cp_genericize_r, &wtd, NULL);
delete wtd.p_set;
+ delete wtd.instrumented_dtor_set;
if (sanitize_flags_p (SANITIZE_VPTR))
cp_ubsan_instrument_member_accesses (t_p);
}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr101355.C
b/gcc/testsuite/g++.dg/coroutines/pr101355.C
new file mode 100644
index 00000000000..cd0becbf62a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr101355.C
@@ -0,0 +1,79 @@
+// { dg-do run }
+// { dg-options "-fsanitize=undefined -Wall -Werror -std=c++20" }
+#if __has_include(<coroutine>)
+#include <coroutine>
+#elif defined(__clang__) && __has_include(<experimental/coroutine>)
+#include <experimental/coroutine>
+namespace std
+{
+using namespace experimental;
+}
+#endif
+#include <utility>
+
+struct coro
+{
+ struct promise_type
+ {
+ coro
+ get_return_object ()
+ {
+ return {};
+ }
+
+ std::suspend_never
+ initial_suspend () noexcept
+ {
+ return {};
+ }
+
+ std::suspend_never
+ final_suspend () noexcept
+ {
+ return {};
+ }
+
+ void
+ unhandled_exception () {}
+
+ void
+ return_void () {}
+ };
+
+ bool
+ await_ready ()
+ {
+ return true;
+ }
+
+ void
+ await_resume () {}
+
+ template <typename U>
+ void
+ await_suspend (U &) {}
+};
+
+struct b
+{
+ ~b () {}
+};
+
+struct a
+{
+ a (b) {}
+ ~a () {}
+};
+
+coro
+f (b obj)
+{
+ auto obj2 = a{ obj };
+ co_return;
+}
+
+int
+main ()
+{
+ f ({});
+}
