On Wed, Sep 04, 2024 at 10:31:48PM +0200, Franz Sirl wrote:
> Hmm, it just occured to me, how about adding !NONVIRTUAL here? When
> NONVIRTUAL is true, there is no conditional stmt at all, or?

Yeah, that makes sense, the problem doesn't happen in that case.

Here is an adjusted patch, bootstrapped/regtested on x86_64-linux
and i686-linux, ok for trunk?

2024-09-06  Jakub Jelinek  <ja...@redhat.com>

        PR c++/116449
        * typeck.cc (get_member_function_from_ptrfunc): Use save_expr
        on instance_ptr and function even if it doesn't have side-effects,
        as long as it isn't a decl.

        * g++.dg/ubsan/pr116449.C: New test.

--- gcc/cp/typeck.cc.jj 2024-09-02 17:07:30.115098114 +0200
+++ gcc/cp/typeck.cc    2024-09-04 19:08:24.127490242 +0200
@@ -4188,10 +4188,23 @@ get_member_function_from_ptrfunc (tree *
       if (!nonvirtual && is_dummy_object (instance_ptr))
        nonvirtual = true;
 
-      if (TREE_SIDE_EFFECTS (instance_ptr))
-       instance_ptr = instance_save_expr = save_expr (instance_ptr);
+      /* 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.  */
+      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;
+       }
 
-      if (TREE_SIDE_EFFECTS (function))
+      /* See above comment.  */
+      if (TREE_SIDE_EFFECTS (function)
+         || (!nonvirtual && !DECL_P (function)))
        function = save_expr (function);
 
       /* Start by extracting all the information from the PMF itself.  */
--- gcc/testsuite/g++.dg/ubsan/pr116449.C.jj    2024-09-04 18:58:46.106764285 
+0200
+++ gcc/testsuite/g++.dg/ubsan/pr116449.C       2024-09-04 18:58:46.106764285 
+0200
@@ -0,0 +1,14 @@
+// PR c++/116449
+// { dg-do compile }
+// { dg-options "-O2 -Wall -fsanitize=undefined" }
+
+struct C { void foo (int); void bar (); int c[16]; };
+typedef void (C::*P) ();
+struct D { P d; };
+static D e[1] = { { &C::bar } };
+
+void
+C::foo (int x)
+{
+  (this->*e[c[x]].d) ();
+}


        Jakub

Reply via email to