Segfault searching for tags

Segfault searching for tags

[Jeffrey Ollie]

Getting the following segfault with 306635c2 on Fedora 12.  Seems to
be happening with any 'tag:' search that returns results.  For
example, 'notmuch search tag:inbox' and 'notmuch search tag:unread'
segfault but 'notmuch search tag:nosuchtag', 'notmuch search
subject:logwatch' and 'notmuch search video' seem to work fine.

Core was generated by `/usr/bin/notmuch search --sort=oldest-first tag:inbox'.
Program terminated with signal 11, Segmentation fault.
\#0  Xapian::TermIterator::operator* (this=<value optimized out>)
    at api/omtermlistiterator.cc:78
78	    RETURN(internal->get_termname());
Current language:  auto
The current source language is "auto; currently c++".

Thread 1 (Thread 15005):
\#0  Xapian::TermIterator::operator* (this=<value optimized out>)
    at api/omtermlistiterator.cc:78
No locals.
\#1  0x000000000040d213 in _notmuch_message_get_in_reply_to (message=0x1594f70)
    at lib/message.cc:288
        prefix = 0x415b77 "XREPLYTO"
        prefix_len = 0
        i = {internal = {dest = 0x0}}
        in_reply_to = ""
\#2  0x000000000040f842 in _resolve_thread_relationships (thread=0x1595a00)
    at lib/thread.cc:157
        node = 0x1596130
        message = 0x1594f70
        parent = 0x7fff2cade9c8
        prev = 0x1595cd0
        in_reply_to = <value optimized out>
\#3  _notmuch_thread_create (thread=0x1595a00) at lib/thread.cc:285
        thread = 0x1595a00
        thread_id_query = 0x158beb0
        matched_query = <value optimized out>
        messages = 0x7fff2cade948
        message = <value optimized out>
        thread_id_query_string = <value optimized out>
        matched_query_string = <value optimized out>
\#4  0x000000000040f3d0 in notmuch_query_search_threads (
    query=<value optimized out>, first=<value optimized out>,
    max_threads=<value optimized out>) at lib/query.cc:217
        threads = 0x158b5f0
        thread = 0x6e00000077
        messages = 0x158b7c0
        message = 0x158c580
        thread_id = 0x158b890 "2065b08615b4cbbb22d9ee874bb84d3e"
        seen = 0x15454a0
        messages_seen = 0
        threads_seen = 0
\#5  0x00000000004089a1 in do_search_threads (ctx=0x1543140, query=
    0x7fff2cade8a0, sort=NOTMUCH_SORT_OLDEST_FIRST,
    first=<value optimized out>, max_threads=<value optimized out>)
    at notmuch-search.c:40
        thread = <value optimized out>
        threads = 0x1551290
        tags = 0x2
        date = <value optimized out>
        relative_date = 0x2 <Address 0x2 out of bounds>
\#6  0x0000000000408ddd in notmuch_search_command (ctx=<value optimized out>,
    argc=1, argv=<value optimized out>) at notmuch-search.c:156
        config = <value optimized out>
        query = 0x158b510
        query_str = <value optimized out>
        i = 1
        first = <value optimized out>
        max_threads = <value optimized out>
        opt = <value optimized out>
        end = 0x0
        sort = <value optimized out>
\#7  0x000000000040636f in main (argc=4, argv=0x7fff2cadec98) at notmuch.c:400
        local = 0x1543140
        command = <value optimized out>


-- 
Jeff Ollie

Re: Segfault searching for tags

[Adrian Perez de Castro]

[-- Attachment #1: Type: text/plain, Size: 3328 bytes --]

On Wed, 18 Nov 2009 12:00:10 -0600, Jeffrey wrote:

> Getting the following segfault with 306635c2 on Fedora 12.  Seems to
> be happening with any 'tag:' search that returns results.  For
> example, 'notmuch search tag:inbox' and 'notmuch search tag:unread'
> segfault but 'notmuch search tag:nosuchtag', 'notmuch search
> subject:logwatch' and 'notmuch search video' seem to work fine.
> 
> Core was generated by `/usr/bin/notmuch search --sort=oldest-first tag:inbox'.
> Program terminated with signal 11, Segmentation fault.
> \#0  Xapian::TermIterator::operator* (this=<value optimized out>)
>     at api/omtermlistiterator.cc:78
> 78	    RETURN(internal->get_termname());
> Current language:  auto
> The current source language is "auto; currently c++".

I have hit what I believe is exactly the same problem. In my case, some
results are printed when I execute "notmuch search tag:inbox", and then
the program crashes in the same exact place.

The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
instance of Xapian::TermIterator is dereferenced. In my particular case,
the culpript is a cache file of Claws-Mail, as seen in the following GDB
session:

Program received signal SIGSEGV, Segmentation fault.
Xapian::TermIterator::operator* (this=<value optimized out>) at api/omtermlistiterator.cc:78
78   RETURN(internal->get_termname()); Current language:  auto
The current source language is "auto; currently c++".
(gdb) bt
#0  Xapian::TermIterator::operator* (this=<value optimized out>) at api/omtermlistiterator.cc:78
#1  0x000000000040f611 in _notmuch_message_get_in_reply_to(message=0x76dcd0) at lib/message.cc:288
#2  0x0000000000412030 in _resolve_thread_relationships (thread=0x6a8b80) at lib/thread.cc:157
#3  0x0000000000412454 in _notmuch_thread_create (ctx=0x65f1b0, notmuch=0x62d320, thread_id= 0x765530 "01b17ddb4479a0dc0b416bb63b92c43d", query_string=0x65f220 "tag:inbox") at lib/thread.cc:285
#4  0x0000000000411982 in notmuch_query_search_threads (query=0x65f1b0, first=100, max_threads=-1) at lib/query.cc:218
#5  0x000000000040924d in do_search_threads (ctx=0x61f140, query=0x65f1b0, sort=NOTMUCH_SORT_NEWEST_FIRST, first=100, max_threads=-1) at notmuch-search.c:40
#6  0x00000000004097ef in notmuch_search_command (ctx=0x61f140, argc=1, argv=0x7fffffffe188) at notmuch-search.c:164
#7  0x00000000004066f1 in main (argc=3, argv=0x7fffffffe178) at notmuch.c:400
(gdb) frame 1
#1  0x000000000040f611 in _notmuch_message_get_in_reply_to (message=0x76dcd0) at lib/message.cc:288
288	    in_reply_to = *i;
(gdb) p *message
$1 = {notmuch = 0x62d320, doc_id = 1, frozen = 0, message_id = 0x76db60 "", thread_id = 0x0, 
  in_reply_to = 0x0, filename = 0x76dc50 "/home/aperez/.mail/inbox/.claws_cache", message_file = 0x0, 
  replies = 0x76d250, doc = {internal = {dest = 0x76d450}}}

As you can see, there "filename" points to a Claws-Mail cache file, which
is a binary file (I can provide a copy if needed). I suspect that this is
related to the fact that the iterator ends up being NULL somehow.

I will experiment a bit more with this issue -- maybe just avoiding adding
files whose name starts with a dot will suffice as temporary fix.

Cheers,

-- 
Adrian Perez de Castro <aperez@igalia.com>
Igalia - Free Software Engineering

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Segfault searching for tags

[Jeffrey Ollie]

[-- Attachment #1: Type: text/plain, Size: 1974 bytes --]

On Thu, Nov 19, 2009 at 9:45 AM, Adrian Perez de Castro
<aperez@igalia.com> wrote:
> On Wed, 18 Nov 2009 12:00:10 -0600, Jeffrey wrote:
>
>> Getting the following segfault with 306635c2 on Fedora 12.  Seems to
>> be happening with any 'tag:' search that returns results.  For
>> example, 'notmuch search tag:inbox' and 'notmuch search tag:unread'
>> segfault but 'notmuch search tag:nosuchtag', 'notmuch search
>> subject:logwatch' and 'notmuch search video' seem to work fine.
>>
>> Core was generated by `/usr/bin/notmuch search --sort=oldest-first tag:inbox'.
>> Program terminated with signal 11, Segmentation fault.
>> \#0  Xapian::TermIterator::operator* (this=<value optimized out>)
>>     at api/omtermlistiterator.cc:78
>> 78        RETURN(internal->get_termname());
>> Current language:  auto
>> The current source language is "auto; currently c++".
>
> I have hit what I believe is exactly the same problem. In my case, some
> results are printed when I execute "notmuch search tag:inbox", and then
> the program crashes in the same exact place.
>
> The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
> instance of Xapian::TermIterator is dereferenced. In my particular case,
> the culpript is a cache file of Claws-Mail, as seen in the following GDB
> session:
> [...]
> As you can see, there "filename" points to a Claws-Mail cache file, which
> is a binary file (I can provide a copy if needed). I suspect that this is
> related to the fact that the iterator ends up being NULL somehow.

I straced some of the crashes, and the last file that was read before
the crash was a malformed message.  I've attached one of the messages.
 I've been using offlineimap to sync my gmail mailbox to my laptop so
that I can use notmuch.  offlineimap isn't the most stable program,
but I'm not sure yet if offlineimap is causing the problem or if
that's the way the message is in gmail.

-- 
Jeff Ollie

[-- Attachment #2: message.txt --]
[-- Type: text/plain, Size: 980 bytes --]

Delivered-To: jeff@ollie.clive.ia.us
Received: by 10.90.86.18 with SMTP id j18cs228556agb;
        Thu, 5 Nov 2009 22:23:50 -0800 (PST)
Received: by 10.90.16.38 with SMTP id 38mr7468290agp.112.1257488620374;
        Thu, 05 Nov 2009 22:23:40 -0800 (PST)
Return-Path: <xhutteesjj@yahoo.com.au>
Received: from 209.85.223.101 ([116.208.64.100])
        by mx.google.com with SMTP id 41si9046203iwn.112.2009.11.05.22.23.38;
        Thu, 05 Nov 2009 22:23:40 -0800 (PST)
Received-SPF: neutral (google.com: 116.208.64.100 is neither permitted nor denied by best guess record for domain of xhutteesjj@yahoo.com.au) client-ip=116.208.64.100;
Authentication-Results: mx.google.com; spf=neutral (google.com: 116.208.64.100 is neither permitted nor denied by best guess record for domain of xhutteesjj@yahoo.com.au) smtp.mail=xhutteesjj@yahoo.com.au
Date: Thu, 05 Nov 2009 22:23:40 -0800 (PST)
Received: from 146.2.76.118 by 116.208.64.100; Fri, 06 Nov 2009 03:20:42 -0300
Message-ID: <C[20

Re: Segfault searching for tags

[Carl Worth]

On Thu, 19 Nov 2009 16:45:43 +0100, Adrian Perez de Castro <aperez@igalia.com> wrote:
> The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
> instance of Xapian::TermIterator is dereferenced. In my particular case,
> the culpript is a cache file of Claws-Mail, as seen in the following GDB
> session:

Not quite NULL, (nor is it quite dereferencing---this is nasty C++
overloading), but yeah, the idea is the same. We need to protect all of
our "calls" to this overloaded operator to not call it when the iterator
is equal to the value returned by termlist_end ().

On Thu, 19 Nov 2009 20:23:15 -0600, Jeffrey Ollie <jeff@ocjtech.us> wrote:
> I straced some of the crashes, and the last file that was read before
> the crash was a malformed message.  I've attached one of the messages.
>  I've been using offlineimap to sync my gmail mailbox to my laptop so
> that I can use notmuch.  offlineimap isn't the most stable program,
> but I'm not sure yet if offlineimap is causing the problem or if
> that's the way the message is in gmail.

Thanks for the file. I never like to push code that I haven't tested, so
this was very helpful.

Below is the patch that I just pushed which seems to do the trick.

-Carl

commit 31b54bc78735c628035a046e526ac4c596d830cf
Author: Carl Worth <cworth@cworth.org>
Date:   Fri Nov 20 12:06:11 2009 +0100

    Avoid access of a Xapian iterator's object when there's nothing
    there.
    
    This eliminates a crash when a message (either corrupted or a
    non-mail
    file that wasn't properly detected as not being mail) has no
    In-Reply-To
    header, (and so few terms that trying to skip to the prefix of the
    In-Reply-To terms actually brings us to the end of the termlist).

diff --git a/lib/message.cc b/lib/message.cc
index 9488fb6..41dddd0 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -285,7 +285,8 @@ _notmuch_message_get_in_reply_to (notmuch_message_t
*message
     i = message->doc.termlist_begin ();
     i.skip_to (prefix);
 
-    in_reply_to = *i;
+    if (i != message->doc.termlist_end ())
+       in_reply_to = *i;
 
     /* It's perfectly valid for a message to have no In-Reply-To
      * header. For these cases, we return an empty string. */
@@ -332,10 +333,10 @@ notmuch_message_get_thread_id (notmuch_message_t
     *message)
        return message->thread_id;
 
     i = message->doc.termlist_begin ();
-
     i.skip_to (prefix);
 
-    id = *i;
+    if (i != message->doc.termlist_end ())
+       id = *i;
 
     if (i == message->doc.termlist_end () || id[0] != *prefix)
        INTERNAL_ERROR ("Message with document ID of %d has no thread
        ID.\n",
@@ -466,7 +467,7 @@ notmuch_message_get_tags (notmuch_message_t
        *message)
 
     i.skip_to (prefix);
 
-    while (1) {
+    while (i != end) {
        tag = *i;
 
        if (tag.empty () || tag[0] != *prefix)

Re: Segfault searching for tags

[Jeffrey Ollie]

On Fri, Nov 20, 2009 at 5:32 AM, Carl Worth <cworth@cworth.org> wrote:
> On Thu, 19 Nov 2009 16:45:43 +0100, Adrian Perez de Castro <aperez@igalia.com> wrote:
>> The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
>> instance of Xapian::TermIterator is dereferenced. In my particular case,
>> the culpript is a cache file of Claws-Mail, as seen in the following GDB
>> session:
>
> Not quite NULL, (nor is it quite dereferencing---this is nasty C++
> overloading), but yeah, the idea is the same. We need to protect all of
> our "calls" to this overloaded operator to not call it when the iterator
> is equal to the value returned by termlist_end ().
>
> On Thu, 19 Nov 2009 20:23:15 -0600, Jeffrey Ollie <jeff@ocjtech.us> wrote:
>> I straced some of the crashes, and the last file that was read before
>> the crash was a malformed message.  I've attached one of the messages.
>
> Thanks for the file. I never like to push code that I haven't tested, so
> this was very helpful.
>
> Below is the patch that I just pushed which seems to do the trick.

Ah, excellent!  This does indeed seem to prevent the crash.  Now I
just need to figure out how to get all my mail out of GMail.

-- 
Jeff Ollie

Re: Segfault searching for tags

[Jan Janak]

On Fri, Nov 20, 2009 at 2:10 PM, Jeffrey Ollie <jeff@ocjtech.us> wrote:
> On Fri, Nov 20, 2009 at 5:32 AM, Carl Worth <cworth@cworth.org> wrote:
>> On Thu, 19 Nov 2009 16:45:43 +0100, Adrian Perez de Castro <aperez@igalia.com> wrote:
>>> The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
>>> instance of Xapian::TermIterator is dereferenced. In my particular case,
>>> the culpript is a cache file of Claws-Mail, as seen in the following GDB
>>> session:
>>
>> Not quite NULL, (nor is it quite dereferencing---this is nasty C++
>> overloading), but yeah, the idea is the same. We need to protect all of
>> our "calls" to this overloaded operator to not call it when the iterator
>> is equal to the value returned by termlist_end ().
>>
>> On Thu, 19 Nov 2009 20:23:15 -0600, Jeffrey Ollie <jeff@ocjtech.us> wrote:
>>> I straced some of the crashes, and the last file that was read before
>>> the crash was a malformed message.  I've attached one of the messages.
>>
>> Thanks for the file. I never like to push code that I haven't tested, so
>> this was very helpful.
>>
>> Below is the patch that I just pushed which seems to do the trick.
>
> Ah, excellent!  This does indeed seem to prevent the crash.  Now I
> just need to figure out how to get all my mail out of GMail.

I did exactly that with offlineimap. It crashes from time to time, but
then you can just restart it and continue.

A few days ago I sent a patch which converts mail subdirectories to
tags and because Gmail IMAP server converts labels to subdirectories,
you can use that to convert gmail's labels to notmuch tags
automatically.

   -- Jan

Re: Segfault searching for tags

[Carl Worth]

On Fri, 20 Nov 2009 14:20:45 +0100, Jan Janak <jan@ryngle.com> wrote:
> > Ah, excellent!  This does indeed seem to prevent the crash.  Now I
> > just need to figure out how to get all my mail out of GMail.
> 
> I did exactly that with offlineimap. It crashes from time to time, but
> then you can just restart it and continue.

It sounds like a lot of people are wanting to do that. So it probably
wouldn't hurt for someone who's done it to write up instructions for
that, (or post a link to existing instructions).

I'd say that it might make sense to put these up in a wiki, (and I'll
probably eventually setup an ikiwiki instance for notmuchmail.org just
like I do for cworth.org and cairographics.org). But then again, we're
*already* using email, and I've got this half-baked idea about being
able to just tag useful posts and have them appear on the webpage.

> A few days ago I sent a patch which converts mail subdirectories to
> tags and because Gmail IMAP server converts labels to subdirectories,
> you can use that to convert gmail's labels to notmuch tags
> automatically.

I haven't lost that. Sorry it's taking me a bit to get to it, though!

I think there were two different patches from two people for this
feature. So I've got those both tagged and will review them soon.

-Carl

Re: Segfault searching for tags

[Adrian Perez de Castro]

[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]

On Fri, 20 Nov 2009 12:32:41 +0100, Carl wrote:

> On Thu, 19 Nov 2009 16:45:43 +0100, Adrian Perez de Castro <aperez@igalia.com> wrote:
> > The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
> > instance of Xapian::TermIterator is dereferenced. In my particular case,
> > the culpript is a cache file of Claws-Mail, as seen in the following GDB
> > session:
> 
> Not quite NULL, (nor is it quite dereferencing---this is nasty C++
> overloading), but yeah, the idea is the same. We need to protect all of
> our "calls" to this overloaded operator to not call it when the iterator
> is equal to the value returned by termlist_end ().

Well, of course you are right, it is an overloaded operator, which
(unfortunately, IMHO) looks like a pointer dereference. That is exactly
one of the things that I find more confusing about C++: it has features
like operator overloading which look cool initially, but that in the end
imply more complexity than needed. I can understand why you decided to
wrap Xapian with a plain C API :)
 
> On Thu, 19 Nov 2009 20:23:15 -0600, Jeffrey Ollie <jeff@ocjtech.us> wrote:
> > I straced some of the crashes, and the last file that was read before
> > the crash was a malformed message.  I've attached one of the messages.
> >  I've been using offlineimap to sync my gmail mailbox to my laptop so
> > that I can use notmuch.  offlineimap isn't the most stable program,
> > but I'm not sure yet if offlineimap is causing the problem or if
> > that's the way the message is in gmail.
> 
> Thanks for the file. I never like to push code that I haven't tested, so
> this was very helpful.
> 
> Below is the patch that I just pushed which seems to do the trick.

I can confirm that this patch avoids the segfault in my case, too. Thanks
a lot for the quick fix.

Best regards,

-- 
Adrian Perez de Castro <aperez@igalia.com>
Igalia - Free Software Engineering

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Segfault searching for tags

[Carl Worth]

On Fri, 20 Nov 2009 20:03:00 +0100, Adrian Perez de Castro <aperez@igalia.com> wrote:
> Well, of course you are right, it is an overloaded operator, which
> (unfortunately, IMHO) looks like a pointer dereference. That is exactly
> one of the things that I find more confusing about C++: it has features
> like operator overloading which look cool initially, but that in the end
> imply more complexity than needed. I can understand why you decided to
> wrap Xapian with a plain C API :)

I'm glad you agree.

Though I should mention that I earned my summer's salary during an
internship once by solving a performance problem that had dodged the
engineers on the project, (since they overlooked an overloaded array
subscript operator on a std::string class as something that could be
expensive---profiling made it obvious, and a temporary copy to a real
array with a real subscript fixed the bug).

So I can't say that operator overloading never helped me. But I know I
left that internship determined not to use it myself.

> I can confirm that this patch avoids the segfault in my case, too. Thanks
> a lot for the quick fix.

Excellent. I'm glad to hear it worked for you.

I'm sorry that the bug was there, since this was a regression that's
come back once or twice now. The project is overdue for a test suite
already...

-Carl