Merge pull request #1075 from darshan-hpc/dev-fcntl-dup-wrap-wrappers

Wrap fcntl() calls

Author: roblatham00

darshan-runtime/lib/darshan-posix.c

@@ -55,6 +55,8 @@ DARSHAN_FORWARD_DECL(creat64, int, (const char* path, mode_t mode));
 DARSHAN_FORWARD_DECL(dup, int, (int oldfd));
 DARSHAN_FORWARD_DECL(dup2, int, (int oldfd, int newfd));
 DARSHAN_FORWARD_DECL(dup3, int, (int oldfd, int newfd, int flags));
+DARSHAN_FORWARD_DECL(fcntl, int, (int, int, ...));
+DARSHAN_FORWARD_DECL(fcntl64, int, (int, int, ...));
 DARSHAN_FORWARD_DECL(fileno, int, (FILE *stream));
 DARSHAN_FORWARD_DECL(mkstemp, int, (char *template));
 DARSHAN_FORWARD_DECL(mkostemp, int, (char *template, int flags));
@@ -787,6 +789,85 @@ int DARSHAN_DECL(dup3)(int oldfd, int newfd, int flags)
     return(ret);
 }
 
+/* wrapping fcntl is a little strange for two related reasons:
+ * - the code can do a lot of different things based on 'cmd'
+ * - some of those 'cmd' (not all) take a third argument
+ *
+ * There are some worrying notes in the documentation about calling va_args
+ * when there's no third (variable) argument, but we observed glibc doing that
+ * and our testing seems to indicate it's ok to do so.
+ *
+ * So we'll always grab the variable argument, even if it's not there, and
+ * always pass whatever we get to the real fcntl.  Then we'll figure out if the
+ * command was something we should log or not, and update our stats accordingly
+ */
+
+int DARSHAN_DECL(fcntl)(int fd, int cmd, ...)
+{
+    int ret;
+    double tm1, tm2;
+    va_list arg;
+    void *next;
+
+    MAP_OR_FAIL(fcntl);
+
+    va_start(arg, cmd);
+    next = va_arg(arg, void*);
+    va_end(arg);
+
+    tm1 = POSIX_WTIME();
+    ret  = __real_fcntl(fd, cmd, next);
+    tm2 = POSIX_WTIME();
+
+    struct posix_file_record_ref *rec_ref;
+
+    /* some code (e.g. python) prefers (portabilty? functionality?) to
+     * duplicate the file descriptor via fcntl instead of dup/dup2/dup3 */
+    if (ret >= 0 && (cmd == F_DUPFD || cmd == F_DUPFD_CLOEXEC)) {
+        POSIX_PRE_RECORD();
+        rec_ref = darshan_lookup_record_ref(posix_runtime->fd_hash,
+                &fd, sizeof(fd));
+
+        POSIX_RECORD_REFOPEN(ret, rec_ref, tm1, tm2, POSIX_DUPS);
+        POSIX_POST_RECORD();
+    }
+
+    return ret;
+}
+
+int DARSHAN_DECL(fcntl64)(int fd, int cmd, ...)
+{
+    int ret;
+    double tm1, tm2;
+    va_list arg;
+    void *next;
+
+    MAP_OR_FAIL(fcntl64);
+
+    va_start(arg, cmd);
+    next = va_arg(arg, void*);
+    va_end(arg);
+
+    tm1 = POSIX_WTIME();
+    ret  = __real_fcntl64(fd, cmd, next);
+    tm2 = POSIX_WTIME();
+
+    struct posix_file_record_ref *rec_ref;
+
+    /* this is a duplicate of code in fcntl, but when I try to break this out
+     * into a separate 'fcntl_common' routine, POSIX_PRE_RECORD() macro cannot
+     * find the __darshan_disabled variable, and furthermore I get weird
+     * "failed to seek" errors. Am I going to have to make this a macro? */
+    if (ret >= 0 && (cmd == F_DUPFD || cmd == F_DUPFD_CLOEXEC)) {
+        POSIX_PRE_RECORD();
+        rec_ref = darshan_lookup_record_ref(posix_runtime->fd_hash,
+                &fd, sizeof(fd));
+        POSIX_RECORD_REFOPEN(ret, rec_ref, tm1, tm2, POSIX_DUPS);
+        POSIX_POST_RECORD();
+    }
+
+    return ret;
+}
 int DARSHAN_DECL(fileno)(FILE *stream)
 {
     int ret;

darshan-runtime/share/ld-opts/darshan-posix-ld-opts

@@ -8,6 +8,8 @@
 --wrap=dup
 --wrap=dup2
 --wrap=dup3
+--wrap=fcntl
+--wrap=fcntl64
 --wrap=mkstemp
 --wrap=mkostemp
 --wrap=mkstemps

darshan-test/python_mpi_scripts/runtime_prog_issue_1072.py

@@ -0,0 +1,15 @@
+#!/usr/bin/env python3
+
+from mpi4py import MPI
+import numpy as np
+import os
+
+def main():
+    comm = MPI.COMM_WORLD
+    arr1 = np.arange(50)
+    np.save('single_array.npy', arr1)
+    os.remove('single_array.npy')
+
+
+if __name__ == "__main__":
+    main()

darshan-test/python_runtime_tests.py

@@ -231,3 +231,63 @@ def test_forked_process_mpi(tmpdir):
     # regression test for gh-786, mpi version
     darshan_install_path = os.environ.get("DARSHAN_INSTALL_PATH")
     do_forked_process_test(tmpdir, darshan_install_path)
+
+def test_os_dup(tmpdir):
+    # numpy calls Python's os.dup() which in turn was duplicating a file
+    # descriptor via fcntl
+    n_ranks = 1
+    root_path = os.environ.get("DARSHAN_ROOT_PATH")
+    darshan_install_path = os.environ.get("DARSHAN_INSTALL_PATH")
+    test_script_path = os.path.join(root_path,
+                                    "darshan-test",
+                                    "python_mpi_scripts",
+                                    "runtime_prog_issue_1072.py")
+    darshan_lib_path = os.path.join(darshan_install_path,
+                                    "lib",
+                                    "libdarshan.so")
+    hdf5_lib_path = os.environ.get("HDF5_LIB")
+
+    with tmpdir.as_cwd():
+        cwd = os.getcwd()
+        subprocess.check_output(["mpirun",
+                     "--allow-run-as-root",
+                     "-n",
+                     f"{n_ranks}",
+                     "-x",
+                     f"LD_PRELOAD={darshan_lib_path}:{hdf5_lib_path}",
+                     "-x",
+                     f"DARSHAN_LOGPATH={cwd}",
+                     "python",
+                     f"{test_script_path}"])
+
+        log_file_list = glob.glob("*.darshan")
+        # only a single log file should be generated
+        # by darshan
+        assert len(log_file_list) == 1
+        path_to_log = os.path.join(cwd, log_file_list[0])
+        # numpy will read a bunch of python files but we only care about the
+        # numpy file
+        target_filename = "single_array.npy"
+
+        # common stuff done.  Real check for the "dup via fcntl" issue
+
+        io_module = "POSIX"
+        key = "POSIX_BYTES_WRITTEN"
+
+        # check if log file exists
+        if not os.path.isfile(path_to_log) :
+            print(f"Darshan log file '{path_to_log}' does not exist")
+        else :
+            print(f"Darshan log file '{path_to_log}'")
+
+        try:
+            with darshan.DarshanReport(path_to_log, filter_patterns=[target_filename], filter_mode='include') as report:
+                # numpy will write the header (128 bytes) out via posix...
+                posix_records = report.records["POSIX"].to_dict()
+                # but flush its payload (50 8 byte ints: 400 bytes) with stdio (flush at fclose)
+                stdio_records = report.records["STDIO"].to_dict()
+                posix_bytes = posix_records[0]["counters"]["POSIX_BYTES_WRITTEN"]
+                stdio_bytes = stdio_records[0]["counters"]["STDIO_BYTES_WRITTEN"]
+                assert posix_bytes + stdio_bytes == 528
+        except RuntimeError:
+            print(f"Error: The file '{path_to_log}' was not found.")