@@ -313,7 +313,7 @@ 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));
+ t = fold_build2 (LT_EXPR, boolean_type_node, size, build_int_cst (type, 0));
if (flag_sanitize_trap & SANITIZE_VLA)
tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0);
else
@@ -7201,16 +7201,20 @@ grokdeclarator (const struct c_declarator *declarator,
with known value. */
this_size_varies = size_varies = true;
warn_variable_length_array (name, size);
- if (sanitize_flags_p (SANITIZE_VLA)
- && current_function_decl != NULL_TREE
- && decl_context == NORMAL)
+ if (sanitize_flags_p (SANITIZE_VLA))
{
/* Evaluate the array size only once. */
size = save_expr (size);
size = c_fully_fold (size, false, NULL);
- size = fold_build2 (COMPOUND_EXPR, TREE_TYPE (size),
- ubsan_instrument_vla (loc, size),
- size);
+ tree instr = ubsan_instrument_vla (loc, size);
+ /* We have to build this in the right order, so
+ instrumentation is done before the size can
+ be used in other parameters. */
+ if (*expr)
+ *expr = build2 (COMPOUND_EXPR, TREE_TYPE (instr),
+ *expr, instr);
+ else
+ *expr = instr;
}
}
@@ -110,9 +110,7 @@ main (void)
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -5\[^\n\r]*(\n|\r\n|\r)" } */
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -3\[^\n\r]*(\n|\r\n|\r)" } */
-/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value 0\[^\n\r]*(\n|\r\n|\r)" } */
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
-/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value 0\[^\n\r]*(\n|\r\n|\r)" } */
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -6\[^\n\r]*(\n|\r\n|\r)" } */
new file mode 100644
@@ -0,0 +1,16 @@
+/* PR 98608 */
+/* { dg-do run } */
+/* { dg-options "-fsanitize=vla-bound" } */
+
+void f(int n, double (*x)[n])
+{
+ typeof(*x) y;
+}
+
+int main()
+{
+ f(-1, 0);
+}
+
+/* { dg-output "variable length array bound evaluates to non-positive value -1" } */
+
new file mode 100644
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-options "-Wstringop-overflow=0 -fsanitize=vla-bound -fsanitize-recover=vla-bound" } */
+
+void f1(int n, char buf[n]) { }
+/* { dg-output "variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
+void f2(int n, float m[n][2]) { }
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
+void f3(int n, int m, float x[n][m]) { }
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
+void f4(int n) { typedef char buf[n]; }
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
+void f5(int n) { struct { char buf[n]; } x; }
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1\[^\n\r]*(\n|\r\n|\r)" } */
+void f6(int n, struct { char buf[n]; } *p) { } /* { dg-warning "anonymous" } */
+/* { dg-output "\[^\n\r]*variable length array bound evaluates to non-positive value -1" } */
+
+int main()
+{
+ f1(-1, 0);
+ f2(-1, 0);
+ f3(-1, 1, 0);
+ f4(-1);
+ f5(-1);
+ f6(-1, 0);
+}
+
+
new file mode 100644
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+/* { dg-options "-Wstringop-overflow=0 -fsanitize=vla-bound -fsanitize-recover=vla-bound" } */
+
+void f1(int n, char buf[n]) { }
+void f2(int n, float m[n][2]) { }
+void f3(int n, int m, float x[n][m]) { }
+void f4(int n) { typedef char buf[n]; }
+void f5(int n) { struct { char buf[n]; } x; }
+void f6(int n, struct { char buf[n]; } *p) { } /* { dg-warning "anonymous" } */
+
+int main()
+{
+ f1(0, 0);
+ f2(0, 0);
+ f3(0, 1, 0);
+ f4(0);
+ f5(0);
+ f6(0, 0);
+}
+
+
new file mode 100644
@@ -0,0 +1,14 @@
+/* { dg-do run }
+ * { dg-options "-fsanitize=vla-bound" } */
+
+void foo(int n, char (*buf)[n], char p[n = 1])
+{
+}
+
+int main()
+{
+ foo(-1, 0, 0);
+}
+
+/* { dg-output "variable length array bound evaluates to non-positive value -1" } */
+
Here is a patch that adds the missing cases for vla size instrumentation. This now includes all cases where a type with size < 0 is created, which is already UB and not just cases where a VLA is allocated. But a VLA can be allocated based on an typedef, which is also now indirectly protected in this way. I moved the instrumentation from the size itself as its own term into the expression that evaluate size expressions for side effects. This avoids confusing other warning code that looks at the size expressions (-Wvla-parameter). There is one open question though: How to to treat n == 0? Here I preliminary changed this to n > 0 (also for the existing case), because when also detecting n == 0 this tools especially when instrumenting all the types becomes basically useless because of the very common (and unproblematic) use of n == 0. But strictly speaking n == 0 is also UB and as pointed out in PR98609 the error message is then not entirely accurate because it says non-positive and not negative. I do not think it is confusing though because it is still always correct. One could consider splitting it up into vla-bound / vla-bound-strict, but changing the error message would require further upstream changes and dealing with this far exceeds the time I can afford contributing to this this. Another complication is that we ran out of bits for sanitizer flags in unsigned int, so this would also require more changes. Any advice? I think it would be important to have complete UBSan coverage for all size and bounds issues related to VM types and it would be nice to get this in GCC 14. (I find this extremely useful in my projects). Martin c: Add missing cases where vla sizes are not instrumented by UBSan [PR98608] Add vla-bound instrumentation for all VLAs including VLAs in parameters and fields, but allow zero-sized errors. Bootstrapped and regression tested on x86. PR c/98608 gcc/c: * c-decl.cc (grokdeclarator): Instrument all VLAs. gcc/c-family: * c-ubsan.cc (ubsan_instrument_vla): Do not include zero length. gcc/testsuite: * gcc.dg/ubsan/pr89608.c: New test. * gcc.dg/ubsan/vla-1.c: New test. * gcc.dg/ubsan/vla-2.c: New test. * gcc.dg/ubsan/vla-3.c: New test. * c-c++-common/ubsan/vla-1.c: Adapt.