This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix uninitialized variable with ubsan on ARM (PR sanitizer/66977)


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  <polacek@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]