669136 - crash [@ nsParseMailMessageState::FinalizeHeaders()], [libSystem.B.dylib@0x4123b | ParseString | nsParseMailMessageState::FinalizeHeaders ] (5.0, Mac )

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Ludovic Hirlimann [:Usul]]

This bug was filed from the Socorro interface and is 
report bp-39b78a25-3802-4d25-9d11-5bb7e2110629 .
============================================================= 
0 	libSystem.B.dylib 	libSystem.B.dylib@0x4123b 	
1 	XUL 	ParseString 	
2 	XUL 	nsParseMailMessageState::FinalizeHeaders 	mailnews/local/src/nsParseMailbox.cpp:1623
3 	XUL 	nsParseMailMessageState::ParseFolderLine 	mailnews/local/src/nsParseMailbox.cpp:683
4 	XUL 	nsMsgLineBuffer::BufferInput 	mailnews/base/util/nsMsgLineBuffer.cpp:265
5 	XUL 	nsMsgMailboxParser::ProcessMailboxInputStream 	mailnews/local/src/nsParseMailbox.cpp:365
6 	XUL 	nsMsgMailboxParser::OnDataAvailable 	mailnews/local/src/nsParseMailbox.cpp:115
7 	XUL 	nsMailboxProtocol::ReadFolderResponse 	mailnews/local/src/nsMailboxProtocol.cpp:554
8 	XUL 	nsMailboxProtocol::ProcessProtocolState 	mailnews/local/src/nsMailboxProtocol.cpp:689
9 	XUL 	nsMsgProtocol::OnDataAvailable 	mailnews/base/util/nsMsgProtocol.cpp:387
10 	XUL 	nsInputStreamPump::OnStateTransfer 	netwerk/base/src/nsInputStreamPump.cpp:510
11 	XUL 	nsInputStreamPump::OnInputStreamReady 	netwerk/base/src/nsInputStreamPump.cpp:400
12 	XUL 	nsInputStreamReadyEvent::Run 	xpcom/io/nsStreamUtils.cpp:114
13 	XUL 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:618
14 	XUL 	NS_ProcessPendingEvents_P 	nsThreadUtils.cpp:200
15 	XUL 	nsBaseAppShell::NativeEventCallback 	widget/src/xpwidgets/nsBaseAppShell.cpp:130
16 	XUL 	nsAppShell::ProcessGeckoEvents 	widget/src/cocoa/nsAppShell.mm:399
17 	CoreFoundation 	CoreFoundation@0x4e400 	
18 	CoreFoundation 	CoreFoundation@0x4c5f8

Comment 1

[Wayne Mery (:wsmwk)]

I expect this is a duplicate, or at least has a windows counterpart, but I didn't investigate fully.

Bug 517456 - crash [@strchr | nsParseMailMessageState::ParseHeaders() ], formerly [@ nsParseMailMessageState::ParseHeaders()] 

(but more likely) there is a windows crash with signature nsParseMailMessageState::FinalizeHeaders in this list https://crash-stats.mozilla.com/query/query?product=Thunderbird&version=ALL%3AALL&range_value=4&range_unit=weeks&date=07%2F04%2F2011+08%3A30%3A36&query_search=signature&query_type=contains&query=nsParseMailMessageState&reason=&build_id=&hang_type=any&do_query=1

Comment 2

[Wayne Mery (:wsmwk)]

(not a new crash for v5)

windows crashes are [@ nsParseMailMessageState::FinalizeHeaders()] and
[@ strlen | nsParseMailMessageState::FinalizeHeaders() ] 
bp-382c1ce6-b9a9-4aa5-af3c-3d4172110716
EXCEPTION_ACCESS_VIOLATION_READ
0x11801033
0	mozcrt19.dll	strlen	strlen.asm:69
1	xul.dll	nsParseMailMessageState::FinalizeHeaders()	mailnews/local/src/nsParseMailbox.cpp:1329
2	xul.dll	nsParseMailMessageState::ParseFolderLine(char const*,unsigned int)	mailnews/local/src/nsParseMailbox.cpp:683
3	xul.dll	nsParseMailMessageState::ParseAFolderLine(char const*,unsigned int)	mailnews/local/src/nsParseMailbox.cpp:665
4	xul.dll	nsMsgLocalMailFolder::CopyData(nsIInputStream*,int)	mailnews/local/src/nsLocalMailFolder.cpp:2493
5	xul.dll	nsCopyMessageStreamListener::OnDataAvailable(nsIRequest*,nsISupports*,nsIInputStream*,unsigned int,unsigned int)	mailnews/base/src/nsCopyMessageStreamListener.cpp:126
6	xul.dll	nsStreamListenerTee::OnDataAvailable(nsIRequest*,nsISupports*,nsIInputStream*,unsigned int,unsigned int)	netwerk/base/src/nsStreamListenerTee.cpp:111 

and also libSystem.B.dylib@0x4175b for mac

Comment 3

[Wayne Mery (:wsmwk)]

several people have said this crash doesn't happen on the same message when using version 3.

Comment 4

[Cesar Eduardo Barros]

My crash on Linux (reported at https://crash-stats.mozilla.com/report/index/4e2dee91-b679-4eaa-8ada-dd2eb2110915) has a similar stack trace.

Comment 5

[Cesar Eduardo Barros]

I can reproduce my Linux crash at will, I will try to attach gdb to thunderbird and see if I can find out anything.

Comment 6

[Cesar Eduardo Barros]

OK, I have a small reproducer. I attached with gdb, made it crash, looked around to find some text from the broken message in memory, looked for the message within the large crashing spam folder, and extracted it.

The message looks a bit odd to me, but this mbox has only ever been changed by Thunderbird itself (mostly by moving large amounts of spam messages into it), so whatever is wrong with it, it is Thunderbird's fault. And even then, it should not crash.

The crash happens on a memchr() with an insane size:

#1  0x00007f8840f69596 in memchr (__c=<optimized out>, __s=0x7f8827dff789, 
    __n=18446744073708146894) at /usr/include/string.h:90
90	  return __builtin_memchr (__s, __c, __n);

Which seems to be caused by a negative length somewhere else:

#5  0x00007f88410ba258 in nsParseMailMessageState::FinalizeHeaders (
    this=0x7f882334ac08) at nsParseMailbox.cpp:1623
1623	          ParseString(Substring(keywords->value, keywords->value + keywords->length), ' ', newKeywordArray);
(gdb) p *keywords
$1 = {value = 0x7f8827ca8893 "\n Dec 26 21:46:17 2008", length = -60}

The full gdb session post-crash is as follows. I will attach the reproducer after submitting this comment.

Program received signal SIGSEGV, Segmentation fault.
memchr () at ../sysdeps/x86_64/memchr.S:46
46	2:	movdqa	(%rdi,%rsi), %xmm0
(gdb) where
#0  memchr () at ../sysdeps/x86_64/memchr.S:46
#1  0x00007f8840f69596 in memchr (__c=<optimized out>, __s=0x7f8827dff789, 
    __n=18446744073708146894) at /usr/include/string.h:90
#2  find (c=<optimized out>, n=18446744073708146894, 
    s=0x7f8827dff789 "\344\211.\210\177")
    at ../../../dist/include/nsCharTraits.h:636
#3  FindCharInReadable (aChar=<optimized out>, aSearchStart=..., 
    aSearchEnd=<optimized out>)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/string/src/nsReadableUtils.cpp:970
#4  0x00007f8840f69a3c in ParseString (aSource=<optimized out>, 
    aDelimiter=32 ' ', aArray=...)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/string/src/nsReadableUtils.cpp:768
#5  0x00007f88410ba258 in nsParseMailMessageState::FinalizeHeaders (
    this=0x7f882334ac08) at nsParseMailbox.cpp:1623
#6  0x00007f88410ba65c in nsParseMailMessageState::ParseFolderLine (
    this=0x7f882334ac08, line=<optimized out>, lineLength=1)
    at nsParseMailbox.cpp:683
#7  0x00007f88411d2346 in nsMsgLineBuffer::BufferInput (this=0x7f882334ae30, 
    net_buffer=0x7f8827b5addc "\n\nHi,\ni am here sitting in the internet caffe. Found your ad on that dating site www.mypo2you.com and\ndecided to write. I am 25 y.o.girl.\nI have a picture if you want. No need to reply here as \nthis i"..---Type <return> to continue, or q <return> to quit---
., net_buffer_size=270) at nsMsgLineBuffer.cpp:202
#8  0x00007f88410b57ee in nsMsgMailboxParser::ProcessMailboxInputStream (
    this=0x7f882334ac00, aURL=<optimized out>, aIStream=0x7f8827b8e610, 
    aLength=1770) at nsParseMailbox.cpp:365
#9  0x00007f88410b5691 in nsMsgMailboxParser::OnDataAvailable (
    this=0x7f882334ac00, request=<optimized out>, ctxt=<optimized out>, 
    aIStream=0x7f8827b8e610, sourceOffset=<optimized out>, 
    aLength=<optimized out>) at nsParseMailbox.cpp:115
#10 0x00007f88411fa7d1 in nsMailboxProtocol::ReadFolderResponse (
    this=0x7f882e38e120, inputStream=0x7f8827b8e610, 
    sourceOffset=<optimized out>, length=<optimized out>)
    at nsMailboxProtocol.cpp:554
#11 0x00007f88411fb33f in nsMailboxProtocol::ProcessProtocolState (
    this=0x7f882e38e120, url=<optimized out>, inputStream=0x7f8827b8e610, 
    offset=0, length=1770) at nsMailboxProtocol.cpp:689
#12 0x00007f88411e834f in nsMsgProtocol::OnDataAvailable (this=0x7f882e38e120, 
    request=<optimized out>, ctxt=<optimized out>, inStr=0x7f8827b8e610, 
    sourceOffset=0, count=<optimized out>) at nsMsgProtocol.cpp:387
#13 0x00007f88405f0f6f in nsInputStreamPump::OnStateTransfer (
    this=0x7f8826c5bac0)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/netwerk/base/src/nsInputStreamPump.cpp:510
#14 0x00007f88405f10d9 in nsInputStreamPump::OnInputStreamReady (
---Type <return> to continue, or q <return> to quit---
    this=0x7f8826c5bac0, stream=<optimized out>)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/netwerk/base/src/nsInputStreamPump.cpp:400
#15 0x00007f8840f477e6 in nsInputStreamReadyEvent::Run (this=0x7f882d3e85b0)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/io/nsStreamUtils.cpp:114
#16 0x00007f8840f598c6 in nsThread::ProcessNextEvent (this=0x7f883f90ff50, 
    mayWait=0, result=0x7ffff0fbb7ec)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/threads/nsThread.cpp:618
#17 0x00007f8840f2ac4c in NS_ProcessNextEvent_P (thread=<optimized out>, 
    mayWait=<optimized out>)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/build/nsThreadUtils.cpp:245
#18 0x00007f8840ec08d0 in mozilla::ipc::MessagePump::Run (this=0x7f8837e10a40, 
    aDelegate=0x7f883f990ff0)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/ipc/glue/MessagePump.cpp:110
#19 0x00007f8840f821ff in RunHandler (this=0x7f883f990ff0)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/ipc/chromium/src/base/message_loop.cc:202
#20 MessageLoop::Run (this=0x7f883f990ff0)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/ipc/chromium/src/ba---Type <return> to continue, or q <return> to quit---
se/message_loop.cc:176
#21 0x00007f8840e3d458 in nsBaseAppShell::Run (this=0x7f883f9ff890)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:189
#22 0x00007f8840cfbd0e in nsAppStartup::Run (this=0x7f8834d55700)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/toolkit/components/startup/nsAppStartup.cpp:222
#23 0x00007f88405d41a3 in XRE_main (argc=<optimized out>, 
    argv=<optimized out>, aAppData=<optimized out>)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/toolkit/xre/nsAppRunner.cpp:3686
#24 0x00000000004017c3 in main (argc=1, argv=0x7ffff0fbc228)
    at nsMailApp.cpp:104
(gdb) up
#1  0x00007f8840f69596 in memchr (__c=<optimized out>, __s=0x7f8827dff789, 
    __n=18446744073708146894) at /usr/include/string.h:90
90	  return __builtin_memchr (__s, __c, __n);
(gdb) up
#2  find (c=<optimized out>, n=18446744073708146894, 
    s=0x7f8827dff789 "\344\211.\210\177")
    at ../../../dist/include/nsCharTraits.h:636
636	        return reinterpret_cast<const char_type*>(memchr(s, to_int_type(c), n));
(gdb) up
#3  FindCharInReadable (aChar=<optimized out>, aSearchStart=..., 
    aSearchEnd=<optimized out>)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/string/src/nsReadableUtils.cpp:970
970	    const char* charFoundAt = nsCharTraits<char>::find(aSearchStart.get(), fragmentLength, aChar);
(gdb) up
#4  0x00007f8840f69a3c in ParseString (aSource=<optimized out>, 
    aDelimiter=32 ' ', aArray=...)
    at /usr/src/debug/thunderbird-6.0.2/comm-release/mozilla/xpcom/string/src/nsReadableUtils.cpp:768
768	        FindCharInReadable(aDelimiter, delimiter, end);
(gdb) up
#5  0x00007f88410ba258 in nsParseMailMessageState::FinalizeHeaders (
    this=0x7f882334ac08) at nsParseMailbox.cpp:1623
1623	          ParseString(Substring(keywords->value, keywords->value + keywords->length), ' ', newKeywordArray);
(gdb) p *keywords
$1 = {value = 0x7f8827ca8893 "\n Dec 26 21:46:17 2008", length = -60}
(gdb)

Comment 7

[Cesar Eduardo Barros]

Attachment: Small testcase extracted from crashing folder

This small mbox is enough to reproduce the crash for me.

Comment 8

[Cesar Eduardo Barros]

Comment on attachment 560505 [details]
Small testcase extracted from crashing folder

Bugzilla did not like message/rfc822, changing to text/plain.

Comment 9

[Cesar Eduardo Barros]

Finally, I let it crash in the gdb session above (using the "cont" command) and let it submit the crash. It is at https://crash-stats.mozilla.com/report/index/e60de6cc-3511-4b20-a0bb-bd9a62110915, but it seems that the crash reporter got confused by gdb and could not extract anything useful.

Comment 10

[David :Bienvenu]

The test case doesn't cause a crash for me with trunk builds - are you sure that's the message that causes the crash?

Comment 11

[Cesar Eduardo Barros]

(In reply to David :Bienvenu from comment #10)
> The test case doesn't cause a crash for me with trunk builds - are you sure
> that's the message that causes the crash?

I am on Fedora 15's 6.0.2, not trunk - it might already be fixed on trunk. That is a mbox which causes the crash for me (I compared the SHA1 after uploading to make sure it was exactly the same), there might be other broken messages on the big mbox this came from. To reproduce, I do the following:

1. Copy that file with some name to within one of the .sbd directories within Local Folders (I am using ".../Mail/Local Folders/Old.sbd/Broken.sbd/crashme" as the file name).
2. Open thunderbird and expand the parent folders.
3. Hover over the folder for a couple of seconds (it shows a small square empty tooltip), and click on it (hovering might not make any difference, but it is hard to avoid hovering with the trackpad).
4. Wait around half a dozen seconds, it will crash by itself (the window closes and it opens the crash reporter dialog box).

To reproduce again, I just remove the corresponding .msf (this might not be needed, I always removed it before reproducing).

Comment 12

[Cesar Eduardo Barros]

The testcase does not crash for me anymore with Fedora 15's 7.0.1 (thunderbird-7.0.1-1.fc15.x86_64), either with the reduced testcase or with the full folder where I originally saw the crash. Perhaps other people seeing this bug could try 7.0.1 and see if it fixes the crash for them too.

Comment 13

[Wayne Mery (:wsmwk)]

Cesar, thanks for testing that. 

There are still examples for 7.0.1 on crash-stats https://crash-stats.mozilla.com/report/list?product=Thunderbird&query_search=signature&query_type=exact&query=nsParseMailMessageState%3A%3AFinalizeHeaders%28%29&reason_type=contains&date=11%2F10%2F2011%2009%3A33%3A55&range_value=16&range_unit=weeks&hang_type=any&process_type=any&do_query=1&admin=1&signature=nsParseMailMessageState%3A%3AFinalizeHeaders%28%29

so I think we need to keep this open, unless usul or someone examines the signatures and determines we have multiple crash stacks that deserve separate bugs.

Comment 14

[Wayne Mery (:wsmwk)]

some bad spots in nsParseMailbox.cpp:1346 :(

the crash of comment 0, in current releases, might now be
mozalloc_abort(char const* const) | NS_DebugBreak_P | nsACString_internal::Assign(nsACString_internal const&)
bp-0f832d3c-e6aa-4734-9b42-646252120813 TB15
line #1607 of 
1602 // When there are many keywords, some may not have been written
1603 // to the message file, so add extra keywords from the backup
1604 nsCAutoString oldKeywords;
1605 m_newMsgHdr->GetStringProperty("keywords", getter_Copies(oldKeywords));
1606 nsTArray<nsCString> newKeywordArray, oldKeywordArray;
1607 ParseString(Substring(keywords->value, keywords->value + keywords->length), ' ', newKeywordArray);
1608 ParseString(oldKeywords, ' ', oldKeywordArray); 


the crashes in v7 era cited in comment 13 are a different issue
 1583 m_newMsgHdr->SetDate(resultTime);
 1584 PRTime2Seconds(resultTime, &rcvTimeSecs); 
... which may or may not be gone - don't know.

and not looking the same is
strlen | nsParseMailMessageState::FinalizeHeaders() 
for example bp-c5fceb39-3e48-45e3-ac3c-d5a862120813 TB14, source line is 1346
1344 if (mozstatus)
1345 {
1346 if (strlen(mozstatus->value) == 4) 

further ... nsParseMailMessageState::FinalizeHeaders() for example bp-a5f84fee-36cd-4266-be2c-ef0d02120714 TB13 is source line 1344
nsParseMailbox.cpp
1344 if (mozstatus)

Comment 15

[Hiroyuki Ikezoe (:hiro)]

Attachment: Fix

Fix for the case of:
(In reply to Wayne Mery (:wsmwk) from comment #14)
> and not looking the same is
> strlen | nsParseMailMessageState::FinalizeHeaders() 
> for example bp-c5fceb39-3e48-45e3-ac3c-d5a862120813 TB14, source line is 1346
> 1344 if (mozstatus)
> 1345 {
> 1346 if (strlen(mozstatus->value) == 4) 

Yes, this is not the same of the original one. Should I open a new bug?

Comment 16

[Wayne Mery (:wsmwk)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #15)
> Created attachment 653256 [details] [diff] [review]
> Fix
> 
> Fix for the case of:
> (In reply to Wayne Mery (:wsmwk) from comment #14)
> > and not looking the same is
> > strlen | nsParseMailMessageState::FinalizeHeaders() 
> > for example bp-c5fceb39-3e48-45e3-ac3c-d5a862120813 TB14, source line is 1346
> > 1344 if (mozstatus)
> > 1345 {
> > 1346 if (strlen(mozstatus->value) == 4) 
> 
> Yes, this is not the same of the original one. Should I open a new bug?
 
filed bug 791149

Comment 17

[rsx11m]

Comment on attachment 653256 [details] [diff] [review]
Fix

According to the history log, this patch received a review+ on 2012-08-21 but there is no indication that it was ever landed. What happened?

Comment 18

[Ludovic Hirlimann [:Usul]]

(In reply to rsx11m from comment #17)
> Comment on attachment 653256 [details] [diff] [review]
> Fix
> 
> According to the history log, this patch received a review+ on 2012-08-21
> but there is no indication that it was ever landed. What happened?

someone forgot to add checking needed.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/2606440ab850

Comment 20

[Wayne Mery (:wsmwk)]

Comment on attachment 653256 [details] [diff] [review]
Fix

[Approval Request Comment]
Regression caused by (bug #): 
User impact if declined: 
Testing completed (on c-c, etc.): 
Risk to taking this patch (and alternatives if risky):

[Approval Request Comment]
Regression caused by (bug #): -
User impact if declined: #29 topcrash continues for release 17 branch ~250 crashes per week. Crash doesn't appear for ESR17 for some - perhaps not enough users yet?
Testing completed (on c-c, etc.): TB20a1, TB19.0a2
Risk to taking this patch (and alternatives if risky): no regressions have been noted in the bug, which landed ~2 months ago.

Comment 21

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr17/rev/9d685c35a839

Comment 22

[Wayne Mery (:wsmwk)]

two users of beta 19 are still crashing
bp-c60ba7bf-8350-4452-bf86-066802130130
bp-68e15a50-f215-4906-a2fc-8e4502130205
... will see about getting testcases.

I haven't determined whether crash rate in alphas and betas has been reduced since landing. but

Comment 23

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk) from comment #22)
> two users of beta 19 are still crashing
> bp-c60ba7bf-8350-4452-bf86-066802130130
> bp-68e15a50-f215-4906-a2fc-8e4502130205
> ... will see about getting testcases.
> 
> I haven't determined whether crash rate in alphas and betas has been reduced
> since landing. but

I don't remember how I meant to finish that sentence, but (new information) ...

- in bug 877831 comment 1 hiro writes "the patch committed in bug #669136 (this bug) is not exactly fix for bug #669136. The patch is for bug #791149. **
- I'm unable to verify with crash-stats that the patch helped, because the crash signature strlen | nsParseMailMessageState::FinalizeHeaders()  dies out after version 14, bp-3c6ace25-1d70-41d8-a620-1d9a52120818 being an example of version 14

Which leaves the question, where do we go from here? Do we close bug 791149 as fixed, reopen this bug, and dupe bug 877831 to this bug?  Or leave this bug closed, continue the work in bug 877831, and close bug 791149?  

Still leaves an open question of why did strlen | nsParseMailMessageState::FinalizeHeaders() stop happening in release before this patch landed.  (And how to sort out the confusing variety of remaining crash stacks)


** changed from if (strlen(mozstatus->value) == 4) to if (mozstatus->length == 4)

Comment 24

[Mark Banner (:standard8)]

(In reply to Wayne Mery (:wsmwk) from comment #23)
> Which leaves the question, where do we go from here? Do we close bug 791149
> as fixed, reopen this bug, and dupe bug 877831 to this bug?  Or leave this
> bug closed, continue the work in bug 877831, and close bug 791149?  

I don't think it matters, these all seem to cover similar subjects.

> Still leaves an open question of why did strlen |
> nsParseMailMessageState::FinalizeHeaders() stop happening in release before
> this patch landed.  (And how to sort out the confusing variety of remaining
> crash stacks)

Probably would need to investigate lots of things/patches, that may or may not bear any fruit, so I'm not convinced we need to determine that.