*** Bug 629466 has been marked as a duplicate of this bug. ***

looks related to bug 628497

Valgrind found this issue for me, and it seems related, like evo causing memory corruption and crash in another module. Akhil's bug maybe just another result of the same reason.

==10711== Invalid read of size 4
==10711==    at 0x43109A7: e_bit_array_delete_real (e-bit-array.c:99)
==10711==    by 0x4310AF5: e_bit_array_delete (e-bit-array.c:128)
==10711==    by 0x41C5E00: e_selection_model_array_delete_rows (e-selection-model-array.c:81)
==10711==    by 0x40FC86C: model_rows_deleted (e-table-selection-model.c:163)
==10711==    by 0x431EAEE: e_marshal_VOID__INT_INT (e-marshal.c:1016)
==10711==    by 0x47128DC: g_closure_invoke (gclosure.c:766)
==10711==    by 0x472A96B: signal_emit_unlocked_R (gsignal.c:3252)
==10711==    by 0x4729CBA: g_signal_emit_valist (gsignal.c:2983)
==10711==    by 0x4729FA6: g_signal_emit (gsignal.c:3040)
==10711==    by 0x40FB065: e_table_model_rows_deleted (e-table-model.c:584)
==10711==    by 0x5F391CD: redo_queries (e-cal-model.c:2364)
==10711==    by 0x5F39696: e_cal_model_set_search_query (e-cal-model.c:2450)
==10711==  Address 0x6c6afdc is 0 bytes after a block of size 4 alloc'd
==10711==    at 0x4005CD2: realloc (vg_replace_malloc.c:476)
==10711==    by 0x479EAF9: g_realloc (gmem.c:181)
==10711==    by 0x479EE46: g_realloc_n (gmem.c:314)
==10711==    by 0x4310A5C: e_bit_array_delete_real (e-bit-array.c:113)
==10711==    by 0x4310AF5: e_bit_array_delete (e-bit-array.c:128)
==10711==    by 0x41C5E00: e_selection_model_array_delete_rows (e-selection-model-array.c:81)
==10711==    by 0x40FC86C: model_rows_deleted (e-table-selection-model.c:163)
==10711==    by 0x431EAEE: e_marshal_VOID__INT_INT (e-marshal.c:1016)
==10711==    by 0x47128DC: g_closure_invoke (gclosure.c:766)
==10711==    by 0x472A96B: signal_emit_unlocked_R (gsignal.c:3252)
==10711==    by 0x4729CBA: g_signal_emit_valist (gsignal.c:2983)
==10711==    by 0x4729FA6: g_signal_emit (gsignal.c:3040)

Created attachment 170465 [details] [review]
proposed evo patch

for evolution;

Thomas, could you apply this patch and give it a try, please? While I was able to reproduce a similar crash without this patch, I'm not able to reproduce it with it, but it's possible I didn't try enough. Thanks in advance.

*** Bug 628522 has been marked as a duplicate of this bug. ***

I don't think the trace in comment #3 is going to be causing memory corruption. That happens when shrinking an e-bit-array by deleting from the middle, and it's only *reading* from off the end of the array; copying bit N+1 into bit N when the array is only N bits long.

It's an off-by-one in the copying, which I think is better fixed like this:

--- a/e-util/e-bit-array.c
+++ b/e-util/e-bit-array.c
@@ -78,7 +78,7 @@ e_bit_array_delete_real (EBitArray *eba, gint row, gboolean move_selection_mode)
        if (eba->bit_count >= 0) {
                guint32 bitmask;
                box = row >> 5;
-               last = eba->bit_count >> 5;
+               last = (eba->bit_count - 1) >> 5;
 
                /* Build bitmasks for the left and right half of the box */
                bitmask = BITMASK_RIGHT (row) >> 1;

I don't think the patch in comment #4 is the best approach -- if you have a bug which causes you to read off the end of the array, it's better to *fix* that bug rather than deliberately making the array larger than it needs to be.

And I don't think *any* of this is actually relevant to the originally-reported bug, so I'll file the patch against bug 628522 which was opened specifically for the e-bit-array problem.

(In reply to comment #6)
> I don't think the patch in comment #4 is the best approach -- if you have a bug
> which causes you to read off the end of the array, it's better to *fix* that
> bug rather than deliberately making the array larger than it needs to be.

Yes, I obviously overlooked that, and it seemed to me that the rest of the EBitArray expected a "bit_count + 1" array size.

I'm obsoleting my "patch" in a favour of yours in bug #628522 and I'm reopening it.

Thomas, could you try with the latest patch from bug #628522, please? Thanks in advance.

(In reply to comment #7)
> (In reply to comment #6)
> > I don't think the patch in comment #4 is the best approach -- if you have a bug
> > which causes you to read off the end of the array, it's better to *fix* that
> > bug rather than deliberately making the array larger than it needs to be.
> 
> Yes, I obviously overlooked that, and it seemed to me that the rest of the
> EBitArray expected a "bit_count + 1" array size.
> 
> I'm obsoleting my "patch" in a favour of yours in bug #628522 and I'm reopening
> it.
> 
> Thomas, could you try with the latest patch from bug #628522, please? Thanks in
> advance.

Yes, will test it both on the new 2.32 version and on my "ancient" 2.28.

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > I don't think the patch in comment #4 is the best approach -- if you have a bug
> > > which causes you to read off the end of the array, it's better to *fix* that
> > > bug rather than deliberately making the array larger than it needs to be.
> > 
> > Yes, I obviously overlooked that, and it seemed to me that the rest of the
> > EBitArray expected a "bit_count + 1" array size.
> > 
> > I'm obsoleting my "patch" in a favour of yours in bug #628522 and I'm reopening
> > it.
> > 
> > Thomas, could you try with the latest patch from bug #628522, please? Thanks in
> > advance.
> 
> Yes, will test it both on the new 2.32 version and on my "ancient" 2.28.

My tests with the 2.32 version repeatedly caused that hang in _xcb_conn_wait in library libxcb1. See https://bugzilla.gnome.org/show_bug.cgi?id=627620#c11. Here is the stack trace. Please tell me if I should file a separate bug.


(gdb) bt

+ Trace 223773

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)

> > > Thomas, could you try with the latest patch from bug #628522, please? 

Looks good, so far. On both machines.

(In reply to comment #9)
> My tests with the 2.32 version repeatedly caused that hang in _xcb_conn_wait
> in library libxcb1.
> See https://bugzilla.gnome.org/show_bug.cgi?id=627620#c11.
> Here is the stack trace. Please tell me if I should file a separate bug.

We can deal with it in bug #627620, as you have it filled there.

(In reply to comment #10)
> > > > Thomas, could you try with the latest patch from bug #628522, please? 
> 
> Looks good, so far. On both machines.

Thanks, good, so I'm marking this as a duplicate of that bug.

*** This bug has been marked as a duplicate of bug 628522 ***

I still don't see how the trace which was given when this bug was originally filed could possibly be caused by bug 628522. But 628522 doesn't cause memory corruption -- the only crash it could cause would be *in* e_bit_array_delete_real() as it tries to read from the word after the end of the array.

Thomas, please could you confirm that you can reliably reproduce the crash you first reported in trace 223674, and that it goes away when you apply the patch from bug 628522 and nothing more?

(In reply to comment #12)
> I still don't see how the trace which was given when this bug was originally
> filed could possibly be caused by bug 628522. But 628522 doesn't cause memory
> corruption -- the only crash it could cause would be *in*
> e_bit_array_delete_real() as it tries to read from the word after the end of
> the array.
> 
> Thomas, please could you confirm that you can reliably reproduce the crash you
> first reported in trace 223674, and that it goes away when you apply the patch
> from bug 628522 and nothing more?

Yes. I can confirm that now with version 2.32. Reverted the patch and it crashed
in pango-layout.c, one time with a slightly different stack trace, though, which follows. But I could definitely reproduce it and it goes away with the patch applied.

(gdb) bt

+ Trace 223817

*** Bug 631089 has been marked as a duplicate of this bug. ***