crash [@ nsMsgFilter::CreateTerm(nsIMsgSearchTerm**)]
3 crashes in last few months. only v3.1.6. (looks like) only one person.
0 thunderbird-bin nsMsgFilter::CreateTerm mailnews/base/search/src/nsMsgFilter.cpp:290
1 thunderbird-bin VirtualFolderChangeListener::Init mailnews/base/src/nsMsgAccountManager.cpp:2622
2 thunderbird-bin nsMsgAccountManager::AddVFListenersForVF mailnews/base/src/nsMsgAccountManager.cpp:3267
3 thunderbird-bin nsMsgAccountManager::LoadVirtualFolders mailnews/base/src/nsMsgAccountManager.cpp:3088
4 libxpcom_core.dylib libxpcom_core.dylib@0x6e120
5 thunderbird-bin XPCWrappedNative::CallMethod js/src/xpconnect/src/xpcwrappednative.cpp:2722
6 thunderbird-bin XPC_WN_CallMethod js/src/xpconnect/src/xpcwrappednativejsops.cpp:1740
Crash Reason EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE
Crash Address 0x0
Wayne: can you try to include the crash reason+crash address when you file bugs like this? In theory it should help people recognize whether they're null pointer references (like this one) or unhappy pointer references (more work)
285 nsMsgFilter::CreateTerm(nsIMsgSearchTerm **aResult)
287 nsMsgSearchTerm *term = new nsMsgSearchTerm;
288 NS_ENSURE_TRUE(term, NS_ERROR_OUT_OF_MEMORY);
290 *aResult = static_cast<nsIMsgSearchTerm*>(term);
2620 filterList->CreateFilter(NS_LITERAL_STRING("temp"), getter_AddRefs(tempFilter));
2621 NS_ENSURE_SUCCESS(rv, rv);
2622 filterList->ParseCondition(tempFilter, searchTermString.get());
Unfortunately, this seems like a case of pointer confusion, I can't quickly figure out how line 2622 could reach nsMsgFilter::CreateTerm
one crash in 8 months and the stack looks pretty worthless, so incomplete
bp-3dbaf602-f651-4a9b-8dfe-5d95a2111221 version 8
but if someone gets bored ...
I guess we could at least add NS_ENSURE_ARG_POINTER(aResult); into nsMsgSearchSession::CreateTerm(nsIMsgSearchTerm **aResult) like it is now in nsMsgFilter::CreateTerm . (It wasn't there at the time of comment 1, it was added in bug 707306).
I have not yet looked into nsMsgSearchSession.cpp file for possible missing null-arg checks so this could be a good place to do it.
Created attachment 606860 [details] [diff] [review]
null-args and cleanup
Comment on attachment 606860 [details] [diff] [review]
null-args and cleanup
Review of attachment 606860 [details] [diff] [review]:
@@ +78,3 @@
> +/* [noscript] void AddSearchTerm(in nsMsgSearchAttribute attrib, in nsMsgSearchOperator op, in nsMsgSearchValue value, in boolean BooleanAND, in string customString); */
I think we can just drop these type of lines - they don't add any real value to the code.
Maybe the at least note which arguments are input and which output and readonly.
Should I remove them before each function?
Created attachment 611962 [details] [diff] [review]
Removes comment lines. r=standard8.
Any chance of getting a test for this?