[Valgrind-developers] [valgrind/VALGRIND_3_25_BRANCH] Check whether file descriptor is inherited before printing where_opened

[Mark W. <ma...@so...>]

https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=8a55301a0bdb8dcd308bd076cb12e924917e8964

commit 8a55301a0bdb8dcd308bd076cb12e924917e8964
Author: Mark Wielaard <ma...@kl...>
Date:   Sun May 18 15:31:36 2025 +0200

    Check whether file descriptor is inherited before printing where_opened
    
    Inherited file descriptors don't have an ExeContext where they were
    opened (by the program). So don't try to print the NULL where_opened
    when reporting double close errors for such file descriptors.
    
    Add a testcase none/tests/fdleak_doubleclose0 that crashes valgrind
    before this fix.
    
    https://bugs.kde.org/show_bug.cgi?id=504466
    (cherry picked from commit 8187386962598d1393eaf6cf4e032996f5edabb3)

Diff:
---
 coregrind/m_syswrap/syswrap-generic.c     | 13 +++++++++----
 none/tests/Makefile.am                    |  3 ++-
 none/tests/fdleak_doubleclose0.c          | 10 ++++++++++
 none/tests/fdleak_doubleclose0.stderr.exp | 12 ++++++++++++
 none/tests/fdleak_doubleclose0.vgtest     |  3 +++
 5 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/coregrind/m_syswrap/syswrap-generic.c b/coregrind/m_syswrap/syswrap-generic.c
index 81c8fc028d..98cbb172fd 100644
--- a/coregrind/m_syswrap/syswrap-generic.c
+++ b/coregrind/m_syswrap/syswrap-generic.c
@@ -637,7 +637,8 @@ struct BadCloseExtra {
    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 */
+   ExeContext *where_opened;      /* recordwhere the fd  was opened,
+                                     NULL if inherited file descriptor */
 };
 
 struct FdBadUse {
@@ -646,7 +647,8 @@ struct FdBadUse {
   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 */
+  ExeContext *where_opened;      /* recordwhere the fd  was opened,
+                                    NULL if inherited file descriptor */
 };
 
 struct NotClosedExtra {
@@ -1197,8 +1199,11 @@ void fd_pp_Error (const Error *err)
       VG_(pp_ExeContext)( where );
       VG_(emit)("%sPreviously closed%s\n", auxpre, auxpost);
       VG_(pp_ExeContext)(bce->where_closed);
-      VG_(emit)("%sOriginally opened%s\n", auxpre, auxpost);
-      VG_(pp_ExeContext)(bce->where_opened);
+      // Inherited file descriptors where never opened (by the program)
+      if (bce->where_opened) {
+         VG_(emit)("%sOriginally opened%s\n", auxpre, auxpost);
+         VG_(pp_ExeContext)(bce->where_opened);
+      }
    } else if (VG_(get_error_kind)(err) == FdNotClosed) {
       if (xml) VG_(emit)("  <kind>FdNotClosed</kind>\n");
       struct NotClosedExtra *nce = (struct NotClosedExtra *)
diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am
index bebc2f4dd1..6305044ca6 100644
--- a/none/tests/Makefile.am
+++ b/none/tests/Makefile.am
@@ -150,6 +150,7 @@ EXTRA_DIST = \
 	fdleak_creat_xml.stderr.exp fdleak_creat_xml.vgtest \
 	fdleak_creat_sup.stderr.exp fdleak_creat_sup.supp \
 	fdleak_creat_sup.vgtest \
+	fdleak_doubleclose0.stderr.exp fdleak_doubleclose0.vgtest \
 	fdleak_dup.stderr.exp fdleak_dup.vgtest \
 	fdleak_dup_xml.stderr.exp fdleak_dup_xml.vgtest \
 	fdleak_dup2.stderr.exp fdleak_dup2.vgtest \
@@ -283,7 +284,7 @@ check_PROGRAMS = \
 	bug492678 \
 	closeall coolo_strlen \
 	discard exec-sigmask execve faultstatus fcntl_setown \
-	fdleak_cmsg fdleak_creat fdleak_dup fdleak_dup2 \
+	fdleak_cmsg fdleak_creat fdleak_doubleclose0 fdleak_dup fdleak_dup2 \
 	fdleak_fcntl fdleak_ipv4 fdleak_open fdleak_pipe \
 	fdleak_socketpair \
 	floored fork fucomip \
diff --git a/none/tests/fdleak_doubleclose0.c b/none/tests/fdleak_doubleclose0.c
new file mode 100644
index 0000000000..83c89a8c4b
--- /dev/null
+++ b/none/tests/fdleak_doubleclose0.c
@@ -0,0 +1,10 @@
+#include <unistd.h>
+
+int main (int argc, char **argv)
+{
+   close (0);
+   close (1);
+   close (1);
+   close (0);
+   return 0;
+}
diff --git a/none/tests/fdleak_doubleclose0.stderr.exp b/none/tests/fdleak_doubleclose0.stderr.exp
new file mode 100644
index 0000000000..6487cf8a42
--- /dev/null
+++ b/none/tests/fdleak_doubleclose0.stderr.exp
@@ -0,0 +1,12 @@
+File descriptor ...: ... is already closed
+   at 0x........: close (in /...libc...)
+   by 0x........: main
+ Previously closed
+   at 0x........: close (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
diff --git a/none/tests/fdleak_doubleclose0.vgtest b/none/tests/fdleak_doubleclose0.vgtest
new file mode 100644
index 0000000000..36d3e36a60
--- /dev/null
+++ b/none/tests/fdleak_doubleclose0.vgtest
@@ -0,0 +1,3 @@
+prog: fdleak_doubleclose0
+vgopts: -q --track-fds=yes
+stderr_filter: filter_fdleak