Bug 328563 - make track-fds support xml output
Summary: make track-fds support xml output
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.15 SVN
Platform: unspecified All
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-12-09 09:00 UTC by ewirch
Modified: 2024-04-19 00:12 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Patch - Adds support for xml output of --track-fds (6.61 KB, application/mbox)
2020-04-11 00:38 UTC, Evan Hunter
Details
Patch - Adds support for xml output of --track-fds (6.86 KB, text/plain)
2023-11-22 11:19 UTC, Mark Wielaard
Details
patch (18.68 KB, patch)
2024-04-15 12:01 UTC, Alexandra Hajkova
Details
patch (9.33 KB, patch)
2024-04-18 15:21 UTC, Alexandra Hajkova
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ewirch 2013-12-09 09:00:40 UTC
When using --track-fds together with --xml=yes, the file descriptor information is printed in standard text format.

When using valgrind in an automated test environment, it'd make it easier to parse the file descriptor info.

Reproducible: Always
Comment 1 Hideaki Kimura 2016-09-02 22:13:49 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.
Comment 2 Evan Hunter 2020-04-11 00:33:29 UTC
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.
Comment 3 Evan Hunter 2020-04-11 00:38:14 UTC
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()
Comment 4 Mark Wielaard 2023-11-22 11:19:36 UTC
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?
Comment 5 Alexandra Hajkova 2024-04-15 12:01:05 UTC
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.
Comment 6 Alexandra Hajkova 2024-04-18 15:21:44 UTC
Created attachment 168646 [details]
patch

follow up for the previous patch, Improve file descriptor xml output, add fd and path elements
Comment 7 Mark Wielaard 2024-04-19 00:12:05 UTC
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>