Re: [bug] possible condition depending on uninitialized value in _notmuch_message_sync
On Fri, May 20 2022 at 09:46 -03, David Bremner wrote: > Eliza Velasquez writes: > >> On Mon, May 16 2022 at 06:47 -03, David Bremner wrote: >> >>> Ideally of course I'd like a reproducer in C. It would help to have >>> line numbers in the valgrind output. It might be enough you install the >>> notmuch debug symbols? >> >> Took me a while to figure out the debugging workflow in NixOS, but I >> managed to capture the line numbers. At messsage.cc:1333, at the second >> condition below: >> > [snip] >> So I guess `message->modified' isn't correctly initialized, at least >> according to valgrind. >> >> -- >> Eliza > > Can you see if the following change quiets valgrind? > > diff --git a/lib/message.cc b/lib/message.cc > index 63b216b6..bd3cb5af 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -169,6 +169,7 @@ _notmuch_message_create_for_document (const void > *talloc_owner, > > message->doc = doc; > message->termpos = 0; > +message->modified = FALSE; > > return message; > } That seems to have fixed it. Valgrind is very pleased with all of the test cases now :) ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [bug] possible condition depending on uninitialized value in _notmuch_message_sync
Eliza Velasquez writes: > On Mon, May 16 2022 at 06:47 -03, David Bremner wrote: > >> Ideally of course I'd like a reproducer in C. It would help to have >> line numbers in the valgrind output. It might be enough you install the >> notmuch debug symbols? > > Took me a while to figure out the debugging workflow in NixOS, but I > managed to capture the line numbers. At messsage.cc:1333, at the second > condition below: > [snip] > So I guess `message->modified' isn't correctly initialized, at least > according to valgrind. > > -- > Eliza Can you see if the following change quiets valgrind? diff --git a/lib/message.cc b/lib/message.cc index 63b216b6..bd3cb5af 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -169,6 +169,7 @@ _notmuch_message_create_for_document (const void *talloc_owner, message->doc = doc; message->termpos = 0; +message->modified = FALSE; return message; } ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [bug] possible condition depending on uninitialized value in _notmuch_message_sync
Eliza Velasquez writes:
> It becomes very clear why this error only happens when removing a
> non-existent tag if you look at at message.cc:1570...
>
> --8<---cut here---start->8---
> try {
> message->doc.remove_term (term);
> message->modified = true;
> } catch (const Xapian::InvalidArgumentError) {
> /* We'll let the philosophers try to wrestle with the
>* question of whether failing to remove that which was not
>* there in the first place is failure. For us, we'll silently
>* consider it all good. */
> }
> --8<---cut here---end--->8---
OK, I see why that assignment gets skipped. I think that's not the
actual bug, but rather message->modified should be initialized in
_notmuch_message_create_for_document. I'll have a closer look later
today
d
.
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [bug] possible condition depending on uninitialized value in _notmuch_message_sync
On Mon, May 16 2022 at 06:47 -03, David Bremner wrote:
> Ideally of course I'd like a reproducer in C. It would help to have
> line numbers in the valgrind output. It might be enough you install the
> notmuch debug symbols?
Took me a while to figure out the debugging workflow in NixOS, but I
managed to capture the line numbers. At messsage.cc:1333, at the second
condition below:
--8<---cut here---start->8---
/* Synchronize changes made to message->doc out into the database. */
void
_notmuch_message_sync (notmuch_message_t *message)
{
if (_notmuch_database_mode (message->notmuch) ==
NOTMUCH_DATABASE_MODE_READ_ONLY)
return;
if (! message->modified)
return;
...
}
--8<---cut here---end--->8---
It becomes very clear why this error only happens when removing a
non-existent tag if you look at at message.cc:1570...
--8<---cut here---start->8---
try {
message->doc.remove_term (term);
message->modified = true;
} catch (const Xapian::InvalidArgumentError) {
/* We'll let the philosophers try to wrestle with the
* question of whether failing to remove that which was not
* there in the first place is failure. For us, we'll silently
* consider it all good. */
}
--8<---cut here---end--->8---
So I guess `message->modified' isn't correctly initialized, at least
according to valgrind.
--
Eliza
___
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-le...@notmuchmail.org
Re: [bug] possible condition depending on uninitialized value in _notmuch_message_sync
Eliza Velasquez writes: > Is it possible then that there's a potential memory error with removing > a non-existent tag on a message? I wanted to ask about this on the > mailing list before diving in deeper, since this isn't quite the latest > version of notmuch and I wasn't sure if it had been fixed in 0.36. I > searched the mailing list archives for this particular issue, but I > wasn't able to find anything. Ideally of course I'd like a reproducer in C. It would help to have line numbers in the valgrind output. It might be enough you install the notmuch debug symbols? d ___ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org
