Handle bit-fields in ubsan.c (PR sanitizer/58413)

On Mon, Sep 16, 2013 at 06:04:23PM +0200, Jakub Jelinek wrote:
> On Mon, Sep 16, 2013 at 05:59:28PM +0200, Marek Polacek wrote:
> > Regtested/ran bootstrap-ubsan on x86_64-linux.
> 
> That looks wrong.  ubsan_type_descriptor shouldn't change TYPE_PRECISION of
> the type it has been called with, whether type is a bitfield or not can
> change what typedescriptor is generated, but not unrelated code.

Ok, that was weird.  What about this one?  We use the TYPE_SIZE precision
if we're dealing with bit-fields, but don't change the type in any
way.

Ubsan testsuite passes...

2013-09-16  Marek Polacek  <polacek@redhat.com>

	PR sanitizer/58413
	* ubsan.c (get_ubsan_type_info_for_type): For bit-fields, use the
	precision of its TYPE_SIZE.


	Marek

On Mon, Sep 16, 2013 at 06:18:11PM +0200, Marek Polacek wrote:
> On Mon, Sep 16, 2013 at 06:04:23PM +0200, Jakub Jelinek wrote:
> > On Mon, Sep 16, 2013 at 05:59:28PM +0200, Marek Polacek wrote:
> > > Regtested/ran bootstrap-ubsan on x86_64-linux.
> > 
> > That looks wrong.  ubsan_type_descriptor shouldn't change TYPE_PRECISION of
> > the type it has been called with, whether type is a bitfield or not can
> > change what typedescriptor is generated, but not unrelated code.
> 
> Ok, that was weird.  What about this one?  We use the TYPE_SIZE precision
> if we're dealing with bit-fields, but don't change the type in any
> way.
> 
> Ubsan testsuite passes...
> 
> 2013-09-16  Marek Polacek  <polacek@redhat.com>
> 
> 	PR sanitizer/58413
> 	* ubsan.c (get_ubsan_type_info_for_type): For bit-fields, use the
> 	precision of its TYPE_SIZE.

Is this one ok?  (With testcase from
http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01212.html .)

> --- 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);
> +
> +  prec = exact_log2 (prec);
>    if (prec == -1)
>      error ("unexpected size of type %qT", type);

	Marek

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.
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?
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.

	Jakub