"ASSERTION: parser should have rejected negative length"

RESOLVED FIXED in mozilla7

Status

()

Core
CSS Parsing and Computation
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: arno renevier)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla7
x86
Mac OS X
assertion, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 540012 [details]
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
(Assignee)

Comment 1

6 years ago
Created attachment 540231 [details] [diff] [review]
patch v1

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).
(Assignee)

Updated

6 years ago
Attachment #540231 - Flags: review?(bzbarsky)
Comment on attachment 540231 [details] [diff] [review]
patch v1

Should FindNextLargerFontSize be using the saturating computation functions?
(Assignee)

Comment 3

6 years ago
Created attachment 543231 [details] [diff] [review]
patch v2
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?
(Assignee)

Comment 5

6 years ago
Created attachment 543266 [details] [diff] [review]
patch v2

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?
(Assignee)

Comment 7

6 years ago
Created attachment 543360 [details] [diff] [review]
patch v2.1
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+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/7281d9de99eb
Assignee: nobody → arno
Status: NEW → RESOLVED
Last Resolved: 6 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.