[Pixman] [PATCH] pixman-combine: Fix wrong value of RB_MASK_PLUS_ONE.

[Søren Sandmann]

Hi,

The reason this bug doesn't show up in the test suite is that it
doesn't have an effect on the output. The purpose of the
UN8_rb_ADD_UN8_rb macro is to add two numbers like these:

    00aa00bb
    00cc00dd

together, but to saturate aa+cc and bb+dd to 0xff instead of allowing
them to overflow. Let's pretend that bb and dd are both zero and focus
on aa+cc. If that addition does in fact overflow (ie., if aa+cc >
0xff), the result looks like this:

    01??0000

The macro first detects the overflow shifting this value by 8 and
masking to 0x00ff00ff:

    00010000

Then it subtracts this from the RB_MASK_PLUS_ONE value:

    with bug:       10000000 - 00010000 = 0fff0000
    with bug fix:   01000000 - 00010000 = 00ff0000

The macro then ors this value with the original sum:

    with bug:        0fff0000 | 01??0000 = 0fff0000
    with bug fix:    00ff0000 | 01??0000 = 01ff0000

But then it masks the result with 00ff00ff, and so both with and
without the bug, the final result is the same:

    00ff0000

which is why the bug doesn't affect the calculation.


Søren


On Sun, Feb 9, 2020 at 4:26 AM Shiyou Yin <yinshiyou-hf at loongson.cn> wrote:
>
> >-----Original Message-----
> >From: Matt Turner [mailto:mattst88 at gmail.com]
> >Sent: Sunday, February 9, 2020 7:01 AM
> >To: Yin Shiyou
> >Cc: pixman at lists.freedesktop.org
> >Subject: Re: [Pixman] [PATCH] pixman-combine: Fix wrong value of RB_MASK_PLUS_ONE.
> >
> >On Mon, Feb 3, 2020 at 1:56 AM Yin Shiyou <yinshiyou-hf at loongson.cn> wrote:
> >>
> >> ---
> >>  pixman/pixman-combine32.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/pixman/pixman-combine32.h b/pixman/pixman-combine32.h
> >> index cdd56a6..59bb247 100644
> >> --- a/pixman/pixman-combine32.h
> >> +++ b/pixman/pixman-combine32.h
> >> @@ -12,7 +12,7 @@
> >>  #define RB_MASK 0xff00ff
> >>  #define AG_MASK 0xff00ff00
> >>  #define RB_ONE_HALF 0x800080
> >> -#define RB_MASK_PLUS_ONE 0x10000100
> >> +#define RB_MASK_PLUS_ONE 0x1000100
> >
> >
> >Thanks. The patch looks correct, but obviously nothing in the test
> >suite is failing. How did you discover this? Does this patch fix
> >something for you?
>
> I just found this mistake while analyzing macro 'UN8_rb_ADD_UN8_rb'
> and functions called this macro. I can't verify this error with existing
> test suite, it all passed no matter Fix it or not.
>
> _______________________________________________
> Pixman mailing list
> Pixman at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pixman