Implement -fsanitize=vla-bound.
authorMarek Polacek <polacek@redhat.com>
Sun, 3 Nov 2013 17:59:31 +0000 (17:59 +0000)
committerMarek Polacek <mpolacek@gcc.gnu.org>
Sun, 3 Nov 2013 17:59:31 +0000 (17:59 +0000)
* opts.c (common_handle_option): Handle vla-bound.
* sanitizer.def (BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE):
Define.
* flag-types.h (enum sanitize_code): Add SANITIZE_VLA.
* asan.c (initialize_sanitizer_builtins): Build BT_FN_VOID_PTR_PTR.
c-family/
* c-ubsan.c: Don't include hash-table.h.
(ubsan_instrument_vla): New function.
* c-ubsan.h: Declare it.
cp/
* decl.c (cp_finish_decl): Move C++1y bounds checking...
(compute_array_index_type): ...here.  Add VLA instrumentation.
Call stabilize_vla_size.
(grokdeclarator): Don't call stabilize_vla_size here.
c/
* c-decl.c (grokdeclarator): Add VLA instrumentation.
testsuite/
* g++.dg/ubsan/cxx1y-vla.C: New test.
* c-c++-common/ubsan/vla-3.c: New test.
* c-c++-common/ubsan/vla-2.c: New test.
* c-c++-common/ubsan/vla-4.c: New test.
* c-c++-common/ubsan/vla-1.c: New test.

From-SVN: r204334

18 files changed:
gcc/ChangeLog
gcc/asan.c
gcc/c-family/ChangeLog
gcc/c-family/c-ubsan.c
gcc/c-family/c-ubsan.h
gcc/c/ChangeLog
gcc/c/c-decl.c
gcc/cp/ChangeLog
gcc/cp/decl.c
gcc/flag-types.h
gcc/opts.c
gcc/sanitizer.def
gcc/testsuite/ChangeLog
gcc/testsuite/c-c++-common/ubsan/vla-1.c [new file with mode: 0644]
gcc/testsuite/c-c++-common/ubsan/vla-2.c [new file with mode: 0644]
gcc/testsuite/c-c++-common/ubsan/vla-3.c [new file with mode: 0644]
gcc/testsuite/c-c++-common/ubsan/vla-4.c [new file with mode: 0644]
gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C [new file with mode: 0644]

index cf54baf..cc163f1 100644 (file)
@@ -1,3 +1,12 @@
+2013-11-03  Marek Polacek  <polacek@redhat.com>
+
+       Implement -fsanitize=vla-bound.
+       * opts.c (common_handle_option): Handle vla-bound.
+       * sanitizer.def (BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE):
+       Define.
+       * flag-types.h (enum sanitize_code): Add SANITIZE_VLA.
+       * asan.c (initialize_sanitizer_builtins): Build BT_FN_VOID_PTR_PTR.
+
 2013-11-02  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
 
        * config/rs6000/rs6000.c (rs6000_expand_vector_set): Adjust for
index 8139719..763c59a 100644 (file)
@@ -2021,6 +2021,9 @@ initialize_sanitizer_builtins (void)
   tree BT_FN_VOID = build_function_type_list (void_type_node, NULL_TREE);
   tree BT_FN_VOID_PTR
     = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE);
+  tree BT_FN_VOID_PTR_PTR
+    = build_function_type_list (void_type_node, ptr_type_node,
+                               ptr_type_node, NULL_TREE);
   tree BT_FN_VOID_PTR_PTR_PTR
     = build_function_type_list (void_type_node, ptr_type_node,
                                ptr_type_node, ptr_type_node, NULL_TREE);
index 13a6687..569f4c2 100644 (file)
@@ -1,3 +1,9 @@
+2013-11-03  Marek Polacek  <polacek@redhat.com>
+
+       * c-ubsan.c: Don't include hash-table.h.
+       (ubsan_instrument_vla): New function.
+       * c-ubsan.h: Declare it.
+
 2013-10-31  David Malcolm  <dmalcolm@redhat.com>
 
        Automated part of renaming of symtab_node_base to symtab_node.
index 0bfc660..c989638 100644 (file)
@@ -25,7 +25,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 #include "cgraph.h"
 #include "gimple.h"
-#include "hash-table.h"
 #include "output.h"
 #include "toplev.h"
 #include "ubsan.h"
@@ -86,8 +85,7 @@ ubsan_instrument_division (location_t loc, tree op0, tree op1)
   return t;
 }
 
-/* Instrument left and right shifts.  If not instrumenting, return
-   NULL_TREE.  */
+/* Instrument left and right shifts.  */
 
 tree
 ubsan_instrument_shift (location_t loc, enum tree_code code,
@@ -158,3 +156,22 @@ ubsan_instrument_shift (location_t loc, enum tree_code code,
 
   return t;
 }
+
+/* Instrument variable length array bound.  */
+
+tree
+ubsan_instrument_vla (location_t loc, tree size)
+{
+  tree type = TREE_TYPE (size);
+  tree t, tt;
+
+  t = fold_build2 (LE_EXPR, boolean_type_node, size, build_int_cst (type, 0));
+  tree data = ubsan_create_data ("__ubsan_vla_data",
+                                loc, ubsan_type_descriptor (type), NULL_TREE);
+  data = build_fold_addr_expr_loc (loc, data);
+  tt = builtin_decl_explicit (BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE);
+  tt = build_call_expr_loc (loc, tt, 2, data, ubsan_encode_value (size));
+  t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node);
+
+  return t;
+}
index b032b70..fdf27d9 100644 (file)
@@ -23,5 +23,6 @@ along with GCC; see the file COPYING3.  If not see
 
 extern tree ubsan_instrument_division (location_t, tree, tree);
 extern tree ubsan_instrument_shift (location_t, enum tree_code, tree, tree);
+extern tree ubsan_instrument_vla (location_t, tree);
 
 #endif  /* GCC_C_UBSAN_H  */
index f009ee9..efb4ba8 100644 (file)
@@ -1,3 +1,7 @@
+2013-11-03  Marek Polacek  <polacek@redhat.com>
+
+       * c-decl.c (grokdeclarator): Add VLA instrumentation.
+
 2013-11-01  Jakub Jelinek  <jakub@redhat.com>
 
        * c-typeck.c (c_finish_omp_clauses) <case OMP_CLAUSE_UNIFORM>: Go to
index 64718c5..2833fdb 100644 (file)
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-common.h"
 #include "c-family/c-objc.h"
 #include "c-family/c-pragma.h"
+#include "c-family/c-ubsan.h"
 #include "c-lang.h"
 #include "langhooks.h"
 #include "tree-iterator.h"
@@ -5411,6 +5412,16 @@ grokdeclarator (const struct c_declarator *declarator,
                       with known value.  */
                    this_size_varies = size_varies = true;
                    warn_variable_length_array (name, size);
+                   if (flag_sanitize & SANITIZE_VLA
+                       && decl_context == NORMAL)
+                     {
+                       /* Evaluate the array size only once.  */
+                       size = c_save_expr (size);
+                       size = c_fully_fold (size, false, NULL);
+                       size = fold_build2 (COMPOUND_EXPR, TREE_TYPE (size),
+                                           ubsan_instrument_vla (loc, size),
+                                           size);
+                     }
                  }
 
                if (integer_zerop (size) && !this_size_varies)
index 694aaae..cadf674 100644 (file)
@@ -1,3 +1,10 @@
+2013-11-03  Marek Polacek  <polacek@redhat.com>
+
+       * decl.c (cp_finish_decl): Move C++1y bounds checking...
+       (compute_array_index_type): ...here.  Add VLA instrumentation.
+       Call stabilize_vla_size.
+       (grokdeclarator): Don't call stabilize_vla_size here.
+
 2013-11-02  Paolo Carlini  <paolo.carlini@oracle.com>
 
        PR c++/29234
index 09c1daa..e662764 100644 (file)
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-objc.h"
 #include "c-family/c-pragma.h"
 #include "c-family/c-target.h"
+#include "c-family/c-ubsan.h"
 #include "diagnostic.h"
 #include "intl.h"
 #include "debug.h"
@@ -6399,17 +6400,6 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
           && TYPE_FOR_JAVA (type) && MAYBE_CLASS_TYPE_P (type))
     error ("non-static data member %qD has Java class type", decl);
 
-  if (cxx_dialect >= cxx1y && array_of_runtime_bound_p (type))
-    {
-      /* If the VLA bound is larger than half the address space, or less
-        than zero, throw std::bad_array_length.  */
-      tree max = convert (ssizetype, TYPE_MAX_VALUE (TYPE_DOMAIN (type)));
-      tree comp = build2 (LT_EXPR, boolean_type_node, max, ssize_int (-1));
-      comp = build3 (COND_EXPR, void_type_node, comp,
-                    throw_bad_array_length (), void_zero_node);
-      finish_expr_stmt (comp);
-    }
-
   /* Add this declaration to the statement-tree.  This needs to happen
      after the call to check_initializer so that the DECL_EXPR for a
      reference temp is added before the DECL_EXPR for the reference itself.  */
@@ -8379,6 +8369,7 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain)
        {
          /* A variable sized array.  */
          itype = variable_size (itype);
+
          if (TREE_CODE (itype) != SAVE_EXPR)
            {
              /* Look for SIZEOF_EXPRs in itype and fold them, otherwise
@@ -8390,6 +8381,32 @@ compute_array_index_type (tree name, tree size, tsubst_flags_t complain)
              if (found)
                itype = variable_size (fold (newitype));
            }
+
+         stabilize_vla_size (itype);
+
+         if (cxx_dialect >= cxx1y)
+           {
+             /* If the VLA bound is larger than half the address space,
+                or less than zero, throw std::bad_array_length.  */
+             tree comp = build2 (LT_EXPR, boolean_type_node, itype,
+                                 ssize_int (-1));
+             comp = build3 (COND_EXPR, void_type_node, comp,
+                            throw_bad_array_length (), void_zero_node);
+             finish_expr_stmt (comp);
+           }
+         else if (flag_sanitize & SANITIZE_VLA)
+           {
+             /* From C++1y onwards, we throw an exception on a negative
+                length size of an array; see above.  */
+
+             /* We have to add 1 -- in the ubsan routine we generate
+                LE_EXPR rather than LT_EXPR.  */
+             tree t = fold_build2 (PLUS_EXPR, TREE_TYPE (itype), itype,
+                                   build_one_cst (TREE_TYPE (itype)));
+             t = fold_build2 (COMPOUND_EXPR, TREE_TYPE (t),
+                              ubsan_instrument_vla (input_location, t), t);
+             finish_expr_stmt (t);
+           }
        }
       /* Make sure that there was no overflow when creating to a signed
         index type.  (For example, on a 32-bit machine, an array with
@@ -9790,12 +9807,8 @@ grokdeclarator (const cp_declarator *declarator,
              && (decl_context == NORMAL || decl_context == FIELD)
              && at_function_scope_p ()
              && variably_modified_type_p (type, NULL_TREE))
-           {
-             /* First break out any side-effects.  */
-             stabilize_vla_size (TYPE_SIZE (type));
-             /* And then force evaluation of the SAVE_EXPR.  */
-             finish_expr_stmt (TYPE_SIZE (type));
-           }
+           /* Force evaluation of the SAVE_EXPR.  */
+           finish_expr_stmt (TYPE_SIZE (type));
 
          if (declarator->kind == cdk_reference)
            {
@@ -9886,14 +9899,6 @@ grokdeclarator (const cp_declarator *declarator,
        }
     }
 
-  /* We need to stabilize side-effects in VLA sizes for regular array
-     declarations too, not just pointers to arrays.  */
-  if (type != error_mark_node && !TYPE_NAME (type)
-      && (decl_context == NORMAL || decl_context == FIELD)
-      && at_function_scope_p ()
-      && variably_modified_type_p (type, NULL_TREE))
-    stabilize_vla_size (TYPE_SIZE (type));
-
   /* A `constexpr' specifier used in an object declaration declares
      the object as `const'.  */
   if (constexpr_p && innermost_code != cdk_function)
index a2be8bb..7d0ac35 100644 (file)
@@ -210,7 +210,9 @@ enum sanitize_code {
   SANITIZE_SHIFT = 1 << 2,
   SANITIZE_DIVIDE = 1 << 3,
   SANITIZE_UNREACHABLE = 1 << 4,
+  SANITIZE_VLA = 1 << 5,
   SANITIZE_UNDEFINED = SANITIZE_SHIFT | SANITIZE_DIVIDE | SANITIZE_UNREACHABLE
+                      | SANITIZE_VLA
 };
 
 /* flag_vtable_verify initialization levels. */
index 1d02b10..4db20f0 100644 (file)
@@ -1444,6 +1444,7 @@ common_handle_option (struct gcc_options *opts,
              { "undefined", SANITIZE_UNDEFINED, sizeof "undefined" - 1 },
              { "unreachable", SANITIZE_UNREACHABLE,
                sizeof "unreachable" - 1 },
+             { "vla-bound", SANITIZE_VLA, sizeof "vla-bound" - 1 },
              { NULL, 0, 0 }
            };
            const char *comma;
index 4c8a037..c7c7809 100644 (file)
@@ -297,3 +297,7 @@ DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_BUILTIN_UNREACHABLE,
                      "__ubsan_handle_builtin_unreachable",
                      BT_FN_VOID_PTR,
                      ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST)
+DEF_SANITIZER_BUILTIN(BUILT_IN_UBSAN_HANDLE_VLA_BOUND_NOT_POSITIVE,
+                     "__ubsan_handle_vla_bound_not_positive",
+                     BT_FN_VOID_PTR_PTR,
+                     ATTR_COLD_NOTHROW_LEAF_LIST)
index e0dad49..bf66f30 100644 (file)
@@ -1,3 +1,11 @@
+2013-11-03  Marek Polacek  <polacek@redhat.com>
+
+       * g++.dg/ubsan/cxx1y-vla.C: New test.
+       * c-c++-common/ubsan/vla-3.c: New test.
+       * c-c++-common/ubsan/vla-2.c: New test.
+       * c-c++-common/ubsan/vla-4.c: New test.
+       * c-c++-common/ubsan/vla-1.c: New test.
+
 2013-11-02  Bill Schmidt  <wschmidt@vnet.linux.ibm.com>
 
        * gcc.dg/vmx/vec-set.c: New.
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-1.c b/gcc/testsuite/c-c++-common/ubsan/vla-1.c
new file mode 100644 (file)
index 0000000..6c1d81e
--- /dev/null
@@ -0,0 +1,48 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=vla-bound -w" } */
+
+static int
+bar (void)
+{
+  return -42;
+}
+
+typedef long int V;
+int
+main (void)
+{
+  int x = -1;
+  double di = -3.2;
+  V v = -666;
+
+  int a[x];
+  int aa[x][x];
+  int aaa[x][x][x];
+  int b[x - 4];
+  int c[(int) di];
+  int d[1 + x];
+  int e[1 ? x : -1];
+  int f[++x];
+  int g[(signed char) --x];
+  int h[(++x, --x, x)];
+  int i[v];
+  int j[bar ()];
+
+  return 0;
+}
+
+/* { dg-output "variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -5(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -3(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value 0(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value 0(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -666(\n|\r\n|\r)" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -42(\n|\r\n|\r)" } */
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-2.c b/gcc/testsuite/c-c++-common/ubsan/vla-2.c
new file mode 100644 (file)
index 0000000..2fbeb71
--- /dev/null
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=vla-bound -w" } */
+
+int
+main (void)
+{
+  const int t = 0;
+  struct s {
+    int x;
+    /* Don't instrument this one.  */
+    int g[t];
+  };
+
+  return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-3.c b/gcc/testsuite/c-c++-common/ubsan/vla-3.c
new file mode 100644 (file)
index 0000000..20dd38b
--- /dev/null
@@ -0,0 +1,16 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=vla-bound -w" } */
+
+/* Don't instrument the arrays here.  */
+int
+foo (int n, int a[])
+{
+  return a[n - 1];
+}
+
+int
+main (void)
+{
+  int a[6] = { };
+  return foo (3, a);
+}
diff --git a/gcc/testsuite/c-c++-common/ubsan/vla-4.c b/gcc/testsuite/c-c++-common/ubsan/vla-4.c
new file mode 100644 (file)
index 0000000..3f7dd1d
--- /dev/null
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=vla-bound" } */
+
+int
+main (void)
+{
+  int x = 1;
+  /* Check that the size of an array is evaluated only once.  */
+  int a[++x];
+  if (x != 2)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C b/gcc/testsuite/g++.dg/ubsan/cxx1y-vla.C
new file mode 100644 (file)
index 0000000..350db3f
--- /dev/null
@@ -0,0 +1,13 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=vla-bound -w -std=c++1y" } */
+/* { dg-shouldfail "ubsan" } */
+
+int
+main (void)
+{
+  int y = -18;
+  int a[y];
+  return 0;
+}
+
+/* { dg-output "terminate called after throwing an instance" } */