From: SourceForge.net <no...@so...> - 2007-03-06 13:02:03
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Earnie Boyd (earnie) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-03-06 16:12:46
|
Bugs item #1674783, was opened at 2007-03-06 08:01 Message generated for change (Settings changed) made by earnie You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) >Assigned to: Nobody/Anonymous (nobody) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-03-09 11:27:23
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Comment added) made by jdrasch You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Nobody/Anonymous (nobody) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: jdrasch (jdrasch) Date: 2007-03-09 11:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-05-28 11:56:28
|
Bugs item #1674783, was opened at 2007-03-06 14:01 Message generated for change (Comment added) made by madwizard You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Nobody/Anonymous (nobody) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 13:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 12:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-05-30 08:50:22
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Comment added) made by jdrasch You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Nobody/Anonymous (nobody) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: jdrasch (jdrasch) Date: 2007-05-30 08:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 11:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 11:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-05-30 09:03:23
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Comment added) made by jdrasch You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Nobody/Anonymous (nobody) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: jdrasch (jdrasch) Date: 2007-05-30 09:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 08:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 11:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 11:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-06-04 21:45:12
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Comment added) made by keithmarshall You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) >Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 21:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 09:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 08:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 11:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 11:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-06-08 00:40:39
|
Bugs item #1674783, was opened at 2007-03-06 10:01 Message generated for change (Comment added) made by cstrauss You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 21:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 18:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 06:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 08:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 08:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-06-08 07:29:30
|
Bugs item #1674783, was opened at 2007-03-06 14:01 Message generated for change (Comment added) made by madwizard You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 09:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-08 02:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 23:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 11:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 10:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 13:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 12:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-07-12 18:12:30
|
Bugs item #1674783, was opened at 2007-03-06 10:01 Message generated for change (Comment added) made by cstrauss You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 15:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 04:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 21:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 18:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 06:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 08:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 08:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-07-12 18:30:59
|
Bugs item #1674783, was opened at 2007-03-06 07:01 Message generated for change (Comment added) made by obonilla You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 12:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 12:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 01:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 18:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 15:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 03:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 02:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 05:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 05:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-07-28 22:49:25
|
Bugs item #1674783, was opened at 2007-03-06 10:01 Message generated for change (Comment added) made by cstrauss You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 19:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 15:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 15:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 04:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 21:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 18:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 06:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 08:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 08:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-11-04 02:44:20
|
Bugs item #1674783, was opened at 2007-03-06 07:01 Message generated for change (Comment added) made by ibison You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-03 21:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 17:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 13:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 13:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 02:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 19:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 16:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 04:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 03:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 06:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 05:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2007-11-04 20:23:36
|
Bugs item #1674783, was opened at 2007-03-06 10:01 Message generated for change (Comment added) made by cstrauss You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: msys Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 18:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-03 23:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 19:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 15:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 15:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 04:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 21:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 18:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 06:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 08:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 08:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2008-06-10 02:59:44
|
Bugs item #1674783, was opened at 2007-03-06 05:01 Message generated for change (Comment added) made by jslab You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-09 19:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 12:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-03 19:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 15:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 11:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 11:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 00:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 17:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 14:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 02:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 01:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 04:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 03:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2008-07-11 02:32:58
|
Bugs item #1674783, was opened at 2007-03-06 10:01 Message generated for change (Comment added) made by cstrauss You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS >Group: Vista Issue >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Cesar Strauss (cstrauss) Date: 2008-07-10 23:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-09 23:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 18:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-04 00:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 19:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 15:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 15:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 04:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 21:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 18:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 06:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 08:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 08:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2008-09-23 19:41:56
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Comment added) made by eldonantonio You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: Vista Issue Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 19:41 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-07-11 02:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-10 02:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 20:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-04 02:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 22:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 18:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 18:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 07:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-08 00:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 21:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 09:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 08:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 11:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 11:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2008-09-23 19:42:22
|
Bugs item #1674783, was opened at 2007-03-06 13:01 Message generated for change (Comment added) made by eldonantonio You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: Vista Issue Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 19:42 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 19:41 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-07-11 02:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-10 02:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 20:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-04 02:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 22:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 18:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 18:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 07:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-08 00:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 21:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 09:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 08:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 11:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 11:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2008-09-25 02:49:10
|
Bugs item #1674783, was opened at 2007-03-06 10:01 Message generated for change (Comment added) made by cstrauss You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: Vista Issue Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- >Comment By: Cesar Strauss (cstrauss) Date: 2008-09-24 23:48 Message: Could you try the msysCORE package? It can be found at http://sourceforge.net/project/showfiles.php?group_id=2435&package_id=24963 Unpack it in a empty directory and run msys.bat. Does it help? Regards, Cesar ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 16:42 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 16:41 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-07-10 23:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-09 23:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 18:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-04 00:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 19:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 15:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 15:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 04:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 21:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 18:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 06:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 08:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 08:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2009-03-19 16:45:58
|
Bugs item #1674783, was opened at 2007-03-06 14:01 Message generated for change (Comment added) made by kappesser You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: Vista Issue Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: Wolf Stephan Kappesser (kappesser) Date: 2009-03-19 17:45 Message: Yes I have same problem on same plattform and it works with http://downloads.sourceforge.net/mingw/msysCORE-1.0.11-20080826.tar.gz?use_mirror=surfnet. Track could close ... ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-09-25 04:48 Message: Could you try the msysCORE package? It can be found at http://sourceforge.net/project/showfiles.php?group_id=2435&package_id=24963 Unpack it in a empty directory and run msys.bat. Does it help? Regards, Cesar ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 21:42 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 21:41 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-07-11 04:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-10 04:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 21:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-04 03:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-29 00:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 20:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 20:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 09:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-08 02:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 23:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 11:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 10:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 13:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 12:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2009-03-19 16:46:49
|
Bugs item #1674783, was opened at 2007-03-06 14:01 Message generated for change (Comment added) made by kappesser You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: Vista Issue Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: Wolf Stephan Kappesser (kappesser) Date: 2009-03-19 17:46 Message: Yes I have same problem on same plattform and it works with http://downloads.sourceforge.net/mingw/msysCORE-1.0.11-20080826.tar.gz?use_mirror=surfnet. Track could close ... ---------------------------------------------------------------------- Comment By: Wolf Stephan Kappesser (kappesser) Date: 2009-03-19 17:45 Message: Yes I have same problem on same plattform and it works with http://downloads.sourceforge.net/mingw/msysCORE-1.0.11-20080826.tar.gz?use_mirror=surfnet. Track could close ... ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-09-25 04:48 Message: Could you try the msysCORE package? It can be found at http://sourceforge.net/project/showfiles.php?group_id=2435&package_id=24963 Unpack it in a empty directory and run msys.bat. Does it help? Regards, Cesar ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 21:42 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 21:41 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-07-11 04:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-10 04:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 21:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-04 03:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-29 00:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 20:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 20:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 09:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-08 02:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 23:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 11:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 10:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 13:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 12:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |
From: SourceForge.net <no...@so...> - 2009-03-19 18:35:36
|
Bugs item #1674783, was opened at 2007-03-06 08:01 Message generated for change (Settings changed) made by earnie You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: MSYS Group: Vista Issue Status: Closed Resolution: Fixed Priority: 5 Private: No Submitted By: jdrasch (jdrasch) Assigned to: Cesar Strauss (cstrauss) Summary: Problem with Vista 64 bit ( Initial Comment: Hi, Msys didn't work on Vista 64 bit in the 32 bit emulation environment (<windir>/syswow64/cmd.exe) After a research I found the following in the nabble forums. http://www.nabble.com/MSYS-and-Vista-64-tf3292881.html#a9183478 The cygwin folks have figured out that Vista's WOW64 works differently than XP's WOW64. This patch should fix it. #### ChangeSet #### 2007-01-22 22:12:41-08:00, ob@... Work around what appears to be a bug in Vista 64-bits. The problem is related to how fork passes information to the child. Basically the child_info struct * is put in the lpReserved2 field of the STARTUPINFO struct that gets passed to CreateProcess(). In all versions of Windows, the size of the data to be passed is in the cbReserved2 field of the same structure, excep on Vista 64-bits. It seems that WOW64 (the 32-bit emmulation layer of 64-bit Vista) expects the size of the data passed in lpReserved2 to be the first word in that same pointer, only it expects the size in "elements" of a BYTE[] and HANDLE[] arrays (the MS C runtime has this convention). We work around it by filling in the size as (sizeof struct)/5 and zero padding the end of the child_info_* structs. ==== rt/src/winsup/cygwin/child_info.h ==== 2007-01-22 22:12:39-08:00, ob@... +4 -1 pad the end of the child_info_* structs and change the zero at the beginning for a placeholder for the size. --- 1.3/rt/src/winsup/cygwin/child_info.h 2001-10-15 15:22:32 -07:00 +++ 1.4/rt/src/winsup/cygwin/child_info.h 2007-01-22 22:12:39 -08:00 @@ -29,7 +29,7 @@ enum class child_info { public: - DWORD zero[1]; // must be zeroed + DWORD msv_cnt; // size of struct on vista64, zero on others DWORD cb; // size of this record DWORD type; // type of record int cygpid; // cygwin pid of child process @@ -53,6 +53,7 @@ public: jmp_buf jmp; // where child will jump to void *stacktop; // location of top of parent stack void *stackbottom; // location of bottom of parent stack + char filler[4]; }; class fhandler_base; @@ -67,6 +68,7 @@ public: int envc; char **envp; HANDLE myself_pinfo; + char filler[4]; }; class child_info_spawn: public child_info @@ -74,6 +76,7 @@ class child_info_spawn: public child_inf public: cygheap_exec_info *moreinfo; HANDLE hexec_proc; + char filler[4]; child_info_spawn (): moreinfo (NULL) {} ~child_info_spawn () ==== rt/src/winsup/cygwin/dcrt0.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -3 the child_info_* structs no longer have a zero at the beginning. --- 1.10/rt/src/winsup/cygwin/dcrt0.cc 2004-03-15 03:51:36 -08:00 +++ 1.11/rt/src/winsup/cygwin/dcrt0.cc 2007-01-22 22:12:40 -08:00 @@ -903,7 +903,6 @@ _dll_crt0 () if (GetEnvironmentVariable ("CYGWIN_TESTING", envbuf, sizeof (envbuf) - 1)) _cygwin_testing = 1; - char zeros[sizeof (fork_info->zero)] = {0}; #ifdef DEBUGGING strace.microseconds (); #endif @@ -924,8 +923,7 @@ _dll_crt0 () &hMainThread, 0, FALSE, DUPLICATE_SAME_ACCESS); GetStartupInfo (&si); - if (si.cbReserved2 >= EXEC_MAGIC_SIZE && - memcmp (fork_info->zero, zeros, sizeof (zeros)) == 0) + if (si.cbReserved2 >= EXEC_MAGIC_SIZE) { switch (fork_info->type) { ==== rt/src/winsup/cygwin/fork.cc ==== 2007-01-22 22:12:40-08:00, ob@... +2 -0 fill in the size of child_info_fork --- 1.4/rt/src/winsup/cygwin/fork.cc 2002-10-31 06:32:21 -08:00 +++ 1.5/rt/src/winsup/cygwin/fork.cc 2007-01-22 22:12:40 -08:00 @@ -430,6 +430,8 @@ fork_parent (HANDLE& hParent, dll *&firs init_child_info (PROC_FORK1, &ch, 1, subproc_ready); + ch.msv_cnt = (sizeof ch)/5; + ch.forker_finished = forker_finished; stack_base (ch); ==== rt/src/winsup/cygwin/sigproc.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info --- 1.5/rt/src/winsup/cygwin/sigproc.cc 2003-02-12 07:50:27 -08:00 +++ 1.6/rt/src/winsup/cygwin/sigproc.cc 2007-01-22 22:12:40 -08:00 @@ -849,6 +849,7 @@ init_child_info (DWORD chtype, child_inf { TRACE_IN; memset (ch, 0, sizeof *ch); + ch->msv_cnt = (sizeof *ch)/5; ch->cb = sizeof *ch; ch->type = chtype; ch->cygpid = pid; ==== rt/src/winsup/cygwin/spawn.cc ==== 2007-01-22 22:12:40-08:00, ob@... +1 -0 fill in the size of child_info_spawn --- 1.22/rt/src/winsup/cygwin/spawn.cc 2003-10-11 07:35:26 -07:00 +++ 1.23/rt/src/winsup/cygwin/spawn.cc 2007-01-22 22:12:40 -08:00 @@ -346,6 +346,7 @@ spawn_guts (HANDLE hToken, const char * STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; child_info_spawn ciresrv; + ciresrv.msv_cnt = (sizeof ciresrv)/5; si.lpReserved2 = (LPBYTE) &ciresrv; si.cbReserved2 = sizeof (ciresrv); ---------------------------------------------------------------------- Comment By: Wolf Stephan Kappesser (kappesser) Date: 2009-03-19 12:46 Message: Yes I have same problem on same plattform and it works with http://downloads.sourceforge.net/mingw/msysCORE-1.0.11-20080826.tar.gz?use_mirror=surfnet. Track could close ... ---------------------------------------------------------------------- Comment By: Wolf Stephan Kappesser (kappesser) Date: 2009-03-19 12:45 Message: Yes I have same problem on same plattform and it works with http://downloads.sourceforge.net/mingw/msysCORE-1.0.11-20080826.tar.gz?use_mirror=surfnet. Track could close ... ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-09-24 22:48 Message: Could you try the msysCORE package? It can be found at http://sourceforge.net/project/showfiles.php?group_id=2435&package_id=24963 Unpack it in a empty directory and run msys.bat. Does it help? Regards, Cesar ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 15:42 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: eldonantonio (eldonantonio) Date: 2008-09-23 15:41 Message: Hey guys, I'm using msys 1.0.11 and I'm still having this problem in Vista64. Trying to launch rxvt in a SysWOW64 cmd prompt opens 130 windows before closing them all. Did I miss something? Thanks, Antoine ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2008-07-10 22:32 Message: Logged In: YES user_id=1369729 Originator: NO The Vista patch seems to be successful, so I am closing this tracker item. Thanks for all the reports and contributions. Regards, Cesar ---------------------------------------------------------------------- Comment By: Alexander J. Vincent (jslab) Date: 2008-06-09 22:59 Message: Logged In: YES user_id=134163 Originator: NO I can confirm that using the MSYS-1.0.11 tarballed files as described by cstrauss fixes the bug I had launching bash, which was precisely as ibison described it. Reference [1] for my observations. [1] http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/2846365cf35a9b32/c8d07608e60e5467?lnk=gst&q=MozillaBuild+on+Vista+x64#c8d07608e60e5467 ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-11-04 15:23 Message: Logged In: YES user_id=1369729 Originator: NO Did you try installing the following package? http://downloads.sourceforge.net/mingw/MSYS-1.0.11-20070729.tar.bz2 It was just moved to a new location, you could have missed it. Unpack it on your existing MSYS "bin" directory. It should overwrite these files: msys-1.0.dll mount.exe ps.exe ---------------------------------------------------------------------- Comment By: ibison (ibison) Date: 2007-11-03 22:44 Message: Logged In: YES user_id=1928467 Originator: NO I cannot get the various versions of MSYS that I've tried to work on 64 bit Vista. Both sh and rxvt fail. sh complains about a fork and then quits, and MSYS continuously spawns multiple copies of rxvt until it crashes. Seems like you may have fixed the problem for cygwin. Can I trouble you for a fix for MSYS? ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-28 18:49 Message: Logged In: YES user_id=1369729 Originator: NO Oscar Bonilla wrote: > Note that the isVistaWOW64 variable will be true for > all vista versions, regardless of whether they are > Vista 32 or Vista 64. You are right, of course. Thanks for the heads-up. Here is a new version (patch4.txt) that uses the IsWow64Process API call. For this, I reused an interesting Cygwin technique called "auto-load functions" (see autoload.cc). If I understand correctly, it transparently intercepts the API call, loads the appropriate system DLL, and either forwards the call to the API function, or returns a failure code if the API doesn't exist at run-time. There is no need to use GetModuleHandle/GetProcAddress anymore... I also had to insert the prototype for IsWow64Process manually, because the w32api which is used to build MSYS pre-dates that. Regards, Cesar File Added: patch4.txt ---------------------------------------------------------------------- Comment By: Oscar Bonilla (obonilla) Date: 2007-07-12 14:30 Message: Logged In: YES user_id=219610 Originator: NO Note that the isVistaWOW64 variable will be true for all vista versions, regardless of whether they are Vista 32 or Vista 64. a better test would be: if(os_version_info.dwMajorVersion == 6) { HMODULE handle = GetModuleHandle("Kernel32"); if (handle) { FARPROC ph = GetProcAddress(handle, "GetSystemWow64DirectoryW"); if (ph && ph(path, sizeof(path))) isVistaWOW64 = 1; } } (typed in the browser, from memory since i don't have access to my windows box right now... so it needs to be tested.) ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-07-12 14:12 Message: Logged In: YES user_id=1369729 Originator: NO madwizard wrote: > The calculated size should be greater than the > actual size determined by cbReserved2; ... > I think if cbReserved2 is changed to size-4 > it might work and you don't have to worry about > reading past the data structure. Seems reasonable. See the attached patch (patch3.txt). Other changes to the original patch.txt are: 1) Move both count initializations to a single place (easier to maintain, less code duplication). 2) Added the "ChangeSet" text as a comment to the code. 3) ChangeLog is included (inline in the patch file). This new patch still needs someone to test it, since I don't have Vista 64. Regards, Cesar Strauss File Added: patch3.txt ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-06-08 03:29 Message: Logged In: YES user_id=1729144 Originator: NO I understand the filler bytes now and it seems a logical choice. However it simply does not work on all systems. I think it might work if cbReserved2 is adjusted to not include the filler bytes. On the MSDN forum someone mentioned: "The actual value may be different from (((sizeof_the_buffer - 4) / 5) + 2) but it should satisfy two conditions: 1. The calculated size should be greater than the actual size determined by cbReserved2; 2. The calculated size should be as close to cbReserved2 as possible otherwise memory will be waisted." The size / 5 formula does not satisfy condition 1. I think if cbReserved2 is changed to size-4 it might work and you don't have to worry about reading past the data structure. ---------------------------------------------------------------------- Comment By: Cesar Strauss (cstrauss) Date: 2007-06-07 20:40 Message: Logged In: YES user_id=1369729 Originator: NO jdrasch, I think I like your first patch (patch.txt) better, with the "size / 5" formula and the filler bytes. First, I wish to remind you all that the child-info data is being copied in chunks of 5 bytes, and that the formula is for determining the number of chunks to be copied. With the new formula, (size - 4) / 5 + 2, the number of bytes copied is always greater than the size of the data. This is a bit dangerous, I think, because you are reading past the end of the structure. You could have the bad luck of walking into a protected memory region, or copying sensitive data from another variable. On the other hand, I believe the original "size / 5" formula does work correctly. Sure, you can lose up to 4 bytes at the end of the structure when doing so. But this is what the filler bytes are for. Besides, this was the solution adopted by the Cygwin project. About this change: - STARTUPINFO si = {0, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, NULL}; + STARTUPINFO si; + memset (&si, 0, sizeof (si)); Is it part of the fix, or just a style change? Finally, I like the fact that the fix only is applied to Vista, which is an improvement over the original patch. Regards, Cesar ---------------------------------------------------------------------- Comment By: Keith Marshall (keithmarshall) Date: 2007-06-04 17:45 Message: Logged In: YES user_id=823908 Originator: NO Thanks for the patch. This needs to be accompanied by a properly formatted ChangeLog, before it can be committed. I'd suggest that most of the detail provided in the so called ChangeSet should be folded into the source files, in the form of comments. The ChangeLog should provide a *complete* itemised list of the changes made, formatted per GNU Coding Standards; it is not the place to explain the rationale for the changes. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 05:03 Message: Logged In: YES user_id=1723996 Originator: YES File Added: patch2.txt ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-05-30 04:50 Message: Logged In: YES user_id=1723996 Originator: YES I've also tried the suggested changes to the patch and they work fine on Vista 64 bit. > Also, I do not see the point of those 4 byte 'filler' members in the > different structures. Why are they included? I removed those as well as > they did not seem to cause or solve any problems. I originally used the above mentioned cygwin patch and "ported" it to MSYS, thats why they were introduced. > Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder > if anyone has had success with this patch. In any case I think it should be > corrected to the right formula. Well at least I had success with this patch :-) But the suggested formular works for me as well so I strongly suggest using it. ---------------------------------------------------------------------- Comment By: madwizard (madwizard) Date: 2007-05-28 07:56 Message: Logged In: YES user_id=1729144 Originator: NO Because the WinAVR package uses some tools (make, sh, rm, mkdir etc.) built with MSYS I ran into the same bug on Vista 64. Because WinAVR uses MSYS 1.0.8 I modified the patch slightly but the code is largely the same in those files. However, this patch did not fix the problem and I found out the formula used for the msv_cnt field is wrong. See this topic on the MSDN forums: http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=998172&SiteID=1 A quote: "A workaround (just as you have said) is writing (((sizeof_the_buffer - 4) / 5) + 2) as a DWORD value to the first 4 bytes of the buffer. Then the buffer will be copied without any alterations." I modified the patch to use (size - 4) / 5 + 2 and the problem was solved. Also, I do not see the point of those 4 byte 'filler' members in the different structures. Why are they included? I removed those as well as they did not seem to cause or solve any problems. Seeing as size / 5 produces a number less than (size - 4) / 5 + 2 I wonder if anyone has had success with this patch. In any case I think it should be corrected to the right formula. ---------------------------------------------------------------------- Comment By: jdrasch (jdrasch) Date: 2007-03-09 06:27 Message: Logged In: YES user_id=1723996 Originator: YES Hi, I finally set up an environment to build MSYS, so I did the patch myself. The originally described patch is not simply possible, as it is based on the actual cygwin code, therefore I did some research and changed the according sourcecode. I also fixed this so that only for Vista the needed changes in the child-info are applied. Tested on Win XP SP2, Vista 32bit and Vista 64bit. Patch is attached for Files: Index: src/winsup/cygwin/spawn.cc Index: src/winsup/cygwin/fork.cc Index: src/winsup/cygwin/winsup.h Index: src/winsup/cygwin/dcrt0.cc Index: src/winsup/cygwin/sigproc.cc Index: src/winsup/cygwin/child_info.h File Added: patch.txt ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=102435&aid=1674783&group_id=2435 |