849688 - Crash with getStartPositionOfChar, svg.text.css-frames.enabled

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase

With:
  user_pref("svg.text.css-frames.enabled", true);

Crash [@ gfxSkipCharsIterator::SetOffsets]

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Scoobidiver (away)]

On Windows: bp-a59c21b6-9b55-4b17-8d91-7c8652130311.

Comment 3

[Robert Longson [:longsonr]]

Attachment: patch

Bug 843072 was almost right but not quite. We need to check we're at the end before calling Next(chars)

Comment 4

[Robert Longson [:longsonr]]

The kid stuff is unused code and never gets hit.

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 724061 [details] [diff] [review]
patch

r=me, but this also means that Next() is buggy (or at least doesn't hold up to its documentation)

It's currently documented as follows:
> 2037   /**
> 2038    * Advances ahead aCount matching characters.  Returns true if there were
> 2039    * enough characters to advance past, and false otherwise.
> 2040    */
> 2041   bool Next(uint32_t aCount);
which implies that it should just return false (not crash) if we're at the end & try to advance.

Could you file a followup on fixing that?  Not sure if it'll still be possible to trigger that behavior after this bug's fixed, but it's a footgun waiting to be loaded when someone adds a Next() call elsewhere and trusts its documentation to be accurate. :)

Comment 6

[Robert Longson [:longsonr]]

https://tbpl.mozilla.org/?tree=Try&rev=1250e503d4ea

Comment 7

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bbde2235d308

Comment 8

[Robert Longson [:longsonr]]

Created bug 655877 per cooment 5

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to Robert Longson from comment #8)
> Created bug 655877 per cooment 5

looks like a mis-paste?

Comment 10

[Daniel Holbert [:dholbert]]

yeah, comment 7 meant to say "bug 850655"

Comment 11

[Robert Longson [:longsonr]]

can't type comment either :-(

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/bbde2235d308