aboutsummaryrefslogtreecommitdiff
path: root/gcc
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2024-10-24 12:56:19 +0200
committerJakub Jelinek <jakub@gcc.gnu.org>2024-10-24 12:56:19 +0200
commitb25d3201b6338d9f71c64f524ca2974d9a1f38e8 (patch)
tree2a0e03718a62cbfd4bec869ac5026382852b6be7 /gcc
parent885143fa77599c44bfdd4e8e6b6987b7824db6ba (diff)
downloadgcc-b25d3201b6338d9f71c64f524ca2974d9a1f38e8.zip
gcc-b25d3201b6338d9f71c64f524ca2974d9a1f38e8.tar.gz
gcc-b25d3201b6338d9f71c64f524ca2974d9a1f38e8.tar.bz2
c++: Further fix for get_member_function_from_ptrfunc [PR117259]
The following testcase shows that the previous get_member_function_from_ptrfunc changes weren't sufficient and we still have cases where -fsanitize=undefined with pointers to member functions can cause wrong code being generated and related false positive warnings. The problem is that save_expr doesn't always create SAVE_EXPR, it can skip some invariant arithmetics and in the end it could be really large expressions which would be evaluated several times (and what is worse, with -fsanitize=undefined those expressions then can have SAVE_EXPRs added to their subparts for -fsanitize=bounds or -fsanitize=null or -fsanitize=alignment instrumentation). Tried to just build1 a SAVE_EXPR + add TREE_SIDE_EFFECTS instead of save_expr, but that doesn't work either, because cp_fold happily optimizes those SAVE_EXPRs away when it sees SAVE_EXPR operand is tree_invariant_p. So, the following patch instead of using save_expr or building SAVE_EXPR manually builds a TARGET_EXPR. Both types are pointers, so it doesn't need to be destroyed in any way, but TARGET_EXPR is what doesn't get optimized away immediately. 2024-10-24 Jakub Jelinek <jakub@redhat.com> PR c++/117259 * typeck.cc (get_member_function_from_ptrfunc): Use force_target_expr rather than save_expr for instance_ptr and function. Don't call it for TREE_CONSTANT. * g++.dg/ubsan/pr117259.C: New test.
Diffstat (limited to 'gcc')
-rw-r--r--gcc/cp/typeck.cc31
-rw-r--r--gcc/testsuite/g++.dg/ubsan/pr117259.C13
2 files changed, 30 insertions, 14 deletions
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 71d879a..bfc0c56 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4193,24 +4193,27 @@ get_member_function_from_ptrfunc (tree *instance_ptrptr, tree function,
if (!nonvirtual && is_dummy_object (instance_ptr))
nonvirtual = true;
- /* Use save_expr even when instance_ptr doesn't have side-effects,
- unless it is a simple decl (save_expr won't do anything on
- constants), so that we don't ubsan instrument the expression
- multiple times. See PR116449. */
+ /* Use force_target_expr even when instance_ptr doesn't have
+ side-effects, unless it is a simple decl or constant, so
+ that we don't ubsan instrument the expression multiple times.
+ Don't use save_expr, as save_expr can avoid building a SAVE_EXPR
+ and building a SAVE_EXPR manually can be optimized away during
+ cp_fold. See PR116449 and PR117259. */
if (TREE_SIDE_EFFECTS (instance_ptr)
- || (!nonvirtual && !DECL_P (instance_ptr)))
- {
- instance_save_expr = save_expr (instance_ptr);
- if (instance_save_expr == instance_ptr)
- instance_save_expr = NULL_TREE;
- else
- instance_ptr = instance_save_expr;
- }
+ || (!nonvirtual
+ && !DECL_P (instance_ptr)
+ && !TREE_CONSTANT (instance_ptr)))
+ instance_ptr = instance_save_expr
+ = force_target_expr (TREE_TYPE (instance_ptr), instance_ptr,
+ complain);
/* See above comment. */
if (TREE_SIDE_EFFECTS (function)
- || (!nonvirtual && !DECL_P (function)))
- function = save_expr (function);
+ || (!nonvirtual
+ && !DECL_P (function)
+ && !TREE_CONSTANT (function)))
+ function
+ = force_target_expr (TREE_TYPE (function), function, complain);
/* Start by extracting all the information from the PMF itself. */
e3 = pfn_from_ptrmemfunc (function);
diff --git a/gcc/testsuite/g++.dg/ubsan/pr117259.C b/gcc/testsuite/g++.dg/ubsan/pr117259.C
new file mode 100644
index 0000000..2b7ba56
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ubsan/pr117259.C
@@ -0,0 +1,13 @@
+// PR c++/117259
+// { dg-do compile }
+// { dg-options "-Wuninitialized -fsanitize=undefined" }
+
+struct A { void foo () {} };
+struct B { void (A::*b) (); B (void (A::*x) ()) : b(x) {}; };
+const B c[1] = { &A::foo };
+
+void
+foo (A *x, int y)
+{
+ (x->*c[y].b) ();
+}