From 2942c04c485076d84a2df8ee6bb901bb0f014c51 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alexandra=20H=C3=A1jkov=C3=A1?= Date: Fri, 20 Jun 2025 03:21:41 -0400 Subject: [PATCH] Add "bad" option for --track-fds. When --track-fds=bad is specified, do not warn about leaked file descriptors and only warn about file decriptors which was not opened or already closed. Update the documentation in docs/xml/manual-core.xml. Add none/tests/track_bad test to test the new option. Adjust none/tests/cmdline1 and none/tests/cmdline2 expected outputs. https://bugs.kde.org/show_bug.cgi?id=493434 --- .gitignore | 1 + NEWS | 1 + coregrind/m_main.c | 12 ++++++-- docs/xml/manual-core.xml | 35 ++++++++++++++++-------- none/tests/Makefile.am | 6 ++-- none/tests/cmdline1.stdout.exp | 8 ++++-- none/tests/cmdline1.stdout.exp-non-linux | 8 ++++-- none/tests/cmdline2.stdout.exp | 8 ++++-- none/tests/cmdline2.stdout.exp-non-linux | 2 +- none/tests/track_bad.c | 34 +++++++++++++++++++++++ none/tests/track_bad.stderr.exp | 28 +++++++++++++++++++ none/tests/track_bad.vgtest | 3 ++ 12 files changed, 123 insertions(+), 23 deletions(-) create mode 100644 none/tests/track_bad.c create mode 100644 none/tests/track_bad.stderr.exp create mode 100644 none/tests/track_bad.vgtest diff --git a/.gitignore b/.gitignore index 8cabb96df..32cedb0d7 100644 --- a/.gitignore +++ b/.gitignore @@ -1676,6 +1676,7 @@ /none/tests/track-fds-exec-children /none/tests/track_new /none/tests/track_std +/none/tests/track_bad /none/tests/unit_debuglog /none/tests/use_after_close /none/tests/valgrind_cpp_test diff --git a/NEWS b/NEWS index 0e0118265..0d9cc798e 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,7 @@ than mailing the developers (or mailing lists) directly -- bugs that are not entered into bugzilla tend to get forgotten about or ignored. 338803 Handling of dwz debug alt files or cross-CU is broken +493434 Add --track-fds=bad mode (no "leak" tracking) 503098 Incorrect NAN-boxing for float registers in RISC-V 503641 close_range syscalls started failing with 3.25.0 503677 duplicated-cond compiler warning in dis_RV64M diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 128e4298c..f7fd20dba 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -113,8 +113,12 @@ static void usage_NORETURN ( int need_help ) " --vgdb-stop-at=event1,event2,... invoke gdbserver for given events [none]\n" " where event is one of:\n" " startup exit abexit valgrindabexit all none\n" -" --track-fds=no|yes|all track open file descriptors? [no]\n" -" all includes reporting inherited file descriptors\n" +" --track-fds=no|yes|all|bad track open file descriptors? [no]\n" +" all also reports on open inherited file\n" +" descriptors at exit (e.g. stdin/out/err)\n" +" bad only reports on file descriptor usage\n" +" errors and doesn't list open file descriptors\n" +" at exit\n" " --modify-fds=no|yes|high modify newly open file descriptors? [no]\n" " --time-stamp=no|yes add timestamps to log messages? [no]\n" " --log-fd= log messages to file descriptor [2=stderr]\n" @@ -643,6 +647,8 @@ static void process_option (Clo_Mode mode, VG_(clo_track_fds) = 2; else if (VG_(strcmp)(tmp_str, "no") == 0) VG_(clo_track_fds) = 0; + else if (VG_(strcmp)(tmp_str, "bad") == 0) + VG_(clo_track_fds) = 3; else VG_(fmsg_bad_option)(arg, "Bad argument, should be 'yes', 'all' or 'no'\n"); @@ -2324,7 +2330,7 @@ void shutdown_actions_NORETURN( ThreadId tid, } /* Print out file descriptor summary and stats. */ - if (VG_(clo_track_fds)) + if (VG_(clo_track_fds) && VG_(clo_track_fds) < 3) VG_(show_open_fds)("at exit"); /* Call the tool's finalisation function. This makes Memcheck's diff --git a/docs/xml/manual-core.xml b/docs/xml/manual-core.xml index 7d18d46f3..9ab09b51b 100644 --- a/docs/xml/manual-core.xml +++ b/docs/xml/manual-core.xml @@ -886,18 +886,31 @@ in most cases. We group the available options by rough categories. - - - - When enabled, Valgrind will print out a list of open file - descriptors on exit or on request, via the gdbserver monitor - command v.info open_fds. Along with each - file descriptor is printed a stack backtrace of where the file - was opened and any details relating to the file descriptor such - as the file name or socket details. Use to - include reporting on stdin, + + + + When enabled, Valgrind will track all file descriptor + usage. It will produce errors for bad file descriptor usage like + closing a file descriptor twice, using a file descriptor + (number) that was never created, or passing an already closed + file descriptor to a system call. It will also print out a list + of open file descriptors on exit. Or on request, via the + gdbserver monitor command v.info open_fds. + + When an error is generated, or when listing the still open + file descriptors at exit, a stack backtrace of where the file + was opened is printed. If the file descriptor has already been + closed, it will also include a backtrace of where it was + previously closed. Any error will include details relating to + the file descriptor such as the file name or socket details. + + Use to also include reporting + on stdin, stdout and - stderr. + stderr (or other inherited file + descriptors) at exit. If option is used, + then exclude reporting on leaked file descriptors at exit and + only produce errors about bad file decriptors usage. diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index ca01f9e42..716ce000d 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -275,7 +275,8 @@ EXTRA_DIST = \ track_new.stderr.exp track_new.stdout.exp \ track_new.vgtest track_new.stderr.exp-illumos \ track_yes.vgtest track_high.vgtest \ - track_yes.stderr.exp track_high.stderr.exp + track_yes.stderr.exp track_high.stderr.exp \ + track_bad.vgtest track_bad.stderr.exp check_PROGRAMS = \ args \ @@ -334,7 +335,8 @@ check_PROGRAMS = \ fdbaduse \ use_after_close \ track_new \ - track_std + track_std \ + track_bad if HAVE_STATIC_LIBC if ! VGCONF_OS_IS_LINUX diff --git a/none/tests/cmdline1.stdout.exp b/none/tests/cmdline1.stdout.exp index 06a679a11..5d3ea0569 100644 --- a/none/tests/cmdline1.stdout.exp +++ b/none/tests/cmdline1.stdout.exp @@ -28,8 +28,12 @@ usage: valgrind [options] prog-and-args --vgdb-stop-at=event1,event2,... invoke gdbserver for given events [none] where event is one of: startup exit abexit valgrindabexit all none - --track-fds=no|yes|all track open file descriptors? [no] - all includes reporting inherited file descriptors + --track-fds=no|yes|all|bad track open file descriptors? [no] + all also reports on open inherited file + descriptors at exit (e.g. stdin/out/err) + bad only reports on file descriptor usage + errors and doesn't list open file descriptors + at exit --modify-fds=no|yes|high modify newly open file descriptors? [no] --time-stamp=no|yes add timestamps to log messages? [no] --log-fd= log messages to file descriptor [2=stderr] diff --git a/none/tests/cmdline1.stdout.exp-non-linux b/none/tests/cmdline1.stdout.exp-non-linux index 4049d5ab2..9b044e78a 100644 --- a/none/tests/cmdline1.stdout.exp-non-linux +++ b/none/tests/cmdline1.stdout.exp-non-linux @@ -28,8 +28,12 @@ usage: valgrind [options] prog-and-args --vgdb-stop-at=event1,event2,... invoke gdbserver for given events [none] where event is one of: startup exit abexit valgrindabexit all none - --track-fds=no|yes|all track open file descriptors? [no] - all includes reporting inherited file descriptors + --track-fds=no|yes|all|bad track open file descriptors? [no] + all also reports on open inherited file + descriptors at exit (e.g. stdin/out/err) + bad only reports on file descriptor usage + errors and doesn't list open file descriptors + at exit --modify-fds=no|yes|high modify newly open file descriptors? [no] --time-stamp=no|yes add timestamps to log messages? [no] --log-fd= log messages to file descriptor [2=stderr] diff --git a/none/tests/cmdline2.stdout.exp b/none/tests/cmdline2.stdout.exp index d7914ae01..618c43acb 100644 --- a/none/tests/cmdline2.stdout.exp +++ b/none/tests/cmdline2.stdout.exp @@ -28,8 +28,12 @@ usage: valgrind [options] prog-and-args --vgdb-stop-at=event1,event2,... invoke gdbserver for given events [none] where event is one of: startup exit abexit valgrindabexit all none - --track-fds=no|yes|all track open file descriptors? [no] - all includes reporting inherited file descriptors + --track-fds=no|yes|all|bad track open file descriptors? [no] + all also reports on open inherited file + descriptors at exit (e.g. stdin/out/err) + bad only reports on file descriptor usage + errors and doesn't list open file descriptors + at exit --modify-fds=no|yes|high modify newly open file descriptors? [no] --time-stamp=no|yes add timestamps to log messages? [no] --log-fd= log messages to file descriptor [2=stderr] diff --git a/none/tests/cmdline2.stdout.exp-non-linux b/none/tests/cmdline2.stdout.exp-non-linux index d709ea365..767f1cb9c 100644 --- a/none/tests/cmdline2.stdout.exp-non-linux +++ b/none/tests/cmdline2.stdout.exp-non-linux @@ -28,7 +28,7 @@ usage: valgrind [options] prog-and-args --vgdb-stop-at=event1,event2,... invoke gdbserver for given events [none] where event is one of: startup exit abexit valgrindabexit all none - --track-fds=no|yes|all track open file descriptors? [no] + --track-fds=no|yes|all|bad track open file descriptors? [no] all includes reporting inherited file descriptors --modify-fds=no|yes|high modify newly open file descriptors? [no] --time-stamp=no|yes add timestamps to log messages? [no] diff --git a/none/tests/track_bad.c b/none/tests/track_bad.c new file mode 100644 index 000000000..b1674d648 --- /dev/null +++ b/none/tests/track_bad.c @@ -0,0 +1,34 @@ +#include +#include +#include +#include +#include "fdleak.h" + +int main(void) +{ + char *string = "bad\n"; + int fd = dup(2); + + /* OK. */ + write(fd, string, 4); + close(fd); + + /* Already closed. */ + write(fd, string, 4); + + /* Never created. */ + write(7, string, 4); + + /* Invalid. */ + write(-7, string, 4); + + /* Double double close. */ + close(fd); + + /* Invalid close. */ + close (-7); + + (void) DO( open("/dev/null", O_RDONLY) ); + + return 0; +} diff --git a/none/tests/track_bad.stderr.exp b/none/tests/track_bad.stderr.exp new file mode 100644 index 000000000..620a6b8d2 --- /dev/null +++ b/none/tests/track_bad.stderr.exp @@ -0,0 +1,28 @@ +bad +File descriptor was closed already + at 0x........: write (in /...libc...) + by 0x........: main + Previously closed + at 0x........: close (in /...libc...) + by 0x........: main + Originally opened + at 0x........: dup (in /...libc...) + by 0x........: main +File descriptor 7 was never created + at 0x........: write (in /...libc...) + by 0x........: main +File descriptor -7 Invalid file descriptor + at 0x........: write (in /...libc...) + by 0x........: main +File descriptor ...: ... is already closed + at 0x........: close (in /...libc...) + by 0x........: main + Previously closed + at 0x........: close (in /...libc...) + by 0x........: main + Originally opened + at 0x........: dup (in /...libc...) + by 0x........: main +File descriptor -7 Invalid file descriptor + at 0x........: close (in /...libc...) + by 0x........: main diff --git a/none/tests/track_bad.vgtest b/none/tests/track_bad.vgtest new file mode 100644 index 000000000..77daf8f44 --- /dev/null +++ b/none/tests/track_bad.vgtest @@ -0,0 +1,3 @@ +prog: track_bad +vgopts: -q --track-fds=bad +stderr_filter: filter_fdleak -- 2.39.5