https://gcc.gnu.org/g:90a9c36dc3ba341cf662ba1d60c939027487fe9a
commit r14-10666-g90a9c36dc3ba341cf662ba1d60c939027487fe9a
Author: Jakub Jelinek <ja...@redhat.com>
Date: Tue Sep 10 18:32:58 2024 +0200
c++: Fix get_member_function_from_ptrfunc with -fsanitize=bounds [PR116449]
The following testcase is miscompiled, because
get_member_function_from_ptrfunc
emits something like
(((FUNCTION.__pfn & 1) != 0)
? ptr + FUNCTION.__delta + FUNCTION.__pfn - 1
: FUNCTION.__pfn) (ptr + FUNCTION.__delta, ...)
or so, so FUNCTION tree is used there 5 times. There is
if (TREE_SIDE_EFFECTS (function)) function = save_expr (function);
but in this case function doesn't have side-effects, just nested ARRAY_REFs.
Now, if all the FUNCTION trees would be shared, it would work fine,
FUNCTION is evaluated in the first operand of COND_EXPR; but unfortunately
that isn't the case, both the BIT_AND_EXPR shortening and conversion to
bool done for build_conditional_expr actually unshare_expr that first
expression, but none of the other 4 are unshared. With -fsanitize=bounds,
.UBSAN_BOUNDS calls are added to the ARRAY_REFs and use save_expr to avoid
evaluating the argument multiple times, but because that FUNCTION tree is
first used in the second argument of COND_EXPR (i.e. conditionally), the
SAVE_EXPR initialization is done just there and then the third argument
of COND_EXPR just uses the uninitialized temporary and so does the first
argument computation as well.
The following patch fixes that by doing save_expr even if
!TREE_SIDE_EFFECTS,
but to avoid doing that too often only if !nonvirtual and if the expression
isn't a simple decl.
2024-09-10 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.
(cherry picked from commit 0008050b9d6046ba4e811a03b406fb5d98707cae)
Diff:
---
gcc/cp/typeck.cc | 19 ++++++++++++++++---
gcc/testsuite/g++.dg/ubsan/pr116449.C | 14 ++++++++++++++
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 21436f836fa6..3d62943f8647 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4175,10 +4175,23 @@ get_member_function_from_ptrfunc (tree
*instance_ptrptr, tree function,
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. */
diff --git a/gcc/testsuite/g++.dg/ubsan/pr116449.C
b/gcc/testsuite/g++.dg/ubsan/pr116449.C
new file mode 100644
index 000000000000..f13368a51b00
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ubsan/pr116449.C
@@ -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) ();
+}
