On Fri, Sep 20, 2013 at 10:37:57AM +0200, Jakub Jelinek wrote:
> On Fri, Sep 20, 2013 at 10:22:43AM +0200, Marek Polacek wrote:
> > > --- gcc/ubsan.c.mp 2013-09-16 18:13:01.075903156 +0200
> > > +++ gcc/ubsan.c 2013-09-16 18:13:20.514974154 +0200
> > > @@ -233,7 +233,13 @@ ubsan_source_location (location_t loc)
> > > static unsigned short
> > > get_ubsan_type_info_for_type (tree type)
> > > {
> > > - int prec = exact_log2 (TYPE_PRECISION (type));
> > > + int prec = TYPE_PRECISION (type);
> > > +
> > > + /* Handle bit-fields. */
> > > + if (compare_tree_int (TYPE_SIZE (type), prec) == 1)
> > > + prec = tree_low_cst (TYPE_SIZE (type), 1);
>
> Makes me wonder why you are using then TYPE_PRECISION at all, when
> you actually want to use TYPE_SIZE.
Yeah, we might as well use TYPE_SIZE directly...
> Note that TYPE_SIZE can be NULL (for incomplete types)
> or non-constant (VLAs) or big enough not to fit into a HWI.
> But you are so far dealing only with integral/scalar float types, right?
Yes, so far only INTEGER_TYPEs. I added the assert nonetheless.
> So perhaps just gcc_assert (TYPE_SIZE (type) && host_integerp (TYPE_SIZE
> (type), 1)
> or something.
>
> > > +
> > > + prec = exact_log2 (prec);
> > > if (prec == -1)
> > > error ("unexpected size of type %qT", type);
>
> This sounds like it should be gcc_assert (prec != -1); or
> sorry, it doesn't look like a bug in user program if we hit that.
Agreed. Added that assert in there. How does it look now?
Regtested/bootstrap-ubsan passed on x86_64-linux.
2013-09-20 Marek Polacek <pola...@redhat.com>
PR sanitizer/58413
* ubsan.c (get_ubsan_type_info_for_type): Use TYPE_SIZE instead of
TYPE_PRECISION. Add asserts.
testsuite/
* c-c++-common/ubsan/shift-4.c: New test.
--- gcc/ubsan.c.mp 2013-09-20 13:31:31.491652058 +0200
+++ gcc/ubsan.c 2013-09-20 13:36:31.159335560 +0200
@@ -233,10 +233,9 @@ ubsan_source_location (location_t loc)
static unsigned short
get_ubsan_type_info_for_type (tree type)
{
- int prec = exact_log2 (TYPE_PRECISION (type));
- if (prec == -1)
- error ("unexpected size of type %qT", type);
-
+ gcc_assert (TYPE_SIZE (type) && host_integerp (TYPE_SIZE (type), 1));
+ int prec = exact_log2 (tree_low_cst (TYPE_SIZE (type), 1));
+ gcc_assert (prec != -1);
return (prec << 1) | !TYPE_UNSIGNED (type);
}
--- gcc/testsuite/c-c++-common/ubsan/shift-4.c.mp 2013-09-20
13:34:36.245909813 +0200
+++ gcc/testsuite/c-c++-common/ubsan/shift-4.c 2013-09-20 13:34:25.419870385
+0200
@@ -0,0 +1,14 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift -w" } */
+
+struct S { unsigned long long int b:40; } s;
+
+int
+main ()
+{
+ s.b = 2;
+ s.b <<= 120;
+ return 0;
+}
+
+/* { dg-output "shift exponent 120 is too large\[^\n\r]*(\n|\r\n|\r)" } */
Marek
