Re: [RFC] Wrong "current" pointers with bitmap_ior and bitmap_ior_and_compl
Mike Stump wrote:
> While I had testing on my target, there were plenty of other regressions
> from trunk intermixed, so I wanted to do a normal bootstrap on Linux which
> I had not done yet. Kaz, you can check your patch, if the same, pretty
> sure it is, it has been approved and you can just check your version in,
> since you have the testing on it. Richard didn't want the extra testing
> my patch.
I've confirmed that the patches only differ by the extra checking
hunk and committed the patch below with your ChangeLog entry.
Regards,
kaz
--
2014-11-28 Mike Stump
* bitmap.c (bitmap_ior): Zap current as it could be deleted.
(bitmap_ior_and_compl): Likewise.
diff --git a/bitmap.c b/bitmap.c
index 8f7f306..ace6aea 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -1595,6 +1595,8 @@ bitmap_ior (bitmap dst, const_bitmap a, const_bitmap b)
if (dst_elt)
{
changed = true;
+ /* Ensure that dst->current is valid. */
+ dst->current = dst->first;
bitmap_elt_clear_from (dst, dst_elt);
}
gcc_checking_assert (!dst->current == !dst->first);
@@ -1951,6 +1953,8 @@ bitmap_ior_and_compl (bitmap dst, const_bitmap a,
const_bitmap b, const_bitmap k
if (dst_elt)
{
changed = true;
+ /* Ensure that dst->current is valid. */
+ dst->current = dst->first;
bitmap_elt_clear_from (dst, dst_elt);
}
gcc_checking_assert (!dst->current == !dst->first);
Re: [RFC] Wrong "current" pointers with bitmap_ior and bitmap_ior_and_compl
On Nov 28, 2014, at 4:03 AM, Richard Biener wrote: > On Fri, Nov 28, 2014 at 11:37 AM, Kaz Kojima wrote: >> Hi, >> >> I've got odd ICEs with some bitmap operations when working on >> sh-lra branch. >> bitmap_ior and bitmap_ior_and_compl are the bitmap operations >> for DST = A | B and DST = A | (B & ~KILL) respectively. It >> looks they could make bitmaps with the wrong "current" pointer. >> One small example is >> >> DST: [(bit 0, bit 127), indx=0] <-> [(bit 151), indx=1] -> 0 >> A: [(bit 141), indx=1] -> 0 >> B: [(bit 142), indx=1] -> 0 >> >> where assuming that BITMAP_ELEMENT_ALL_BITS is 128. >> In this case, bitmap_ior makes DST into the list >> >> [(bit 141, bit 142), indx=1] <-> [(bit 151), indx=1] -> 0 >> >> and unlinks the second element with bitmap_elt_clear_from. >> If the "current" pointer of DST pointed the 2nd element, >> the current pointer points the element freed from the list: >> >> DST: [(bit 141, bit 142), indx=1] -> 0 >> DST->current: [(bit 151), indx=1] >> >> even after bitmap_elt_clear_from done. >> bitmap_elt_clear_from has the code to fix the current pointer >> for the correct list of which elements have strictly ascending >> indx values. It doesn't work for the above list because its >> elements have the same indx. >> It seems that bitmap_and, bitmap_and_compl and bitmap_xor have >> the line to avoid this issue already. The patch below adds >> the same line to bitmap_ior and bitmap_ior_and_compl. >> The patch is tested on i686-pc-linux-gnu with bootstrap and >> the top level "make -k check". > > Huh? Wasn't this fixed by Mike a few weeks ago? While I had testing on my target, there were plenty of other regressions from trunk intermixed, so I wanted to do a normal bootstrap on Linux which I had not done yet. Kaz, you can check your patch, if the same, pretty sure it is, it has been approved and you can just check your version in, since you have the testing on it. Richard didn't want the extra testing my patch. >>* bitmap.c (bitmap_ior): Reset the current pointer before calling >>bitmap_elt_clear_from. >>(bitmap_ior_and_compl): Likewise.
Re: [RFC] Wrong "current" pointers with bitmap_ior and bitmap_ior_and_compl
Richard Biener wrote: > Huh? Wasn't this fixed by Mike a few weeks ago? Ugh. I've missed that thread. Sorry for the noise. Regards, kaz
Re: [RFC] Wrong "current" pointers with bitmap_ior and bitmap_ior_and_compl
On Fri, Nov 28, 2014 at 11:37 AM, Kaz Kojima wrote:
> Hi,
>
> I've got odd ICEs with some bitmap operations when working on
> sh-lra branch.
> bitmap_ior and bitmap_ior_and_compl are the bitmap operations
> for DST = A | B and DST = A | (B & ~KILL) respectively. It
> looks they could make bitmaps with the wrong "current" pointer.
> One small example is
>
> DST: [(bit 0, bit 127), indx=0] <-> [(bit 151), indx=1] -> 0
> A: [(bit 141), indx=1] -> 0
> B: [(bit 142), indx=1] -> 0
>
> where assuming that BITMAP_ELEMENT_ALL_BITS is 128.
> In this case, bitmap_ior makes DST into the list
>
> [(bit 141, bit 142), indx=1] <-> [(bit 151), indx=1] -> 0
>
> and unlinks the second element with bitmap_elt_clear_from.
> If the "current" pointer of DST pointed the 2nd element,
> the current pointer points the element freed from the list:
>
> DST: [(bit 141, bit 142), indx=1] -> 0
> DST->current: [(bit 151), indx=1]
>
> even after bitmap_elt_clear_from done.
> bitmap_elt_clear_from has the code to fix the current pointer
> for the correct list of which elements have strictly ascending
> indx values. It doesn't work for the above list because its
> elements have the same indx.
> It seems that bitmap_and, bitmap_and_compl and bitmap_xor have
> the line to avoid this issue already. The patch below adds
> the same line to bitmap_ior and bitmap_ior_and_compl.
> The patch is tested on i686-pc-linux-gnu with bootstrap and
> the top level "make -k check".
Huh? Wasn't this fixed by Mike a few weeks ago?
Richard.
> Regards,
> kaz
> --
> * bitmap.c (bitmap_ior): Reset the current pointer before calling
> bitmap_elt_clear_from.
> (bitmap_ior_and_compl): Likewise.
>
> diff --git a/bitmap.c b/bitmap.c
> index 8f7f306..ace6aea 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -1595,6 +1595,8 @@ bitmap_ior (bitmap dst, const_bitmap a, const_bitmap b)
>if (dst_elt)
> {
>changed = true;
> + /* Ensure that dst->current is valid. */
> + dst->current = dst->first;
>bitmap_elt_clear_from (dst, dst_elt);
> }
>gcc_checking_assert (!dst->current == !dst->first);
> @@ -1951,6 +1953,8 @@ bitmap_ior_and_compl (bitmap dst, const_bitmap a,
> const_bitmap b, const_bitmap k
>if (dst_elt)
> {
>changed = true;
> + /* Ensure that dst->current is valid. */
> + dst->current = dst->first;
>bitmap_elt_clear_from (dst, dst_elt);
> }
>gcc_checking_assert (!dst->current == !dst->first);
[RFC] Wrong "current" pointers with bitmap_ior and bitmap_ior_and_compl
Hi,
I've got odd ICEs with some bitmap operations when working on
sh-lra branch.
bitmap_ior and bitmap_ior_and_compl are the bitmap operations
for DST = A | B and DST = A | (B & ~KILL) respectively. It
looks they could make bitmaps with the wrong "current" pointer.
One small example is
DST: [(bit 0, bit 127), indx=0] <-> [(bit 151), indx=1] -> 0
A: [(bit 141), indx=1] -> 0
B: [(bit 142), indx=1] -> 0
where assuming that BITMAP_ELEMENT_ALL_BITS is 128.
In this case, bitmap_ior makes DST into the list
[(bit 141, bit 142), indx=1] <-> [(bit 151), indx=1] -> 0
and unlinks the second element with bitmap_elt_clear_from.
If the "current" pointer of DST pointed the 2nd element,
the current pointer points the element freed from the list:
DST: [(bit 141, bit 142), indx=1] -> 0
DST->current: [(bit 151), indx=1]
even after bitmap_elt_clear_from done.
bitmap_elt_clear_from has the code to fix the current pointer
for the correct list of which elements have strictly ascending
indx values. It doesn't work for the above list because its
elements have the same indx.
It seems that bitmap_and, bitmap_and_compl and bitmap_xor have
the line to avoid this issue already. The patch below adds
the same line to bitmap_ior and bitmap_ior_and_compl.
The patch is tested on i686-pc-linux-gnu with bootstrap and
the top level "make -k check".
Regards,
kaz
--
* bitmap.c (bitmap_ior): Reset the current pointer before calling
bitmap_elt_clear_from.
(bitmap_ior_and_compl): Likewise.
diff --git a/bitmap.c b/bitmap.c
index 8f7f306..ace6aea 100644
--- a/bitmap.c
+++ b/bitmap.c
@@ -1595,6 +1595,8 @@ bitmap_ior (bitmap dst, const_bitmap a, const_bitmap b)
if (dst_elt)
{
changed = true;
+ /* Ensure that dst->current is valid. */
+ dst->current = dst->first;
bitmap_elt_clear_from (dst, dst_elt);
}
gcc_checking_assert (!dst->current == !dst->first);
@@ -1951,6 +1953,8 @@ bitmap_ior_and_compl (bitmap dst, const_bitmap a,
const_bitmap b, const_bitmap k
if (dst_elt)
{
changed = true;
+ /* Ensure that dst->current is valid. */
+ dst->current = dst->first;
bitmap_elt_clear_from (dst, dst_elt);
}
gcc_checking_assert (!dst->current == !dst->first);
