519338 - Purify UMR in QCMS

Status: RESOLVED FIXED

(Core :: Graphics: Color Management, defect)

Description

[Robert Sayre]

will attach stack

Comment 1

[Robert Sayre]

Attachment: purify stack

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Initialize the unused element of the input array

This UMR is mostly harmless because we never use the result. We just load the uninitialized value, take it along for the computation and discard it. This code has also been rewritten in bug 512865. Still, it doesn't hurt to initialize it so here's a patch.

Comment 3

[Joe Drew (not getting mail)]

Comment on attachment 403412 [details] [diff] [review]
Initialize the unused element of the input array

>diff --git a/gfx/qcms/transform.c b/gfx/qcms/transform.c
>--- a/gfx/qcms/transform.c
>+++ b/gfx/qcms/transform.c
>@@ -812,6 +812,8 @@ static void qcms_transform_data_rgb_out_
>         /* share input and output locations to save having to keep the
>          * locations in separate registers */
>         uint32_t* output = (uint32_t*)input;
>+
>+        input[3] = 0; /* initialize the unused 4th element of the input array */

Not the greatest place to put this, but C doesn't give us a lot of options.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #3)
> (From update of attachment 403412 [details] [diff] [review])
> >diff --git a/gfx/qcms/transform.c b/gfx/qcms/transform.c
> >         uint32_t* output = (uint32_t*)input;
> >+
> >+        input[3] = 0; /* initialize the unused 4th element of the input array */
> 
> Not the greatest place to put this, but C doesn't give us a lot of options.

Why not?

Comment 5

[Joe Drew (not getting mail)]

It should either go with the input stuff above output, or in the body of the loop (one is impossible, the other sub-optimal performance-wise).

Comment 6

[Daniel Veditz [:dveditz]]

Comment on attachment 403412 [details] [diff] [review]
Initialize the unused element of the input array

Approved for 1.9.1.5, a=dveditz for release-drivers

Is there a testcase that exercises this code? Or does it happen with any graphics?

In addition to Purify, a windows debug build with the -RTCu or -RTC1 switch should also catch UMRs like this -- if a testcase causes the code to be executed.

Comment 7

[Joe Drew (not getting mail)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8415bc81a9d3

Comment 8

[Samuel Sidler (old account; do not CC)]

This should block 1.9.2 since we took the patch on 1.9.1 and it'd otherwise be a regression when 1.9.2 ships.

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Agreed; you can clear the approval request and land.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0417352b24d6

Comment 11

[Samuel Sidler (old account; do not CC)]

Does this need to land on mozilla-central?

Comment 12

[Jeff Muizelaar [:jrmuizel]]

No. Mozilla-central doesn't have this bug because the relevant code has been rewritten.