| Message ID | ZtiVAaH17KaG7U9y@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++, v2: Fix get_member_function_from_ptrfunc with -fsanitize=bounds [PR116449] | expand |
Am 2024-09-04 um 19:12 schrieb Jakub Jelinek: > On Wed, Sep 04, 2024 at 12:34:04PM -0400, Jason Merrill wrote: >>> So, one possibility would be to call save_expr unconditionally in >>> get_member_function_from_ptrfunc as well. >>> >>> Or build a TARGET_EXPR (force_target_expr or similar). >> >> Yes. I don't have a strong preference between the two. > > Here is a patch that uses save_expr but uses it still conditionally, > doesn't make sense to use it for the common case of just decls, there is > nothing to unshare in that case. > > Passed the test, ok if it passes full bootstrap/regtest? > > 2024-09-04 Jakub Jelinek <jakub@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,21 @@ 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) || !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) || !DECL_P (function)) > function = save_expr (function); Hmm, it just occured to me, how about adding !NONVIRTUAL here? When NONVIRTUAL is true, there is no conditional stmt at all, or? Franz
--- 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,21 @@ 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) || !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) || !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) (); +}