| Message ID | 20160506092241.GO5348@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, May 06, 2016 at 11:22:41AM +0200, Marek Polacek wrote: > A program containing an array of structs containing a VLA caused ICE with UBSAN > bounds checking, because in get_ubsan_type_info_for_type we asserted that the > size of a type fits uhwi, which implies it is an INTEGER_CST. But that's not > the case for a struct with VLA. However, the assert here is bogus, for > !REAL_TYPE and !INTEGRAL_TYPE_P get_ubsan_type_info_for_type just returns 0. > And since tree_to_uhwi has > gcc_assert (tree_fits_uhwi_p (t)); > there's no need to duplicate that for the REAL_TYPE / INTEGRAL_TYPE_P cases. Yeah, and for NULL TYPE_SIZE we just segfault, not really need to assert that. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? Ok, thanks. If it affects 6.x branch, it is ok there as well. > 2016-05-06 Marek Polacek <polacek@redhat.com> > > PR sanitizer/70875 > * ubsan.c (get_ubsan_type_info_for_type): Remove assert. > > * gcc.dg/ubsan/bounds-3.c: New test. Jakub
On Fri, May 06, 2016 at 11:29:33AM +0200, Jakub Jelinek wrote: > On Fri, May 06, 2016 at 11:22:41AM +0200, Marek Polacek wrote: > > A program containing an array of structs containing a VLA caused ICE with UBSAN > > bounds checking, because in get_ubsan_type_info_for_type we asserted that the > > size of a type fits uhwi, which implies it is an INTEGER_CST. But that's not > > the case for a struct with VLA. However, the assert here is bogus, for > > !REAL_TYPE and !INTEGRAL_TYPE_P get_ubsan_type_info_for_type just returns 0. > > And since tree_to_uhwi has > > gcc_assert (tree_fits_uhwi_p (t)); > > there's no need to duplicate that for the REAL_TYPE / INTEGRAL_TYPE_P cases. > > Yeah, and for NULL TYPE_SIZE we just segfault, not really need to assert > that. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > Ok, thanks. If it affects 6.x branch, it is ok there as well. Yeah, it does, and since I need to test a backport for PR70342 anyway, I'll add it to my testing & commit it afterwards. Thanks, Marek
diff --git gcc/testsuite/gcc.dg/ubsan/bounds-3.c gcc/testsuite/gcc.dg/ubsan/bounds-3.c index e69de29..50ad673 100644 --- gcc/testsuite/gcc.dg/ubsan/bounds-3.c +++ gcc/testsuite/gcc.dg/ubsan/bounds-3.c @@ -0,0 +1,22 @@ +/* PR sanitizer/70875 */ +/* { dg-do run } */ +/* { dg-options "-fsanitize=bounds" } */ + +int +foo (int n, int k) +{ + struct S + { + int i[n]; + int value; + } s[2]; + return s[k].value = 0; +} + +int +main () +{ + return foo (2, 2); +} + +/* { dg-output "index 2 out of bounds for type 'S \\\[2\\\]'" } */ diff --git gcc/ubsan.c gcc/ubsan.c index 802341e..c5543f8 100644 --- gcc/ubsan.c +++ gcc/ubsan.c @@ -302,7 +302,6 @@ ubsan_source_location (location_t loc) static unsigned short get_ubsan_type_info_for_type (tree type) { - gcc_assert (TYPE_SIZE (type) && tree_fits_uhwi_p (TYPE_SIZE (type))); if (TREE_CODE (type) == REAL_TYPE) return tree_to_uhwi (TYPE_SIZE (type)); else if (INTEGRAL_TYPE_P (type))