[Bug 36636] New: valgrind shows an uninitialized write in shell32/tests/appbar.c
https://bugs.winehq.org/show_bug.cgi?id=36636 Bug ID: 36636 Summary: valgrind shows an uninitialized write in shell32/tests/appbar.c Product: Wine Version: 1.7.19 Hardware: x86 OS: Linux Status: NEW Keywords: download, source, testcase Severity: normal Priority: P2 Component: shell32 Assignee: wine-bugs(a)winehq.org Reporter: austinenglish(a)gmail.com ==20270== Syscall param writev(vector[...]) points to uninitialised byte(s) ==20270== at 0x4E952886: writev (in /usr/lib/libc-2.18.so) ==20270== by 0x7BC7FE6F: send_request (server.c:213) ==20270== by 0x7BC80001: wine_server_call (server.c:294) ==20270== by 0x5254AF8: put_message_in_queue (message.c:3141) ==20270== by 0x5254EA9: send_inter_thread_message (message.c:3209) ==20270== by 0x525518A: send_message (message.c:3277) ==20270== by 0x52555D1: SendMessageTimeoutW (message.c:3414) ==20270== by 0x4D221BC: SHAppBarMessage (appbar.c:142) ==20270== by 0x4A59513: test_setpos (appbar.c:222) ==20270== by 0x4A5ABF9: func_appbar (appbar.c:442) ==20270== by 0x4AB44E4: run_test (test.h:584) ==20270== by 0x4AB48D3: main (test.h:654) ==20270== Address 0x4cdfb20 is on thread 1's stack ==20270== Uninitialised value was created by a stack allocation ==20270== at 0x4A5940F: test_setpos (appbar.c:203) ==20270== -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |valgrind -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #1 from Bruno Jesus <00cpxxx(a)gmail.com> --- Created attachment 48793 --> https://bugs.winehq.org/attachment.cgi?id=48793 patch Please try the attached patch. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #2 from Austin English <austinenglish(a)gmail.com> --- Maybe. If I remove the suppression and apply the patch, instead I get: ../../../tools/runtest -q -P wine -T ../../.. -M shell32.dll -p shell32_test.exe.so appbar && touch appbar.ok preloader: Warning: failed to reserve range 00110000-68000000 preloader: Warning: failed to reserve range 7f000000-82000000 ==15640== Syscall param writev(vector[...]) points to uninitialised byte(s) ==15640== at 0x4EFB9D16: writev (in /usr/lib/libc-2.19.90.so) ==15640== by 0x7BC780ED: send_request (server.c:213) ==15640== by 0x7BC78272: wine_server_call (server.c:294) ==15640== by 0x5482B43: put_message_in_queue (message.c:3141) ==15640== by 0x5482E81: send_inter_thread_message (message.c:3209) ==15640== by 0x548312B: send_message (message.c:3277) ==15640== by 0x548353C: SendMessageTimeoutW (message.c:3414) ==15640== by 0x4E6E0D8: SHAppBarMessage (appbar.c:142) ==15640== by 0x4CCD40B: test_setpos (appbar.c:223) ==15640== by 0x4CCE868: func_appbar (appbar.c:443) ==15640== by 0x4D22597: run_test (test.h:584) ==15640== by 0x4D22963: main (test.h:654) ==15640== Address 0x4e4fb94 is on thread 1's stack ==15640== Uninitialised value was created by a stack allocation ==15640== at 0x4E6DC58: SHAppBarMessage (appbar.c:65) ==15640== ==15720== Syscall param writev(vector[...]) points to uninitialised byte(s) ==15720== at 0x4EFB9D16: writev (in /usr/lib/libc-2.19.90.so) ==15720== by 0x7BC780ED: send_request (server.c:213) ==15720== by 0x7BC78272: wine_server_call (server.c:294) ==15720== by 0x5482B43: put_message_in_queue (message.c:3141) ==15720== by 0x5482E81: send_inter_thread_message (message.c:3209) ==15720== by 0x548312B: send_message (message.c:3277) ==15720== by 0x548353C: SendMessageTimeoutW (message.c:3414) ==15720== by 0x4E6E0D8: SHAppBarMessage (appbar.c:142) ==15720== by 0x4CCD40B: test_setpos (appbar.c:223) ==15720== by 0x4CCE868: func_appbar (appbar.c:443) ==15720== by 0x4D22597: run_test (test.h:584) ==15720== by 0x4D22963: main (test.h:654) ==15720== Address 0x4e4fb94 is on thread 1's stack ==15720== Uninitialised value was created by a stack allocation ==15720== at 0x4E6DC58: SHAppBarMessage (appbar.c:65) ==15720== -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 Bruno Jesus <00cpxxx(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48793|0 |1 is obsolete| | CC| |00cpxxx(a)gmail.com --- Comment #3 from Bruno Jesus <00cpxxx(a)gmail.com> --- Created attachment 48794 --> https://bugs.winehq.org/attachment.cgi?id=48794 patch 2 Good, this patch may do the trick then. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #4 from Austin English <austinenglish(a)gmail.com> --- (In reply to Bruno Jesus from comment #3)
Created attachment 48794 [details] patch 2
Good, this patch may do the trick then.
Still one left: ../../../tools/runtest -q -P wine -T ../../.. -M shell32.dll -p shell32_test.exe.so appbar && touch appbar.ok preloader: Warning: failed to reserve range 00110000-68000000 preloader: Warning: failed to reserve range 7f000000-82000000 ==16530== Syscall param writev(vector[...]) points to uninitialised byte(s) ==16530== at 0x4EFB9D16: writev (in /usr/lib/libc-2.19.90.so) ==16530== by 0x7BC780ED: send_request (server.c:213) ==16530== by 0x7BC78272: wine_server_call (server.c:294) ==16530== by 0x5482B43: put_message_in_queue (message.c:3141) ==16530== by 0x5482E81: send_inter_thread_message (message.c:3209) ==16530== by 0x548312B: send_message (message.c:3277) ==16530== by 0x548353C: SendMessageTimeoutW (message.c:3414) ==16530== by 0x4E6E0D8: SHAppBarMessage (appbar.c:142) ==16530== by 0x4CCD41E: test_setpos (appbar.c:224) ==16530== by 0x4CCE87B: func_appbar (appbar.c:444) ==16530== by 0x4D225AA: run_test (test.h:584) ==16530== by 0x4D22976: main (test.h:654) ==16530== Address 0x4e4fb94 is on thread 1's stack ==16530== Uninitialised value was created by a stack allocation ==16530== at 0x4E6DC58: SHAppBarMessage (appbar.c:65) ==16530== -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 Bruno Jesus <00cpxxx(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48794|0 |1 is obsolete| | --- Comment #5 from Bruno Jesus <00cpxxx(a)gmail.com> --- Created attachment 48795 --> https://bugs.winehq.org/attachment.cgi?id=48795 patch 3 I think all abd variables are properly initialized now. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #6 from Austin English <austinenglish(a)gmail.com> --- (In reply to Bruno Jesus from comment #5)
Created attachment 48795 [details] patch 3
I think all abd variables are properly initialized now.
Valgrind agrees, works here. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #7 from Nikolay Sivov <bunglehead(a)gmail.com> --- (In reply to Bruno Jesus from comment #5)
Created attachment 48795 [details] patch 3
I think all abd variables are properly initialized now.
--- + memset(&command.abd, 0, sizeof(command.abd)); command.abd.hWnd = HandleToLong( data->hWnd ); command.abd.uCallbackMessage = data->uCallbackMessage; command.abd.uEdge = data->uEdge; --- That's too much, you only need to set command.abd.cbSize. Same applies for two other cases - it's better to add what's missing, instead of memset and immediately overwrite it. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #8 from Bruno Jesus <00cpxxx(a)gmail.com> --- (In reply to Nikolay Sivov from comment #7)
That's too much, you only need to set command.abd.cbSize. Same applies for two other cases - it's better to add what's missing, instead of memset and immediately overwrite it.
Thanks for the review, I will send an updated version. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #9 from Bruno Jesus <00cpxxx(a)gmail.com> --- Actually there is no cbSize inside the command.abd struct. 39 struct appbar_data_msg /* platform-independent data */ 40 { 41 ULONG hWnd; 42 UINT uCallbackMessage; 43 UINT uEdge; 44 RECT rc; 45 ULONGLONG lParam; 46 }; 47 48 struct appbar_cmd 49 { 50 ULONG return_map; 51 DWORD return_process; 52 struct appbar_data_msg abd; 53 }; And all existing fields are already being written. The problem may be somewhere else then. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #10 from Henri Verbeet <hverbeet(a)gmail.com> --- (In reply to Bruno Jesus from comment #9)
Actually there is no cbSize inside the command.abd struct.
39 struct appbar_data_msg /* platform-independent data */ 40 { 41 ULONG hWnd; 42 UINT uCallbackMessage; 43 UINT uEdge; 44 RECT rc; 45 ULONGLONG lParam; 46 }; ... And all existing fields are already being written. The problem may be somewhere else then.
I really didn't read the bug very closely, but notice that there's a 4 byte hole between "rc" and "lParam", because "lParam" is a 64-bit value and will be aligned on an 8 byte boundary. If this structure is not visible to applications you can trivially avoid the hole by reordering the fields, otherwise you could perhaps add an explicit "padding" field. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=36636 --- Comment #11 from Austin English <austinenglish(a)gmail.com> --- Still in 4.0-rc3: ==20528== Syscall param writev(vector[...]) points to uninitialised byte(s) ==20528== at 0x43567D3: writev (writev.c:26) ==20528== by 0x7BC75457: send_request (server.c:228) ==20528== by 0x7BC76158: server_call_unlocked (server.c:288) ==20528== by 0x7BC761AD: wine_server_call (server.c:321) ==20528== by 0x56A5D81: put_message_in_queue (message.c:3161) ==20528== by 0x56A36E7: send_inter_thread_message (message.c:3229) ==20528== by 0x56A39CA: send_message (message.c:3305) ==20528== by 0x56A3AFD: SendMessageTimeoutW (message.c:3449) ==20528== by 0x4C2FCAB: SHAppBarMessage (appbar.c:142) ==20528== by 0x4A7E458: test_setpos (appbar.c:222) ==20528== by 0x4A7F3F7: func_appbar (appbar.c:447) ==20528== by 0x4AD1E90: run_test (test.h:617) ==20528== by 0x4AD28CE: main (test.h:701) ==20528== Address 0x4c0fc68 is on thread 1's stack ==20528== in frame #8, created by SHAppBarMessage (appbar.c:65) ==20528== Uninitialised value was created by a stack allocation ==20528== at 0x4A7E39B: test_setpos (appbar.c:203) ==20528== -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org