<<< Date Index >>>     <<< Thread Index >>>

Re: Segmentation fault



Hello Alex,

          [ This time with the right patch and mutt-dev cced ]

> ==31043== Invalid read of size 4
> ==31043==    at 0x80A1A73: find_subject (thread.c:431)
> ==31043==    by 0x80A1C2A: pseudo_threads (thread.c:503)
> ==31043==    by 0x80A242C: mutt_sort_threads (thread.c:956)
> ==31043==    by 0x80A0509: mutt_sort_headers (sort.c:291)
> ==31043==    by 0x80B9418: imap_expunge_mailbox (imap.c:257)
> ==31043==    by 0x80B890D: imap_cmd_finish (command.c:271)
> ==31043==    by 0x80B8669: imap_cmd_step (command.c:157)
> ==31043==    by 0x80B8837: imap_exec (command.c:210)
> ==31043==    by 0x80BA8EE: imap_sync_mailbox (imap.c:1099)
> ==31043==    by 0x8081398: mx_sync_mailbox (mx.c:1177)
> ==31043==  Address 0x1BDDEC7C is 68 bytes inside a block of size 80 free'd
> ==31043==    at 0x1B902490: free (vg_replace_malloc.c:153)
> ==31043==    by 0x80A3FF7: safe_free (lib.c:131)
> ==31043==    by 0x8080F40: mx_update_tables (mx.c:1101)
> ==31043==    by 0x80B9401: imap_expunge_mailbox (imap.c:256)
> ==31043==    by 0x80B890D: imap_cmd_finish (command.c:271)
> ==31043==    by 0x80B8669: imap_cmd_step (command.c:157)
> ==31043==    by 0x80B8837: imap_exec (command.c:210)
> ==31043==    by 0x80BA8EE: imap_sync_mailbox (imap.c:1099)
> ==31043==    by 0x8081398: mx_sync_mailbox (mx.c:1177)
> ==31043==    by 0x8062448: mutt_index_menu (curs_main.c:1012)

As you can see the invalid read of size 4 only happens at line 431 or
later. This means ptr is valid (actually allocated) but the header got
deleted where it points to. I have one idea how this can happen: The
subj_hash is allowes dups and the subject could be added twice because
mx_update_context (in the inital opening) and one in imap_fetch_message.
Maybe it gets on deletion only removed once. So here is a patch which
could fix this behaviour.

Could you please apply and rerun using valgrind and report problems
back?

> ==11945== Invalid read of size 1
> ==11945==    at 0x1B901A63: strcmp (mac_replace_strmem.c:250)
> ==11945==    by 0x807153D: hash_find_hash (hash.c:108)
> ==11945==    by 0x80A24A1: mutt_sort_threads (thread.c:921)
> ==11945==    by 0x80A0509: mutt_sort_headers (sort.c:291)
> ==11945==    by 0x805F5F4: update_index (curs_main.c:317)
> ==11945==    by 0x8064BC3: mutt_index_menu (curs_main.c:492)
> ==11945==    by 0x8077A75: main (main.c:934)
> ==11945==  Address 0x1BCACC48 is 0 bytes inside a block of size 56 free'd
> ==11945==    at 0x1B902490: free (vg_replace_malloc.c:153)
> ==11945==    by 0x80A3FF7: safe_free (lib.c:131)
> ==11945==    by 0x80A60BD: mutt_free_envelope (muttlib.c:656)
> ==11945==    by 0x80BBE74: imap_fetch_message (message.c:474)
> ==11945==    by 0x808197F: mx_open_message (mx.c:1417)
> ==11945==    by 0x805D2AF: mutt_copy_message (copy.c:677)
> ==11945==    by 0x80566B0: mutt_display_message (commands.c:146)
> ==11945==    by 0x806043F: mutt_index_menu (curs_main.c:1144)
> ==11945==    by 0x8077A75: main (main.c:934)

I have to think over this again.

        Thomas
--- a/hash.c
+++ b/hash.c
@@ -123,12 +123,16 @@
      * required for the case where we have multiple entries with the same
      * key
      */
+again:
     if ((data == ptr->data) || (!data && mutt_strcmp (ptr->key, key) == 0))
     {
       *last = ptr->next;
       if (destroy) destroy (ptr->data);
       FREE (&ptr);
-      return;
+      ptr = *last;
+      if (! ptr)
+        return;
+      goto again;
     }
   }
 }