The default bug view has changed. See this FAQ.

Status

()

Core
GFX: Color Management
RESOLVED FIXED
8 years ago
2 years ago

People

(Reporter: Robert Sayre, Assigned: jrmuizel)

Tracking

1.9.2 Branch
x86
Windows Server 2003
Points:
---
Bug Flags:
blocking1.9.2 +

Firefox Tracking Flags

(status1.9.2 beta3-fixed, status1.9.1 .6-fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
will attach stack
(Reporter)

Comment 1

8 years ago
Created attachment 403368 [details]
purify stack
(Reporter)

Updated

8 years ago
Assignee: nobody → jmuizelaar
(Assignee)

Comment 2

8 years ago
Created attachment 403412 [details] [diff] [review]
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.
Attachment #403412 - Flags: review?(joe)
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.
Attachment #403412 - Flags: review?(joe) → review+
(Assignee)

Comment 4

8 years ago
(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?
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).
(Assignee)

Updated

8 years ago
Attachment #403412 - Flags: approval1.9.1.5?
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.
Attachment #403412 - Flags: approval1.9.1.5? → approval1.9.1.5+
Keywords: testcase-wanted
(Assignee)

Updated

8 years ago
Attachment #403412 - Flags: approval1.9.2?
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8415bc81a9d3
status1.9.1: --- → .6-fixed
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.
Flags: blocking1.9.2?
Agreed; you can clear the approval request and land.
Flags: blocking1.9.2? → blocking1.9.2+
(Assignee)

Comment 10

8 years ago
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/0417352b24d6
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Does this need to land on mozilla-central?
Status: RESOLVED → REOPENED
status1.9.2: --- → final-fixed
Resolution: FIXED → ---
(Assignee)

Comment 12

8 years ago
No. Mozilla-central doesn't have this bug because the relevant code has been rewritten.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
Version: unspecified → 1.9.2 Branch
Attachment #403412 - Flags: approval1.9.2?
Keywords: testcase-wanted
You need to log in before you can comment on or make changes to this bug.