https://gcc.gnu.org/g:00b80ef1861db7653b9116f9aa148094b4e707d3

commit r13-9563-g00b80ef1861db7653b9116f9aa148094b4e707d3
Author: Jakub Jelinek <ja...@redhat.com>
Date:   Thu Oct 24 12:56:19 2024 +0200

    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  <ja...@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.
    
    (cherry picked from commit b25d3201b6338d9f71c64f524ca2974d9a1f38e8)

Diff:
---
 gcc/cp/typeck.cc                      | 31 +++++++++++++++++--------------
 gcc/testsuite/g++.dg/ubsan/pr117259.C | 13 +++++++++++++
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index cbd8068c3a2a..a53ff4f466c2 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4179,24 +4179,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 000000000000..2b7ba56c2a36
--- /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) ();
+}

Reply via email to