|
From: Tom H. <th...@cy...> - 2004-08-24 02:13:22
|
Nightly build on ginetta ( Red Hat 8.0 ) started at 2004-08-24 03:10:02 BST Checking out source tree ... done Configuring ... done Building ... done Running regression tests ... done Last 20 lines of log.verbose follow seg_override: valgrind ./seg_override sem: valgrind ./sem semlimit: valgrind ./semlimit sha1_test: valgrind ./sha1_test shortpush: valgrind ./shortpush shorts: valgrind ./shorts smc1: valgrind ./smc1 susphello: valgrind ./susphello syscall-restart1: valgrind ./syscall-restart1 syscall-restart2: valgrind ./syscall-restart2 system: valgrind ./system yield: valgrind ./yield -- Finished tests in none/tests ---------------------------------------- == 178 tests, 3 stderr failures, 0 stdout failures ================= helgrind/tests/race (stderr) helgrind/tests/race2 (stderr) memcheck/tests/writev (stderr) make: *** [regtest] Error 1 |
|
From: Eric E. <eri...@fr...> - 2004-08-24 20:05:55
|
I updated the patch mentioned before for cvs head. Many comments added, many cleanings (except for indentation, otherwise it would be confusing to read). (bigger since -u5). Patch is at http://eric.estievenart.free.fr/tmp/vg-head-valgui-24082004-1.diff Remarks welcome, tell me if anything is wrong; do you enforce strict code formatting I missed, or the "relax, provided it is readable" works ? Tell me if this has some chances for inclusion in 2.2.0. Passed all regtests which passed on a clean cvs tree. Note that I have a few testcases failing on both a clean and a patched tree, I can investigate them if you think it is necessary: $ make regtest [...] == 178 tests, 20 stderr failures, 1 stdout failure ================= corecheck/tests/fdleak_creat (stderr) corecheck/tests/fdleak_dup (stderr) corecheck/tests/fdleak_dup2 (stderr) corecheck/tests/fdleak_fcntl (stderr) corecheck/tests/fdleak_open (stderr) corecheck/tests/fdleak_pipe (stderr) corecheck/tests/fdleak_socketpair (stderr) helgrind/tests/allok (stderr) helgrind/tests/deadlock (stderr) helgrind/tests/inherit (stderr) helgrind/tests/race (stderr) helgrind/tests/race2 (stderr) helgrind/tests/readshared (stderr) memcheck/tests/badjump (stderr) memcheck/tests/buflen_check (stderr) memcheck/tests/execve (stderr) memcheck/tests/execve2 (stderr) memcheck/tests/fwrite (stderr) memcheck/tests/weirdioctl (stderr) memcheck/tests/writev (stderr) none/tests/exec-sigmask (stdout) (Debian unstable, kernel 2.6.7, glibc 2.3.2, gcc 3.3.4, Athlon UP) [For people wondering on Valgui, please wait a bit, initial announcement for pre-release will be made here first once I feel it is ready] Cheers -- Eric |
|
From: Nicholas N. <nj...@ca...> - 2004-08-26 09:39:09
|
On Thu, 26 Aug 2004, Eric Estievenart wrote: > Sorry if I bother you (again), did one of you have a look at > the given patch ? Was something wrong ? > > It is quite important for me, I have worked on that intensively > for the past months. I'm quite near from a pre-release, > and it would really help users if they can get this patch > standard in Valgui in the future. > > Yes, there are a lot more things to do in the future, but I felt it > was reasonable for a 2.2.0. It's unlikely to get into 2.2.0; we're in a semi-feature freeze for it. As for the other questions... I had a quick look, I didn't see anything objectionable with the code (eg. formatting). But I haven't had time to look at it closely. Tom said he didn't like the special --format=valgui option. I'm not too keen on it either. This is a tricky situation -- you've spent a lot of time on Valgui, and you're really keen for the patch to go in. I can understand that. On the other hand, you're asking for a special feature to be added to Valgrind that serves only one purpose. And it's a feature that involves a non-trivial amount of code to be added. And we don't have a whole bunch of people saying "please add this feature so I can use Valgui". And it requires someone to look at it closely, ahead of a zillion other proposed bug-fixes, feature requests, etc. So there's a balancing act here. I guess you have to convince at least one developer that the potential benefit is worth putting some time and effort into looking at and possibly committing this patch. I think you may have explained before why this change is necessary -- can you say again, or point out the previous message? AFAIK, Alleyoop doesn't need changes to Valgrind's output -- why is Valgui different? Could the patch be made smaller, less intrusive, or could it be made part of a more general change? (I know you say "--format=valgui" is general because other formats could be added in the future, but until that happens -- and there's no plans to add any -- it's effectively like a --valgui=yes option.) I don't want to put you off; we certainly do appreciate contributions. But contributors shouldn't expect their contributions to be used unconditionally; there has to be some persuasion. Having multiple people saying "I want this" is the best way -- a similar process works with bugs. Sometimes patience is necessary. I hope this doesn't seem unreasonable. N |
|
From: Eric E. <eri...@fr...> - 2004-08-26 18:54:56
|
Nicholas Nethercote wrote: > It's unlikely to get into 2.2.0; we're in a semi-feature freeze for it. I understand that. I tried to push it just before 2.1.2, but I was in a hurry and the patch was not so clean at that moment, and 2.1.2 was almost released. > As for the other questions... I had a quick look, I didn't see anything > objectionable with the code (eg. formatting). But I haven't had time to > look at it closely. Tom said he didn't like the special --format=valgui > option. I'm not too keen on it either. I'm not very proud of this option name. The problem is that I need one way to now to change a bit the output format, so when the option is not given the output is exactly like before. I asked for other proposals, since I didn't have any other solution looking better (otherwise I would have used it), but nobody seemed to care... > This is a tricky situation -- you've spent a lot of time on Valgui, and > you're really keen for the patch to go in. I can understand that. > > On the other hand, you're asking for a special feature to be added to > Valgrind that serves only one purpose. And it's a feature that involves > a non-trivial amount of code to be added. And we don't have a whole > bunch of people saying "please add this feature so I can use Valgui". > And it requires someone to look at it closely, ahead of a zillion other > proposed bug-fixes, feature requests, etc. Yes the only purpose is to make valgui usable out-of-the-box without asking users to download, patch and install a specific version of V. This would be a waste of time for many users. Most will be able to do it (documented at http://eric.estievenart.free.fr/valgui/doc/html/patches.html), but if it cames as-is for people using binary distributions, this mean less work for users, and less for package maintainers (because they will certainly be asked to include the patch). For now, valgui has not been released yet, so it's normal that nobody asks for the patch to be standard. All the people who had test previews said they would not use anything else than Valgui as an interface for Valgrind, but were bothered to have to patch V. So I assume this would help many users. I would not want to generate extra hassle on Valgrind development by releasing Valgui as-is, and saying in the doc "yes you need to patch, it is painful, I'm sorry, feel free to complain at vg-users/dev @ sf.net. It would be unfair, and I feel it is my responsibility, as-well as to provide a usable high quality initial release of Valgui, to help the future users getting started quickly. That's why I make the request here. That's also on what I worked on my side, making extensive documentation for Valgui, ensuring minimal requirements to compile and install (c++ compiler, make and qt 3.x is enough), fixing many bugs, disabling features not yet ok, and verifying that it works - with all qt versions : 3.0.0 to 3.3.3 are supported - with many g++ : 2.95.2/4 3.3 and 3.4 - with many Valgrind versions : currently patches for 2.0.0, 2.1.1, 2.1.2 and head I can tell you that maintaining the patches for the latter, along with the code which handles subtle differences is a bit a pain, and having a fresh start would relieve me. > So there's a balancing act here. I guess you have to convince at least > one developer that the potential benefit is worth putting some time and > effort into looking at and possibly committing this patch. I think you > may have explained before why this change is necessary -- can you say > again, or point out the previous message? AFAIK, Alleyoop doesn't need > changes to Valgrind's output -- why is Valgui different? Could the > patch be made smaller, less intrusive, or could it be made part of a > more general change? (I know you say "--format=valgui" is general > because other formats could be added in the future, but until that > happens -- and there's no plans to add any -- it's effectively like a > --valgui=yes option.) Ok, I will summarize here the current main benefits of Valgui: * Efficient user interface: - MDI. You can run programs many times, and compare the results - Integrated source editor, opens automatically when you double click on a snippet - You can edit the sources, make, and re-run under valgrind without switching to your IDE, shell or whatever very quickly. - See visually (by coloring) where the errors occur (e.g. red for your code, black for libZZZ, whatever you color you want for /usr/X11/lib/*, etc) - Never asks you to locate a source file, or just once (for all moved files) if you have moved the source tree - filter in one click all the elements you don't want to see (stack addresses, module names, functions, shorten file names, ...) - Permits to view process hierarchy when tracing children, and create separate resultsets for each process. Reports properly process termination and exit codes. * Easy configuration to switch a run with/without debug libraries * Module identification and automatic error classification/filtering based on direct/indirect/other This is one key feature of Valgui, no other product, even commercial, has (AFAIK) (see http://eric.estievenart.free.fr/valgui/doc/kde/errclassif.html for more details, it is a whole documentation chapter) * And a lot more things I have certainly forgotten There will be more in the future. Please feel free to have a look at the documentation. What I hope is that it will be widely used by many users. For the patch, indeed I stripped it a lot, and can't see how I could make it smaller without: - loosing vital informations for valgui - duplicating current code - risking regressions compared to current patch For the --fmt=valgui (yes, it is not --format, but I can change that), I am open to any proposal. For now I see it as a temporary solution, until the logging mechanism in Valgrind can be changed. It does not even need to be documented: Valgui decides internally to use it if Valgrind version seems to support it. For a more general change, I would be happy to do anything in the future, mostly concerning the logging interface, but I think it is completely out-of-plan for 2.2.0. There are also many other things I would be happy to work or help working on. > I don't want to put you off; we certainly do appreciate contributions. > But contributors shouldn't expect their contributions to be used > unconditionally; there has to be some persuasion. Having multiple > people saying "I want this" is the best way -- a similar process works > with bugs. > Sometimes patience is necessary. > > I hope this doesn't seem unreasonable. Indeed you are right, and it is the way things must work. I didn't want to bother you, and knew well that you had many other things to do. I logged a bug (http://bugs.kde.org/show_bug.cgi?id=85050), commented heavily on it, and proposed patches. It is clear that at that time I didn't want to unveil too many things until I felt I was ready for release, in order to continue testing, fixing and documenting without making people loose time by trying valgui if it did not work, reporting bugs I was aware and fixing, and asking questions on things I had not finished documenting. Indeed I have a small pool of testers (4-5); when we agree that the quality is reasonable, I'll make a pre-release announcement here asking for volunteers to test for more. Then, depending on the results, there may be iterations, and ultimately Valgui will be publicly released. If you want, I can ask how many people would want it, but collecting that info will either take me time, or induce extra noise in the MLs. Anyway, if I convinced someone, the latest patch is still at: http://eric.estievenart.free.fr/tmp/vg-head-valgui-24082004-1.diff ;-) Cheers -- Eric |
|
From: Nicholas N. <nj...@ca...> - 2004-08-27 16:03:47
|
On Thu, 26 Aug 2004, Eric Estievenart wrote: > For the patch, indeed I stripped it a lot, and can't see how I could > make it smaller without: > - loosing vital informations for valgui > - duplicating current code > - risking regressions compared to current patch Can you explain this more -- what information do you lose, what code is duplicated? I don't understand the bit about regressions. > For a more general change, I would be happy to do anything in the > future, mostly concerning the logging interface, but I think > it is completely out-of-plan for 2.2.0. What changes to the logging interface are you thinking of? N |
|
From: Eric E. <eri...@fr...> - 2004-08-27 16:48:02
|
Nicholas Nethercote wrote: > On Thu, 26 Aug 2004, Eric Estievenart wrote: > >> For the patch, indeed I stripped it a lot, and can't see how I could >> make it smaller without: >> - loosing vital informations for valgui >> - duplicating current code >> - risking regressions compared to current patch > > Can you explain this more -- what information do you lose, what code is > duplicated? I don't understand the bit about regressions. Basically, loosing: - module name when there are debug infos prevents from categorizing the modules. So error identification can't be done properly, or changes depending of the presence of debug libs or not when running - full source names will be a pain for the user - Small bit sof useful info, like pid relationship in parent (not in child), process exit status makes things weird to understand when tracing a process and its children The patch addresses these issues properly. One question was about making it less intrusive, but it can't be done without risking to loose these infos it generates, or duplicating the old code and tweaking it. For now there is no code duplication, and I'm fine with the patch. For the regressions, I meant that the patch, as it is, seems to work correctly, and changing it more could lead to regressions. > What changes to the logging interface are you thinking of? I'll go on that topic later, sorry I'm a bit in a hurry... BTW there is no urgency. Cheers -- Eric |
|
From: Eric E. <eri...@fr...> - 2004-08-29 21:54:42
|
Nicholas Nethercote wrote: >> What changes to the logging interface are you thinking of? So as promised, I took some time to collect all the bits I had in mind. Current problems are: * Problematic end of block detection Typically, a parser doesn't know that a block is finished until it has received the next block first line. Which may delay the displaying of last error. * Problematic process end detection Because the logging socket is (re)used as-is in forked processes, and is not always closed (maybe a small bug), knowing that a specific vg'ified has exited requires to log at least the exit status (which I did in my patch) * The streams are multiplexed after a fork, even when using --log-socket. Some internal reporting doesn't prefix with the pid, so knowing exactly where an untagged error line should go is tricky. * Parsing the generated stream requires many small kludges, which tends to make any parser relatively fragile, and may slowdown any improvement which may break existing parsers. Especially if --verbose is used. The future issues which may arise: * Localization * People may want to generate reports in different formats (xml, html, colored vt, sql, ...) * People may want valgrind not to read debug infos, and report addresses as-is, to be interpreted afterwards - Faster and less mem/disk usage when tracing very large sets of processes - Faster for people not processing the results (QA teams, ...) - Could help debugging deployed applications - Reports may be processed by proprietary vendors * Attaching user data (strings, stacks, ...) to runtime elements (mem blocks, fds, threads) to be reported in logs * Errors should have some kind of severity attribute, to help users (and parsers if they don't know the error) * Users may want to have a shorter description of the errors, e.g. I translate an "invalid read" with freeing stack into a 'FMR' (Free Memory Read) or an "invalid free" with freeing stack into a 'DMF' (Double Memory Free), etc. As for the communication stream (only in console for now, with as-gdb-attach), we may want (when tracing children) to ask the user if a particular forked/exec'd process must be traced or not, optionally with different vg options from its parent. Here were my first thoughts I have certainly missed things. Comments welcome ;-) Cheers -- Eric |