Re: valgrind for WINE
At 19:45 01/04/03 -0600, Eric Pouech <pouech-eric(a)wanadoo.fr> wrote:
Adam Gundy wrote:
a new version of the valgrind patch has been uploaded to sourceforge - the only change is a small fix to the signal handling which should prevent "signal handler frame" uninitialized errors.
http://sourceforge.net/tracker/index.php?func=detail&aid=710006&group_id=462...
comments? bug reports? success stories?
did test it yet.
As a global note, I think it would be appreciated breaking the patch in smaller bits... it makes it easier to track/follow what happens. For example, I don't see any reason for including the msvcrtd patches into this patch
the reason msvcrtd is there is that you need to use it for valgrind to do any checking of the heap - an instrumented malloc (see the changes to ntdll/heap.c) is required to tell valgrind that freshly allocated blocks are uninitialized, and the freed blocks are now invalid. If you build a debug executable using MSVC you have to use MSVCRTD.DLL, hence the new spec file for msvcrtd.dll.so
I think that Alexandre is currently integrating (bit per bit) your changes into wine. look for the patch about zeroing the request zones (which is another implementation of your structure packing), as well as local availability of the thread PID
haven't seen these yet
on a more minor side of things, I would keep the VALGRIND prefix in heap.c
not sure what you mean here, but the heap.c stuff appears to have been committed (slightly rewritten by the look of it...). To be of ANY use you need the MSVCRTD stuff as well though!
last point, using the .vg extension to the wine script may not be optimal. it won't work (as it is written) for winelib programs (which got started also thru the wine script) one solution would be (to follow your idea) to link (for a winelib app called foo) foo.vg to script wine (and change your rule something like *.vg|*/*.vg). another option would be to embed this support as an option to the wine script (like --valgrind)
maybe, this was just a hack to make it easier to use "in tree". not sure people would want a "--valgrind" switch in a (presumably) production winelib piece of software - it would show the world all your bugs ;-)
how are your patches doing on the "other side" (I mean valgrind's) ?
just starting to make progress into CVS commits - there appear to be several other patches which overlap with mine, so there will probably be some ordering and refactoring required... Seeya, Adam -- Real Programmers don't comment their code. If it was hard to write, it should be hard to read, and even harder to modify. These are all my own opinions.
On April 2, 2003 08:55 am, Adam Gundy wrote:
the reason msvcrtd is there is that you need to use it for valgrind to do any checking of the heap - an instrumented malloc (see the changes to ntdll/heap.c) is required to tell valgrind that freshly allocated blocks are uninitialized, and the freed blocks are now invalid.
What Eric meant is that the msvcrtd can go in as a separate patch. Yes, it is required by VALGRIND, but it's useful on it's own as well, so just submit that one individually.
maybe, this was just a hack to make it easier to use "in tree". not sure people would want a "--valgrind" switch in a (presumably) production winelib piece of software - it would show the world all your bugs ;-)
We can disable that switch completely if we don't configure with VALGRIND. No problem there. -- Dimi.
the reason msvcrtd is there is that you need to use it for valgrind to do any checking of the heap - an instrumented malloc (see the changes to ntdll/heap.c) is required to tell valgrind that freshly allocated blocks are uninitialized, and the freed blocks are now invalid. IMO you need msvcrd for running under wine a windows program compiled with both DEBUG turned on, and linking to MSVCRT you don't need msvcrtd at all to run, say, a winelib program with valgrind enabled (I assume this comes from the fact you did most of the testing with a wine running a windows app which has been compiled with the DEBUG flag on) anyway, that's pure semantic. I doesn't change the real benefit behind: being able to run wine under valgrind !!
on a more minor side of things, I would keep the VALGRIND prefix in heap.c not sure what you mean here, but the heap.c stuff appears to have been committed (slightly rewritten by the look of it...). To be of ANY use you need the MSVCRTD stuff as well though! basically, partly what Alexandre did to your patch => keeping the code (and macros) which are valgrind related being prefixed with VALGRIND
last point, using the .vg extension to the wine script may not be optimal. it won't work (as it is written) for winelib programs (which got started also thru the wine script) one solution would be (to follow your idea) to link (for a winelib app called foo) foo.vg to script wine (and change your rule something like *.vg|*/*.vg). another option would be to embed this support as an option to the wine script (like --valgrind)
maybe, this was just a hack to make it easier to use "in tree". not sure people would want a "--valgrind" switch in a (presumably) production winelib piece of software - it would show the world all your bugs ;-) sure!!! that's why putting it in a file external to winewrapper might be a good solution too (lile the .winewrapper)
A+ -- Eric Pouech
a small contribution (for valgrind) to get current wine running Adam, I think it would be better if you get all those changes together for inclusion in vg, if you don't mind A+ -- Eric Pouech Index: vg_syscalls.c =================================================================== RCS file: /cvsroot/valgrind/valgrind/coregrind/vg_syscalls.c,v retrieving revision 1.25 diff -u -r1.25 vg_syscalls.c --- vg_syscalls.c 24 Feb 2003 21:55:34 -0000 1.25 +++ vg_syscalls.c 2 Apr 2003 19:39:40 -0000 @@ -647,6 +661,14 @@ break; # endif +# if defined(__NR_gettid) + case __NR_gettid: /* syscall 224 */ + /* pid_t gettid(void); */ + MAYBE_PRINTF("gettid ()\n"); + KERNEL_DO_SYSCALL(tid,res); + break; +# endif + # if defined(__NR_setxattr) case __NR_setxattr: /* syscall 226 */ /* int setxattr (const char *path, const char *name,
At 20:34 02/04/03 +0200, Eric Pouech wrote:
the reason msvcrtd is there is that you need to use it for valgrind to do any checking of the heap - an instrumented malloc (see the changes to ntdll/heap.c) is required to tell valgrind that freshly allocated blocks are uninitialized, and the freed blocks are now invalid. IMO you need msvcrd for running under wine a windows program compiled with both DEBUG turned on, and linking to MSVCRT you don't need msvcrtd at all to run, say, a winelib program with valgrind enabled (I assume this comes from the fact you did most of the testing with a wine running a windows app which has been compiled with the DEBUG flag on) anyway, that's pure semantic. I doesn't change the real benefit behind: being able to run wine under valgrind !!
I've never got very far with winelib - as soon as any code involving OLE gets involved, things won't compile. In fact, getting stuff without OLE to compile is frequently painful and slow :-(
last point, using the .vg extension to the wine script may not be optimal. it won't work (as it is written) for winelib programs (which got started also thru the wine script) one solution would be (to follow your idea) to link (for a winelib app called foo) foo.vg to script wine (and change your rule something like *.vg|*/*.vg). another option would be to embed this support as an option to the wine script (like --valgrind)
maybe, this was just a hack to make it easier to use "in tree". not sure people would want a "--valgrind" switch in a (presumably) production winelib piece of software - it would show the world all your bugs ;-) sure!!! that's why putting it in a file external to winewrapper might be a good solution too (lile the .winewrapper)
since I haven't been using winelib (although we did try unsuccessfully) I'll leave that up to you... Seeya, Adam -- Real Programmers don't comment their code. If it was hard to write, it should be hard to read, and even harder to modify. These are all my own opinions.
On April 3, 2003 05:45 am, Adam Gundy wrote:
I've never got very far with winelib - as soon as any code involving OLE gets involved, things won't compile. In fact, getting stuff without OLE to compile is frequently painful and slow :-(
I suggest you look into winegcc if you want to use winelib. It makes it a lot easier. -- Dimi.
At 06:57 03/04/03 -0500, Dimitrie O. Paun wrote:
On April 3, 2003 05:45 am, Adam Gundy wrote:
I've never got very far with winelib - as soon as any code involving OLE gets involved, things won't compile. In fact, getting stuff without OLE to compile is frequently painful and slow :-(
I suggest you look into winegcc if you want to use winelib. It makes it a lot easier.
I used winemaker to create make/configure files etc - after a couple of days of hacking the source I managed to get (most of) MFC to build using this. we need MFC0LE as well - most of that *wouldn't* build... another couple of days trying to build one of our libraries (of about 40) convinced me that it wasn't going to be worth the effort. I see that winemaker didn't use winegcc as the compiler - does it really make that much difference? I had problems with nameless unions and structures, missing definitions in the wine headers, several OLE interfaces being missing, (our) OLE interface definition which defined sub-interfaces (don't know what the real name for these is) refusing to compile. Most of the issues were either: a) missing definitions or functionality in the wine(lib) headers - stuff which works under WINE mostly. b) compiler problems where either Microsoft (in MFCxx) or we had used extensions to C++. Seeya, Adam -- Real Programmers don't comment their code. If it was hard to write, it should be hard to read, and even harder to modify. These are all my own opinions.
On April 3, 2003 08:45 am, Adam Gundy wrote:
another couple of days trying to build one of our libraries (of about 40) convinced me that it wasn't going to be worth the effort.
Unfortunately, winegcc is missing suppport for -shared, so it's not yet useful for building DLLs. I hope to fix that in the future. The path that I always recommend is first build your app with mingw on Windows, then simply change gcc to winegcc, and windres to wrc in the Makefiles and you should be all set. In theory :)
I see that winemaker didn't use winegcc as the compiler - does it really make that much difference?
No it doesn't make that much of a difference, it's just more convenient. winemaker does notuse winegcc yet, due to lack of time on our part to update it.
I had problems with nameless unions and structures, missing definitions in the wine headers, several OLE interfaces being missing, (our) OLE interface definition which defined sub-interfaces (don't know what the real name for these is) refusing to compile.
Most of the issues were either:
a) missing definitions or functionality in the wine(lib) headers - stuff which works under WINE mostly.
Nothing will help with that. Patches for adding those missing bits to our headers are highly appreciated, and one of the reasons we encourage people to compile their apps under Winelib.
b) compiler problems where either Microsoft (in MFCxx) or we had used extensions to C++.
Yes, problematic -- you have to use only stuff that gcc understands. -- Dimi.
You included the following suppression within wine.supp into your valgrind patch, did you do debug or must a bug be open on bugzuilla ? # The CTLCOLOR message handler doesn't fill in pResult - BUG! { CWnd::ReflectChildNotify/CWnd::OnChildNotify(Cond) Memcheck:Cond fun:CWnd::ReflectChildNotify fun:CWnd::OnChildNotify } ===== Sylvain Petreolle (spetreolle at users dot sourceforge dot net) ICQ #170597259 No more War ! "What if tomorrow the War could be over ?" Morpheus, in "Reloaded". For the Law of Oil and Fire, Im an European that lives in France. For all my Brothers and friends, Im a human living on Earth. ___________________________________________________________ Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français ! Yahoo! Mail : http://fr.mail.yahoo.com
At 16:47 03/04/03 +0200, Sylvain Petreolle wrote:
You included the following suppression within wine.supp into your valgrind patch, did you do debug or must a bug be open on bugzuilla ?
# The CTLCOLOR message handler doesn't fill in pResult - BUG! { CWnd::ReflectChildNotify/CWnd::OnChildNotify(Cond) Memcheck:Cond fun:CWnd::ReflectChildNotify fun:CWnd::OnChildNotify }
this is a bug in Microsoft's MFC library - I doubt we will get them to fix it! Seeya, Adam -- Real Programmers don't comment their code. If it was hard to write, it should be hard to read, and even harder to modify. These are all my own opinions.
With your new patch applied to a cvs version of valgrind, valgrind fails to compile. I get : make[1]: Entre dans le répertoire `/home/valgrind/coregrind' source='vg_to_ucode.c' object='vg_to_ucode.o' libtool=no \ depfile='.deps/vg_to_ucode.Po' tmpdepfile='.deps/vg_to_ucode.TPo' \ depmode=gcc3 /bin/sh ../depcomp \ gcc -DHAVE_CONFIG_H -I. -I. -I.. -I./demangle -I../include -DVG_LIBDIR="\"/usr/local/lib"\" -Winline -Wall -Wshadow -O -fomit-frame-pointer -mpreferred-stack-boundary=2 -g -c `test -f 'vg_to_ucode.c' || echo './'`vg_to_ucode.c vg_to_ucode.c: Dans la fonction « disInstr »: vg_to_ucode.c:4391: valeur de « case » double vg_to_ucode.c:4372: précédemment utilisé ici vg_to_ucode.c:4410: valeur de « case » double vg_to_ucode.c:4391: précédemment utilisé ici make[1]: *** [vg_to_ucode.o] Erreur 1 make[1]: Quitte le répertoire `/home/valgrind/coregrind' make: *** [all-recursive] Erreur 1 ===== Sylvain Petreolle (spetreolle at users dot sourceforge dot net) ICQ #170597259 No more War ! "What if tomorrow the War could be over ?" Morpheus, in "Reloaded". For the Law of Oil and Fire, Im an European that lives in France. For all my Brothers and friends, Im a human living on Earth. ___________________________________________________________ Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français ! Yahoo! Mail : http://fr.mail.yahoo.com
At 16:53 03/04/03 +0200, Sylvain Petreolle wrote:
With your new patch applied to a cvs version of valgrind, valgrind fails to compile. I get :
make[1]: Entre dans le répertoire `/home/valgrind/coregrind' source='vg_to_ucode.c' object='vg_to_ucode.o' libtool=no \ depfile='.deps/vg_to_ucode.Po' tmpdepfile='.deps/vg_to_ucode.TPo' \ depmode=gcc3 /bin/sh ../depcomp \ gcc -DHAVE_CONFIG_H -I. -I. -I.. -I./demangle -I../include -DVG_LIBDIR="\"/usr/local/lib"\" -Winline -Wall -Wshadow -O -fomit-frame-pointer -mpreferred-stack-boundary=2 -g -c `test -f 'vg_to_ucode.c' || echo './'`vg_to_ucode.c vg_to_ucode.c: Dans la fonction « disInstr »: vg_to_ucode.c:4391: valeur de « case » double vg_to_ucode.c:4372: précédemment utilisé ici vg_to_ucode.c:4410: valeur de « case » double vg_to_ucode.c:4391: précédemment utilisé ici make[1]: *** [vg_to_ucode.o] Erreur 1 make[1]: Quitte le répertoire `/home/valgrind/coregrind' make: *** [all-recursive] Erreur 1
a (small) part of the valgrind patch has been committed to CVS - I have just updated the patch on sourceforge to remove this bit. New patch includes Eric Pouech's patch for the gettid() system call: http://sourceforge.net/tracker/index.php?func=detail&aid=710006&group_id=462... Seeya, Adam -- Real Programmers don't comment their code. If it was hard to write, it should be hard to read, and even harder to modify. These are all my own opinions.
Thanks, downloaded it. I will wait to recompile it since I removed the file to have a fresh copy and cvs.sf.net seems to be down atm (some maintenance perhaps ?)
a (small) part of the valgrind patch has been committed to CVS - I have just updated the patch on sourceforge to remove this bit. New patch includes Eric Pouech's patch for the gettid() system call:
===== Sylvain Petreolle (spetreolle at users dot sourceforge dot net) ICQ #170597259 No more War ! "What if tomorrow the War could be over ?" Morpheus, in "Reloaded". For the Law of Oil and Fire, Im an European that lives in France. For all my Brothers and friends, Im a human living on Earth. ___________________________________________________________ Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français ! Yahoo! Mail : http://fr.mail.yahoo.com
participants (4)
-
Adam Gundy -
Dimitrie O. Paun -
Eric Pouech -
Sylvain Petreolle