382480 - TB Crash [@ nsBidiPresUtils::ProcessText]

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Ere Maijala (slow)]

Attachment: Patch to make ProcessText use the given length

I just had my Thunderbird debug build crash unexpectedly and I decided to put it in the debugger for a quick check on what happened. To me it seems pretty obvious:

nsBidiPresUtils::ProcessText was called with aText of 132 valid characters (message subject) and rest junk. aLength was properly set to 132, but it wasn't used when assigning aText to mBuffer. mBuffer tried to calculate the length and caused an AV. A possible patch is attached.

Comment 1

[Ere Maijala (slow)]

Fix checked in to trunk.

Comment 3

[Mats Palmgren (inactive)]

Comment on attachment 266640 [details] [diff] [review]
Patch to make ProcessText use the given length

I think we should take this on branches as well.

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 266640 [details] [diff] [review]
Patch to make ProcessText use the given length

approved for 1.8.1.7, a=dveditz for release-drivers

Comment 5

[Scott MacGregor]

Ere, let me know if you'd like me to land this on the branch for you.

Comment 6

[Ere Maijala (slow)]

Please do. Thanks.

Comment 7

[Scott MacGregor]

fixed on the 1.8 branch. thanks Ere!

Comment 8

[Jay Patel [:jay]]

Ere:  Any chance you can test this with the 2.0.0.8rc2 build to verify that crash is fixed?  If you are unable to reproduce with the latest 1.8.1 build, please replace "fixed1.8.1.8" with "verified1.8.1.8".  Thanks!

Comment 9

[Jay Patel [:jay]]

FWIW:  I was not able to find any crashes in Talkback data with this stack signature.

Comment 10

[Ere Maijala (slow)]

Yep, I haven't been able to crash here anymore.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 266640 [details] [diff] [review]
Patch to make ProcessText use the given length

Ere: are you able to land this on the 1.8.0 branch in the next couple of days, or should I land it?

Comment 12

[Ere Maijala (slow)]

I'd appreciate it if you could land it. 

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Whats the difference of the checkin from comment 7? Wasn't it already landed for 1.8.1.8?

Comment 14

[Daniel Veditz [:dveditz]]

yes, this is now fixed as of 1.8.1.8, but it has not yet been fixed in 1.8.0.anything. We've stopped Firefox support on that branch but have one last Thunderbird 1.5.0.x release.

Comment 15

[Daniel Veditz [:dveditz]]

Patch checked into the 1.8.0.x branch.

Ere: did you have a testcase for this one?

Comment 16

[Al Billings [:abillings - ex-MoCo]]

This is hard to verify without some sort of testcase, says QA. :-)

Comment 17

[Ere Maijala (slow)]

Without the patch Thunderbird reliably but intermittently crashed while hovering over a long subject line in the message list and a tooltip would have been displayed. It always happened after a couple of tries, but as it depends on what comes after the string in memory, it might not be repeatable with the same reliability (and apparently isn't, as I'm sure people would have crashed a lot otherwise).

Comment 18

[Wayne Mery (:wsmwk)]

This FIXED bug is flagged with in‑testsuite?   It would be great if assignee or someone else can clear the flag if a test is not appropriate.  And if appropriate, create a test and plus the flag to finish off the bug.

Comment 19

[Mark Banner (:standard8)]

Given the age of this bug, and the fact that the issue was in core/graphics/layout. I'm not convinced its reasonably possible to whip up a test case for this bug. Therefore cancelling the in-testsuite request - if we really want a test for it, we should file a bug in layout which is where it really belongs (I won't move this bug due to the age though).