From: Alexandra Hájková Date: Thu, 18 Apr 2024 09:21:49 +0000 (-0400) Subject: Improve file descriptor xml output, add fd and path elements X-Git-Tag: VALGRIND_3_23_0~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=09b5ad416c5ddf1375f269f6ea2253e90fa08f01;p=thirdparty%2Fvalgrind.git 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 element and (if known) a element. Add docs/internals/xml-output-protocol5.txt. https://bugs.kde.org/show_bug.cgi?id=328563 Co-Authored-By: Mark Wielaard --- diff --git a/NEWS b/NEWS index 66ea47486..954dae3fe 100644 --- 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 168008288..c802f8140 100644 --- 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)("\n"); VG_(printf_xml)("\n"); - VG_(printf_xml)("4\n"); + /* track-fds introduced some new elements. */ + if (VG_(clo_track_fds)) + VG_(printf_xml)("5\n"); + else + VG_(printf_xml)("4\n"); VG_(printf_xml)("%s\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 fa218ddbd..3c4846ade 100644 --- 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)(" FdBadClose\n"); struct BadCloseExtra *bce = (struct BadCloseExtra *) VG_(get_error_extra)(err); + if (xml) { + VG_(emit)(" %d\n", bce->fd); + if (bce->pathname) + VG_(emit)(" %s\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)(" FdNotClosed\n"); struct NotClosedExtra *nce = (struct NotClosedExtra *) VG_(get_error_extra)(err); + if (xml) { + VG_(emit)(" %d\n", nce->fd); + if (nce->pathname) + VG_(emit)(" %s\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 index 000000000..046f59408 --- /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. + +* INT, file descriptor number associated with error. + +* TEXT, (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 10aef9631..c4780a6f6 100644 --- 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 */ }