Closed
Bug 448903
Opened 16 years ago
Closed 16 years ago
Crash [@ nsContextBoxBlur::BoxBlurVertical] with text-shadow
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jruderman, Assigned: ventnor.bugzilla)
References
Details
(Keywords: crash, testcase, Whiteboard: [sg:critical?])
Crash Data
Attachments
(2 files, 1 obsolete file)
229 bytes,
text/html
|
Details | |
530 bytes,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
Loading the testcase makes nsContextBoxBlur::BoxBlurVertical dereference a random address, which usually causes a crash.
Reporter | ||
Updated•16 years ago
|
Whiteboard: [sg:critical?]
Assignee | ||
Comment 1•16 years ago
|
||
No crash here. OS/Build? Is your build up-to-date? I remember fixing a crash in BoxBlurVertical before that Martijn found.
Assignee | ||
Comment 2•16 years ago
|
||
Never mind; its random but I could eventually reproduce this. I get ridiculous values for y for the x[y] operator, but what seemed suspicious was that I was getting frames with height 1. This might explain why you're crashing in vertical blur which is after a perfectly fine horizontal blur. You can't average 1 value and get anything different, so the easiest thing to do is return early if you only get one pixel in the given direction. Lets hope I'm right. Can you try again with this patch?
Assignee: nobody → ventnor.bugzilla
Status: NEW → ASSIGNED
Attachment #332101 -
Flags: superreview?(roc)
Attachment #332101 -
Flags: review?(roc)
Reporter | ||
Comment 3•16 years ago
|
||
I think the problem is this line in BoxBlurVertical: pos = PR_MIN(pos, stride - 1); That constraint makes sense inBoxBlurHorizontal, but in BoxBlurVertical I think pos needs to be restricted by the number of rows instead.
Assignee | ||
Comment 4•16 years ago
|
||
(In reply to comment #3) > I think the problem is this line in BoxBlurVertical: > > pos = PR_MIN(pos, stride - 1); > > That constraint makes sense inBoxBlurHorizontal, but in BoxBlurVertical I think > pos needs to be restricted by the number of rows instead. > No, rows is not used in that block but stride is.
Reporter | ||
Comment 5•16 years ago
|
||
The reason I'm suspicious of "pos = PR_MIN(pos, stride - 1);" is that pos is about to be *multiplied* by stride.
Assignee | ||
Comment 6•16 years ago
|
||
Now that I look at it, you're probably right. I wonder why it worked before then...
Assignee | ||
Comment 7•16 years ago
|
||
Yeah, that is the cause because I re-read the old blurring code in SVG that I copied this from originally.
Attachment #332101 -
Attachment is obsolete: true
Attachment #332108 -
Flags: superreview?(roc)
Attachment #332108 -
Flags: review?(roc)
Attachment #332101 -
Flags: superreview?(roc)
Attachment #332101 -
Flags: review?(roc)
Reporter | ||
Comment 8•16 years ago
|
||
Patch 2 fixes the bug for me :)
Attachment #332108 -
Flags: superreview?(roc)
Attachment #332108 -
Flags: superreview+
Attachment #332108 -
Flags: review?(roc)
Attachment #332108 -
Flags: review+
Pushed 6524c183be7a. I added Jesse's test as a crashtest --- Michael, it would be slightly more convenient if you added that in your patch next time.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Keywords: checkin-needed
Reporter | ||
Comment 11•16 years ago
|
||
Is it possible to test this without gigantic widths and heights?
Updated•13 years ago
|
Crash Signature: [@ nsContextBoxBlur::BoxBlurVertical]
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•9 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•