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]

Re: [PATCH] Handle bit-fields in ubsan.c (PR sanitizer/58413)


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


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