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>

diff --git a/NEWS b/NEWS
index 66ea47486f9bbf8be7e5dc862195b2883f528654..954dae3fe8590ee2cdeeb4cbd311694fc353ed10 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -40,6 +40,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 
 283429  ARM leak checking needs CLEAR_CALLER_SAVED_REGS
 281059  Cannot connect to Oracle using valgrind
+328563  make track-fds support xml output
 362680  --error-exitcode not honored when file descriptor leaks are found
 369723  __builtin_longjmp not supported in clang/llvm on Android arm64 target
 390269  unhandled amd64-darwin syscall: unix:464 (openat_nocancel)

diff --git a/coregrind/m_libcprint.c b/coregrind/m_libcprint.c
index 168008288550b7199111189c564ce629ea6f313f..c802f814038b8da7f94ab919b8f25ca323664048 100644 (file)
--- a/coregrind/m_libcprint.c
+++ b/coregrind/m_libcprint.c
@@ -151,7 +151,11 @@ void VG_(print_preamble)(Bool logging_to_fd)
       VG_(printf_xml)("\n");
       VG_(printf_xml)("<valgrindoutput>\n");
       VG_(printf_xml)("\n");
-      VG_(printf_xml)("<protocolversion>4</protocolversion>\n");
+      /* track-fds introduced some new elements.  */
+      if (VG_(clo_track_fds))
+         VG_(printf_xml)("<protocolversion>5</protocolversion>\n");
+      else
+         VG_(printf_xml)("<protocolversion>4</protocolversion>\n");
       VG_(printf_xml)("<protocoltool>%s</protocoltool>\n", VG_(clo_toolname));
       VG_(printf_xml)("\n");
    }

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index fa218ddbde3e5236ed50476e9853987e0a4027ea..3c4846adefcd6008d934096e27949c6b36fef1a0 100644 (file)
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -540,6 +540,7 @@ typedef struct OpenFd
 {
    Int fd;                        /* The file descriptor */
    HChar *pathname;               /* NULL if not a regular file or unknown */
+   HChar *description;            /* Description saved before close */
    ExeContext *where;             /* NULL if inherited from parent */
    ExeContext *where_closed;      /* record the last close of fd */
    Bool fd_closed;
@@ -584,13 +585,16 @@ void ML_(record_fd_close_range)(ThreadId tid, Int from_fd)
 struct BadCloseExtra {
    Int fd;                        /* The file descriptor */
    HChar *pathname;               /* NULL if not a regular file or unknown */
+   HChar *description;            /* Description of the file descriptor
+                                     might include the pathname */
    ExeContext *where_closed;      /* record the last close of fd */
    ExeContext *where_opened;      /* recordwhere the fd  was opened */
 };
 
 struct NotClosedExtra {
    Int fd;
-   HChar description[256];
+   HChar *pathname;
+   HChar *description;
 };
 
 /* Note the fact that a file descriptor was just closed. */
@@ -607,6 +611,7 @@ void ML_(record_fd_close)(ThreadId tid, Int fd)
            struct BadCloseExtra bce;
            bce.fd = i->fd;
            bce.pathname = i->pathname;
+           bce.description = i->description;
            bce.where_opened = i->where;
            bce.where_closed = i->where_closed;
            VG_(maybe_record_error)(tid, FdBadClose, 0,
@@ -629,11 +634,14 @@ void ML_(record_fd_close)(ThreadId tid, Int fd)
                                  &val, &len) == -1) {
                 HChar *pathname = VG_(malloc)("vg.record_fd_close.fd", 30);
                 VG_(snprintf)(pathname, 30, "file descriptor %d", i->fd);
-                i->pathname = pathname;
+                i->description = pathname;
              } else {
                 HChar *name = VG_(malloc)("vg.record_fd_close.sock", 256);
-                i->pathname = getsockdetails(i->fd, 256, name);
+                i->description = getsockdetails(i->fd, 256, name);
              }
+          } else {
+             i->description = VG_(strdup)("vg.record_fd_close.path",
+                                          i->pathname);
           }
           fd_count--;
         }
@@ -666,6 +674,10 @@ void ML_(record_fd_open_with_given_name)(ThreadId tid, Int fd,
             VG_(free)(i->pathname);
             i->pathname = NULL;
          }
+         if (i->description) {
+            VG_(free)(i->description);
+            i->description = NULL;
+         }
          if (i->fd_closed) /* now we will open it. */
             fd_count++;
          break;
@@ -686,6 +698,7 @@ void ML_(record_fd_open_with_given_name)(ThreadId tid, Int fd,
 
    i->fd = fd;
    i->pathname = VG_(strdup)("syswrap.rfdowgn.2", pathname);
+   i->description = NULL; /* Only set on close.  */
    i->where = (tid == -1) ? NULL : VG_(record_ExeContext)(tid, 0/*first_ip_delta*/);
    i->fd_closed = False;
    i->where_closed = NULL;
@@ -940,8 +953,12 @@ void VG_(show_open_fds) (const HChar* when)
           continue;
 
       struct NotClosedExtra nce;
+      /* The file descriptor was not yet closed, so the description field was
+         also not yet set. Set it now as if the file descriptor was closed at
+         this point.  */
+      i->description = VG_(malloc)("vg.notclosedextra.descriptor", 256);
       if (i->pathname) {
-         VG_(snprintf) (nce.description, 256, "file descriptor %d: %s",
+         VG_(snprintf) (i->description, 256, "file descriptor %d: %s",
                         i->fd, i->pathname);
       } else {
          Int val;
@@ -949,13 +966,17 @@ void VG_(show_open_fds) (const HChar* when)
 
          if (VG_(getsockopt)(i->fd, VKI_SOL_SOCKET, VKI_SO_TYPE, &val, &len)
              == -1) {
-            VG_(sprintf)(nce.description, "file descriptor %d:", i->fd);
+            /* Don't want the : at the end in xml */
+            UChar *colon = VG_(clo_xml) ? "" : ":";
+            VG_(sprintf)(i->description, "file descriptor %d%s", i->fd, colon);
          } else {
-            getsockdetails(i->fd, 256, nce.description);
+            getsockdetails(i->fd, 256, i->description);
          }
       }
 
       nce.fd = i->fd;
+      nce.pathname = i->pathname;
+      nce.description = i->description;
       VG_(unique_error) (1 /* Fake ThreadId */,
                          FdNotClosed,
                          0, /* Addr */
@@ -1106,8 +1127,13 @@ void fd_pp_Error (const Error *err)
       if (xml) VG_(emit)("  <kind>FdBadClose</kind>\n");
       struct BadCloseExtra *bce = (struct BadCloseExtra *)
          VG_(get_error_extra)(err);
+      if (xml) {
+         VG_(emit)("  <fd>%d</fd>\n", bce->fd);
+         if (bce->pathname)
+            VG_(emit)("  <path>%s</path>\n", bce->pathname);
+      }
       VG_(emit)("%sFile descriptor %d: %s is already closed%s\n",
-                whatpre, bce->fd, bce->pathname, whatpost);
+                whatpre, bce->fd, bce->description, whatpost);
       VG_(pp_ExeContext)( VG_(get_error_where)(err) );
       VG_(emit)("%sPreviously closed%s\n", auxpre, auxpost);
       VG_(pp_ExeContext)(bce->where_closed);
@@ -1117,6 +1143,11 @@ void fd_pp_Error (const Error *err)
       if (xml) VG_(emit)("  <kind>FdNotClosed</kind>\n");
       struct NotClosedExtra *nce = (struct NotClosedExtra *)
          VG_(get_error_extra)(err);
+      if (xml) {
+         VG_(emit)("  <fd>%d</fd>\n", nce->fd);
+         if (nce->pathname)
+            VG_(emit)("  <path>%s</path>\n", nce->pathname);
+      }
       VG_(emit)("%sOpen %s%s\n", whatpre, nce->description, whatpost);
       if (where != NULL) {
          VG_(pp_ExeContext)(where);

diff --git a/docs/internals/xml-output-protocol5.txt b/docs/internals/xml-output-protocol5.txt
new file mode 100644 (file)
index 0000000..046f594
--- /dev/null
+++ b/docs/internals/xml-output-protocol5.txt
@@ -0,0 +1,39 @@
+
+====================================================================
+
+18 April 2024
+
+Protocol 5 is identical to protocol 4 but adds core-error elements for
+tool agnostic output. Currently only set when using the --track-fds
+option (otherwise the protocolversion is set to 4).
+
+====================================================================
+
+core-errors can appear where TOOLSPECIFIC definitions can occur.  This
+is always an ERROR which follows the generic ERROR definition with the
+following extra details.
+
+ERROR details for file-tracking core errors
+-------------------------------------------
+
+The KIND is a new unique string described below.
+There can be an fd and (option) PATH element.
+
+* <fd>INT</fd>, file descriptor number associated with error.
+
+* <path>TEXT</path>, (optional) file path associated with fd (if known).
+
+KIND for file-tracking core errors
+----------------------------------
+
+This is a small enumeration indicating roughly the nature of an error.
+The possible values are:
+
+   FdBadClose
+
+      The file descriptor was already closed previously
+
+   FdNotClosed
+
+      On exit the file descriptor is still open
+

diff --git a/none/nl_main.c b/none/nl_main.c
index 10aef96314111ff3d885d38ea4674b2d30f17e73..c4780a6f67a274e428acfaee002511680cc8f01f 100644 (file)
--- a/none/nl_main.c
+++ b/none/nl_main.c
@@ -62,6 +62,7 @@ static void nl_pre_clo_init(void)
    VG_(basic_tool_funcs)        (nl_post_clo_init,
                                  nl_instrument,
                                  nl_fini);
+   VG_(needs_xml_output)        ();
 
    /* No needs, no core events to track */
 }