| Message ID | ZttMPuWWdMPV5haM@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | c++, v3: Fix get_member_function_from_ptrfunc with -fsanitize=bounds [PR116449] | expand |
On 9/6/24 2:38 PM, Jakub Jelinek wrote: > 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? OK. > 2024-09-06 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,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 >
--- 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) (); +}