new valgrind errors in riched20

[Dylan Smith]

The error was a memory access of a freed object.  In ME_AddUndoItem I
checked the top of the undo stack to end a coalescing undo transaction,
assuming that this should be either a valid undo item, or NULL, instead
it was already freed.

The undo item being added was actually being added to the redo stack by
ME_Undo, and before this was done the top of the undo stack was
destroyed by not set to NULL, thus causing the valgrind error.

I fixed this in two places. First of all I moved my code to
conditionally turn a coalescing end transaction into an actual end
transaction, since it doesn't need to be done when adding to the redo
stack.  Second of all, I made sure the undo and redo stack are in
valid states for ME_Undo and ME_Redo before calling ME_AddUndoItem or
ME_PlayItem since I could see someone else making the same assumption.
This should fix the error and make it harder for a regression to occur.

Could you verify that Valgrind tests pass with the patch I attached.

I'll try to get started on using Valgrind, but for now I don't have it set
up.

On Fri, Jun 27, 2008 at 11:08 PM, Dan Kegel <dank at kegel.com> wrote:

> Seem to be four or so new valgrind warnings in riched20 today,
> probably due to Dylan's changes (though my cat may have been
> sufing to friskies.com and affected the results, who knows):
> http://kegel.com/wine/valgrind/logs-2008-06-27/vg-riched20_editor-diff.txt
>
> + Invalid read of size 4
> +    at  ME_AddUndoItem (undo.c:59)
> +    by  ME_SetParaFormat (para.c:362)
> +    by  ME_PlayUndoItem (undo.c:288)
> +    by  ME_Undo (undo.c:360)
> +    by  RichEditWndProc_common (editor.c:2067)
> +    by  RichEditWndProcA (editor.c:3386)
> +    by  ??? (library.h:163)
> +    by  call_window_proc (winproc.c:457)
> +    by  WINPROC_call_window (winproc.c:2207)
> +    by  call_window_proc (message.c:1639)
> +    by  send_message (message.c:2463)
> +    by  SendMessageA (message.c:2608)
> +    by  test_EM_SETUNDOLIMIT (editor.c:2238)
> +    by  func_editor (editor.c:4581)
> +    by  run_test (test.h:449)
> +    by  main (test.h:498)
>
> ...
>
> + Conditional jump or move depends on uninitialised value(s)
> +    at  test_EM_AUTOURLDETECT (editor.c:1719)
> +    by  func_editor (editor.c:4600)
> +    by  run_test (test.h:449)
> +    by  main (test.h:498)
> +  Uninitialised value was created by a client request
> +    at  mark_block_uninitialized (heap.c:164)
> +    by  RtlAllocateHeap (heap.c:1239)
> +    by  heap_alloc (editor.h:28)
> +    by  RichEditWndProc_common (editor.c:2573)
> +    by  RichEditWndProcA (editor.c:3386)
> +    by  ??? (library.h:163)
> +    by  call_window_proc (winproc.c:457)
> +    by  WINPROC_call_window (winproc.c:2207)
> +    by  call_window_proc (message.c:1639)
> +    by  send_message (message.c:2463)
> +    by  SendMessageA (message.c:2608)
> +    by  test_EM_AUTOURLDETECT (editor.c:1705)
> +    by  func_editor (editor.c:4600)
> +    by  run_test (test.h:449)
> +    by  main (test.h:498)
>
> ...
>
> + Invalid write of size 4
> +    at  ME_AddUndoItem (undo.c:62)
> +    by  ME_InternalDeleteText (caret.c:322)
> +    by  ME_PlayUndoItem (undo.c:308)
> +    by  ME_Undo (undo.c:360)
> +    by  RichEditWndProc_common (editor.c:2067)
> +    by  RichEditWndProcW (editor.c:3381)
> +    by  ??? (library.h:163)
> +    by  call_window_proc (winproc.c:457)
> +    by  WINPROC_CallProcAtoW (winproc.c:1011)
> +    by  WINPROC_call_window (winproc.c:2209)
> +    by  call_window_proc (message.c:1639)
> +    by  send_message (message.c:2463)
> +    by  SendMessageA (message.c:2608)
> +    by  test_undo_coalescing (editor.c:4398)
> +    by  func_editor (editor.c:4602)
> +    by  run_test (test.h:449)
> +    by  main (test.h:498)
>
> ...
>
> + Conditional jump or move depends on uninitialised value(s)
> +    at  ME_AddUndoItem (undo.c:59)
> +    by  ME_InternalDeleteText (caret.c:322)
> +    by  ME_PlayUndoItem (undo.c:308)
> +    by  ME_Undo (undo.c:360)
> +    by  RichEditWndProc_common (editor.c:2067)
> +    by  RichEditWndProcW (editor.c:3381)
> +    by  ??? (library.h:163)
> +    by  call_window_proc (winproc.c:457)
> +    by  WINPROC_CallProcAtoW (winproc.c:1011)
> +    by  WINPROC_call_window (winproc.c:2209)
> +    by  call_window_proc (message.c:1639)
> +    by  send_message (message.c:2463)
> +    by  SendMessageA (message.c:2608)
> +    by  test_undo_coalescing (editor.c:4398)
> +    by  func_editor (editor.c:4602)
> +    by  run_test (test.h:449)
> +    by  main (test.h:498)
> +  Uninitialised value was created by a client request
> +    at  mark_block_uninitialized (heap.c:164)
> +    by  RtlAllocateHeap (heap.c:1239)
> +    by  heap_alloc (editor.h:28)
> +    by  ME_MakeStringN (string.c:46)
> +    by  ME_InternalDeleteText (caret.c:326)
> +    by  ME_PlayUndoItem (undo.c:308)
> +    by  ME_Undo (undo.c:360)
> +    by  RichEditWndProc_common (editor.c:2067)
> +    by  RichEditWndProcW (editor.c:3381)
> +    by  ??? (library.h:163)
> +    by  call_window_proc (winproc.c:457)
> +    by  WINPROC_CallProcAtoW (winproc.c:1011)
> +    by  WINPROC_call_window (winproc.c:2209)
> +    by  call_window_proc (message.c:1639)
> +    by  send_message (message.c:2463)
> +    by  SendMessageA (message.c:2608)
> +    by  test_undo_coalescing (editor.c:4398)
> +    by  func_editor (editor.c:4602)
> +    by  run_test (test.h:449)
> +    by  main (test.h:498)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.winehq.org/pipermail/wine-devel/attachments/20080628/07fbc8f0/attachment.htm 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 0001-richedit-Fixed-Valgrind-error-related-to-undoing.diff.txt
Url: http://www.winehq.org/pipermail/wine-devel/attachments/20080628/07fbc8f0/attachment.txt