[wide-int] out-of-range set_bit in java

[Richard Biener]

On Fri, May 2, 2014 at 5:20 PM, Richard Sandiford
<rsandifo@linux.vnet.ibm.com> wrote:
> I locally tried adding an assertion to the wide-int version of set_bit
> to make sure that the bit number was in range.  It triggers for this
> code in boehm.c:mark_reference_fields (quoting trunk version):
>
>           /* First word in object corresponds to most significant byte of
>              bitmap.
>
>              In the case of a multiple-word record, we set pointer
>              bits for all words in the record. This is conservative, but the
>              size_words != 1 case is impossible in regular java code. */
>           for (i = 0; i < size_words; ++i)
>             *mask = (*mask).set_bit (ubit - count - i - 1);
>
>           if (count >= ubit - 2)
>             *pointer_after_end = 1;
>
> if count + i + 1 >= ubit.
>
> AIUI the lower 2 bits are used for something else:
>
>       /* Bottom two bits for bitmap mark type are 01.  */
>       mask = mask.set_bit (0);
>       value = double_int_to_tree (value_type, mask);
>
> which is why the pointer_after_end condition checks for count >= ubit - 2.
> We never actually use the mask if pointer_after_end is true, so this
> patch puts the set_bit in an else branch.
>
> On face value it looks like the condition should be:
>
>   count + size_words > ubit - 2
>
> instead, but it'd go without saying that I don't really understand this code.
>
> Tested on x86_64-linux-gnu and powerpc64-linux-gnu for wide-int.
> OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/java/
>         * boehm.c (mark_reference_fields): Don't update the mask when
>         setting pointer_after_end.
>
> Index: gcc/java/boehm.c
> ===================================================================
> --- gcc/java/boehm.c    2014-01-13 15:05:22.543887284 +0000
> +++ gcc/java/boehm.c    2014-05-02 16:08:25.500760537 +0100
> @@ -101,17 +101,17 @@ mark_reference_fields (tree field,
>
>           *last_set_index = count;
>
> -         /* First word in object corresponds to most significant byte of
> -            bitmap.
> -
> -            In the case of a multiple-word record, we set pointer
> -            bits for all words in the record. This is conservative, but the
> -            size_words != 1 case is impossible in regular java code. */
> -         for (i = 0; i < size_words; ++i)
> -           *mask = wi::set_bit (*mask, ubit - count - i - 1);
> -
>           if (count >= ubit - 2)
>             *pointer_after_end = 1;
> +         else
> +           /* First word in object corresponds to most significant byte of
> +              bitmap.
> +
> +              In the case of a multiple-word record, we set pointer
> +              bits for all words in the record. This is conservative, but the
> +              size_words != 1 case is impossible in regular java code. */
> +           for (i = 0; i < size_words; ++i)
> +             *mask = wi::set_bit (*mask, ubit - count - i - 1);
>
>           /* If we saw a non-reference field earlier, then we can't
>              use the count representation.  We keep track of that in
>