This fixes a problem where on ARM ubsan can introduce an uninitialized variable. It's ARM only since the ARM C++ ABI says that when creating a pointer to member function, the LSB of ptr discriminates between the address of a non-virtual member function and the offset in the class's virtual table of the address of a virtual function. That means the compiler will create a RSHIFT_EXPR, and with ubsan this RSHIFT_EXPR is instrumented, i.e. the expression involves SAVE_EXPRs.
But this expr is used more times and that is the crux of the problem: get_member_function_from_ptrfunc returns a tree that contains the expr, and here 4927 fn = get_member_function_from_ptrfunc (&object_addr, fn, 4928 complain); 4929 vec_safe_insert (*args, 0, object_addr); 4930 } it also saves the expr into OBJECT_ADDR which is then pushed to args. Long story short: can't use unshare_expr here, because that doesn't copy SAVE_EXPRs. I could use copy_tree_r, as outlined in the PR. But I think we can just not instrument the RSHIFT_EXPR -- we know that this one can't overflow anyway. I have tried on a cross that the problem indeed goes away. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2015-07-28 Marek Polacek <pola...@redhat.com> PR sanitizer/66977 * typeck.c (get_member_function_from_ptrfunc): Don't sanitize RSHIFT_EXPR. * g++.dg/ubsan/pr66977.C: New test. diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 2ed43be..8530be5 100644 --- gcc/cp/typeck.c +++ gcc/cp/typeck.c @@ -3288,6 +3288,7 @@ get_member_function_from_ptrfunc (tree *instance_ptrptr, tree function, idx = build1 (NOP_EXPR, vtable_index_type, e3); switch (TARGET_PTRMEMFUNC_VBIT_LOCATION) { + int flag_sanitize_save; case ptrmemfunc_vbit_in_pfn: e1 = cp_build_binary_op (input_location, BIT_AND_EXPR, idx, integer_one_node, @@ -3303,9 +3304,15 @@ get_member_function_from_ptrfunc (tree *instance_ptrptr, tree function, e1 = cp_build_binary_op (input_location, BIT_AND_EXPR, delta, integer_one_node, complain); + /* Don't instrument the RSHIFT_EXPR we're about to create because + we're going to use DELTA number of times, and that wouldn't play + well with SAVE_EXPRs therein. */ + flag_sanitize_save = flag_sanitize; + flag_sanitize = 0; delta = cp_build_binary_op (input_location, RSHIFT_EXPR, delta, integer_one_node, complain); + flag_sanitize = flag_sanitize_save; if (delta == error_mark_node) return error_mark_node; break; diff --git gcc/testsuite/g++.dg/ubsan/pr66977.C gcc/testsuite/g++.dg/ubsan/pr66977.C index e69de29..3ab8d90 100644 --- gcc/testsuite/g++.dg/ubsan/pr66977.C +++ gcc/testsuite/g++.dg/ubsan/pr66977.C @@ -0,0 +1,27 @@ +// PR sanitizer/66977 +// { dg-do compile } +// { dg-options "-fsanitize=shift -Wmaybe-uninitialized -O" } + +class Foo { + +private: + + int a_; + +public: + + Foo (int a) : a_(a) {}; + + inline int get_a () { return a_; }; +}; + +int bar (int (Foo::*get)()) { + Foo *A = new Foo(1); + int result = (A->*get)(); + delete (A); + return result; +} + +int main () { + return bar (&Foo::get_a); +} Marek