| Summary: | make track-fds support xml output | ||
|---|---|---|---|
| Product: | [Developer tools] valgrind | Reporter: | ewirch <wirch.eduard> |
| Component: | general | Assignee: | Julian Seward <jseward> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | ahajkova, evan, hideaki.kimura, mark |
| Priority: | NOR | ||
| Version First Reported In: | 3.15 SVN | ||
| Target Milestone: | --- | ||
| Platform: | unspecified | ||
| OS: | All | ||
| Latest Commit: | Version Fixed/Implemented In: | ||
| Sentry Crash Report: | |||
| Attachments: |
Patch - Adds support for xml output of --track-fds
Patch - Adds support for xml output of --track-fds patch patch |
||
|
Description
ewirch
2013-12-09 09:00:40 UTC
I'm also hitting this, but in a bit different form from the original comment probably due to version difference (mine is 3.11.0).
When I enable track-fds and xml output, valgrind _does_ output open-at-exit file descriptors information in XML. However, the output is far from ideal.
For example,
// test.c
#include <fcntl.h>
int main(void) {
open("bluh", O_CREAT, 0);
return 0;
}
Then
gcc test.c; rm -f bluh; valgrind --xml=yes --xml-file=out.xml --track-fds=yes --leak-check=full --error-exitcode=1 ./a.out
a.out emits no error code:
> echo $?
0
The out.xml would look like:
<?xml version="1.0"?>
<valgrindoutput>
<protocolversion>4</protocolversion>
<protocoltool>memcheck</protocoltool>
<preamble>...snip...</preamble>
<pid>20082</pid>
<ppid>25595</ppid>
<tool>memcheck</tool>
<args>
<vargv>
<exe>/usr/bin/valgrind</exe>
<arg>--xml=yes</arg>
<arg>--xml-file=out.xml</arg>
<arg>--track-fds=yes</arg>
<arg>--leak-check=full</arg>
<arg>--error-exitcode=1</arg>
</vargv>
<argv>
<exe>./a.out</exe>
</argv>
</args>
<status>
<state>RUNNING</state>
<time>00:00:00:00.056 </time>
</status>
<status>
<state>FINISHED</state>
<time>00:00:00:00.242 </time>
</status>
<stack>
<frame>
<ip>0x4F27490</ip>
<obj>/usr/lib64/libc-2.22.so</obj>
<fn>__open_nocancel</fn>
</frame>
<frame>
<ip>0x400552</ip>
<obj>/home/kimurhid/a.out</obj>
<fn>main</fn>
</frame>
</stack>
<errorcounts></errorcounts>
<suppcounts></suppcounts>
</valgrindoutput>
This is it. The <stack> element shows where the open-at-exit FD was made, which is useful but the way the xml shows it is very user-unfriendly. It doesn't even say what this <stack> element is about!
In my case, this is also causing an issue to integrate with other tools like Continuous Integration (Jenkins) and CTest because the output/exit-code looks normal even when there are leaked FDs. Jenkins/CTest just report that there were no errors.. even when I have leaked FDs!
In sum, I suggest the following:
1. In XML, make the leaked-FD output as beefy as usual memory-leak errors, ideally follow the same format.
2. Add an option to emit error exit-code on observing leaked-FDs, again just like usual memory-leak errors.
Hi Valgrind architects,
I have implemented a fix for this feature request, but have a question:
Where should I put the tests for this feature?
Other tests for --track-fds are in /none/tests/ however
this will not work for xml tests, since Nulgrind does not
have VG_(needs).output_xml, hence (clo_xml) will get unset.
Would you prefer:
a) Move track-fds tests to another tool test
directory which does support xml?
b) Set VG(needs).output_xml in Nulgrind?
c) Add a new dunmy tool that VG_(needs) everything
but just provides minimal stubs.
Created attachment 127438 [details]
Patch - Adds support for xml output of --track-fds
Patch enables xml output for --track-fds
modifies show_open_fds() and getsockdetails()
Created attachment 163356 [details]
Patch - Adds support for xml output of --track-fds
This is a rebase of the patch to current (VALGRIND_3_22_0-61-gb748cc9e1) git.
It seems to work, but I haven't fully reviewed it yet.
It would also need a bit of documentation about the proposed xml structure output.
And indeed some tests would be nice. It is a pity the none tool doesn't do xml output.
Maybe it should? or should we just stick some new tests under the memcheck tool?
Created attachment 168547 [details] patch This xml support is based on the patch in 362680: https://bugs.kde.org/show_bug.cgi?id=362680 and that it doesn't include the extras in Evan's patch yet. Created attachment 168646 [details]
patch
follow up for the previous patch, Improve file descriptor xml output, add fd and path elements
commit 09b5ad416c5ddf1375f269f6ea2253e90fa08f01 Author: Alexandra Hájková <ahajkova@redhat.com> Date: Thu Apr 18 05:21:49 2024 -0400 Improve file descriptor xml output, add fd and path elements Add needs_xml_output to none tool so it can also output xml when --track-fds enabled. Use xml protocolversion 5 when clo_track_fds is enabled Split OpenFD pathname and description. Add description when a file descriptor is closed so it can be used in a future error. On error print <fd> element and (if known) a <path> element. Add docs/internals/xml-output-protocol5.txt. https://bugs.kde.org/show_bug.cgi?id=328563 Co-Authored-By: Mark Wielaard <mark@klomp.org> |