Closed Bug 664955 Opened 9 years ago Closed 9 years ago

"ASSERTION: parser should have rejected negative length"

Categories

(Core :: CSS Parsing and Computation, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: jruderman, Assigned: arno)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files, 3 obsolete files)

Attached file testcase
###!!! ASSERTION: FindNextLargerFontSize failed: '*aSize > parentSize', file layout/style/nsRuleNode.cpp, line 2538
(bug 427322)

###!!! ASSERTION: parser should have rejected negative length: 'widthValue.IsCalcUnit()', file layout/style/nsRuleNode.cpp, line 6283
Attached patch patch v1 (obsolete) — Splinter Review
This is because nsStyleUtil::FindNextLargerFontSize returns a negative value (because of PRInt32 overflow).
Maybe it can be fixed by checking that returned value is positive (and also, below or equal nscoord_MAX).
Attachment #540231 - Flags: review?(bzbarsky)
Comment on attachment 540231 [details] [diff] [review]
patch v1

Should FindNextLargerFontSize be using the saturating computation functions?
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #540231 - Attachment is obsolete: true
Attachment #540231 - Flags: review?(bzbarsky)
Attachment #543231 - Flags: review?(bzbarsky)
That looks identical to v1 to me.  What about my question from comment 2?
Attached patch patch v2 (obsolete) — Splinter Review
Sorry, wrong file attached. I should have checked more.
Attachment #543231 - Attachment is obsolete: true
Attachment #543231 - Flags: review?(bzbarsky)
Attachment #543266 - Flags: review?(bzbarsky)
>+      largerSize = indexFontSize + NSCoordSaturatingNonnegativeMultiply(largerIndexFontSize - indexFontSize, relativePosition);

Should that be a saturating add?
Attached patch patch v2.1Splinter Review
Attachment #543266 - Attachment is obsolete: true
Attachment #543266 - Flags: review?(bzbarsky)
Attachment #543360 - Flags: review?(bzbarsky)
Comment on attachment 543360 [details] [diff] [review]
patch v2.1

r=me.  Thank you!
Attachment #543360 - Flags: review?(bzbarsky) → review+
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/7281d9de99eb
Assignee: nobody → arno
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.