FreeBSD: cleanup and refactor syscalls readlink and readlinkat

There was some code to handle /proc/curproc/file (a symlink to
the exe that wee need to bodge as it refers to the tool exe).
But it was neither tested nor working.

Can't use the same technique as Linux and Solaris which have more
complete /proc filesystems where each pid has symlinks for
each open file, which we use for the guest. Instead need to
copy the path ourselves. So move sys_readlink out of generic.

Simplify the handling of the resolved guest exe name - store it in
a global like VG_(args_the_exename).

diff --git a/.gitignore b/.gitignore
index ae71cbbbddf9bda7af52cafe9e58b249ceb959f4..cc6b189b4b9993134e85facb723ca56345a105c1 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -2101,6 +2101,7 @@
 /none/tests/freebsd/hello_world
 /none/tests/freebsd/452275
 /none/tests/freebsd/usrstack
+/none/tests/freebsd/proc_pid_file
 
 # /none/tests/x86/
 /none/tests/x86/*.dSYM

diff --git a/coregrind/m_clientstate.c b/coregrind/m_clientstate.c
index 7b343f27a50cdba982ce867c3379f15a064d1807..404a60f3720e5fefe14421f62a431786ade7090c 100644 (file)
--- a/coregrind/m_clientstate.c
+++ b/coregrind/m_clientstate.c
@@ -93,6 +93,10 @@ Int VG_(args_for_valgrind_noexecpass) = 0;
    line. */
 const HChar* VG_(args_the_exename) = NULL;
 
+/* The real name of the executable, with resolved
+ * relative paths and symlinks */
+const HChar* VG_(resolved_exename) = NULL;
+
 // Client's original rlimit data and rlimit stack
 struct vki_rlimit VG_(client_rlimit_data);
 struct vki_rlimit VG_(client_rlimit_stack);

diff --git a/coregrind/m_initimg/initimg-freebsd.c b/coregrind/m_initimg/initimg-freebsd.c
index 22c210cdeb16d35759f94108b23555fe7cf0e313..53a9aca8731dad227c7e2ed0b4d0b323844c15c3 100644 (file)
--- a/coregrind/m_initimg/initimg-freebsd.c
+++ b/coregrind/m_initimg/initimg-freebsd.c
@@ -362,8 +362,7 @@ static HChar *copy_bytes(HChar **tab, const HChar *src, SizeT size)
 
    ---------------------------------------------------------------- */
 
-static
-struct auxv *find_auxv(UWord* sp)
+static struct auxv *find_auxv(UWord* sp)
 {
    sp++;                // skip argc (Nb: is word-sized, not int-sized!)
 
@@ -380,13 +379,12 @@ struct auxv *find_auxv(UWord* sp)
    return (struct auxv *)sp;
 }
 
-static
-Addr setup_client_stack( void*  init_sp,
-                         HChar** orig_envp,
-                         const ExeInfo* info,
-                         UInt** client_auxv,
-                         Addr   clstack_end,
-                         SizeT  clstack_max_size )
+static Addr setup_client_stack(void*  init_sp,
+                               HChar** orig_envp,
+                               const ExeInfo* info,
+                               UInt** client_auxv,
+                               Addr   clstack_end,
+                               SizeT  clstack_max_size )
 {
    SysRes res;
    HChar **cpp;
@@ -690,6 +688,7 @@ Addr setup_client_stack( void*  init_sp,
 
       case VKI_AT_EXECPATH:
          auxv->u.a_ptr = copy_str(&strtab, resolved_name);
+         VG_(resolved_exename) = auxv->u.a_ptr;
          break;
       case VKI_AT_CANARY:
          if (canarylen >= 1) {
@@ -795,6 +794,11 @@ Addr setup_client_stack( void*  init_sp,
    if (0) {
       VG_(printf)("startup SP = %#lx\n", client_SP);
    }
+
+   if (VG_(resolved_exename) == NULL) {
+      VG_(resolved_exename) = VG_(strdup)("initimg-freebsd.sre.1", resolved_name);
+   }
+
    return client_SP;
 }
 

diff --git a/coregrind/m_main.c b/coregrind/m_main.c
index 4316e625fa4bc01cec50859bac11ef088144d7b3..0a7e96e5009589357902c3ec2340a02c2d9640a2 100644 (file)
--- a/coregrind/m_main.c
+++ b/coregrind/m_main.c
@@ -1724,6 +1724,13 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp )
       if (!sr_isError(statres) || VKI_S_ISLNK(statbuf.mode)) {
          VG_(have_slash_proc) = True;
       }
+      // each directory contains the following that might get read
+      // file - a symlink to the exe
+      // cmdline - null separate command line
+      // etype - the executable type e.g., FreeBSD ELF64 (same for guest and host)
+      // map - a memory map, tricky to synthesize
+      // rlimit - list of process limits
+      // status - process, pid, ppid pts cty uid gid and some other stuff
    }
 #endif
 

diff --git a/coregrind/m_syswrap/priv_syswrap-freebsd.h b/coregrind/m_syswrap/priv_syswrap-freebsd.h
index dd065c6f585968ced3b7073d897f9079295c66c4..8822f22f21d2862eed00f10ace619191372a8ffb 100644 (file)
--- a/coregrind/m_syswrap/priv_syswrap-freebsd.h
+++ b/coregrind/m_syswrap/priv_syswrap-freebsd.h
@@ -90,7 +90,7 @@ DECL_TEMPLATE(freebsd, sys_ioctl) // 54
 DECL_TEMPLATE(freebsd, sys_reboot) // 55
 DECL_TEMPLATE(freebsd, sys_revoke) // 56
 // generic symlink 57
-// generic readlink 58
+DECL_TEMPLATE(freebsd, sys_readlink) // 58
 // generic execve 59
 // generic umask 60
 // generic chroot 61

diff --git a/coregrind/m_syswrap/syswrap-freebsd.c b/coregrind/m_syswrap/syswrap-freebsd.c
index 13d3b6bf257f5f524e3fd585ecef88f13932ece5..1ab63ba491e23de03045519a55ff8441064a446a 100644 (file)
--- a/coregrind/m_syswrap/syswrap-freebsd.c
+++ b/coregrind/m_syswrap/syswrap-freebsd.c
@@ -1139,8 +1139,52 @@ PRE(sys_revoke)
 // SYS_symlink 57
 // generic
 
+static void do_readlink(const HChar* path, HChar *buf, SizeT bufsize, SyscallStatus* status, Bool* curproc_file)
+{
+   HChar name[30];
+   VG_(sprintf)(name, "/proc/%d/file", VG_(getpid)());
+   if (ML_(safe_to_deref)(path, 1)
+         && (VG_(strcmp)(path, name) == 0
+             || VG_(strcmp)(path, "/proc/curproc/file") == 0)) {
+      vg_assert(VG_(resolved_exename));
+      Int len = VG_(snprintf)(buf, bufsize, "%s",  VG_(resolved_exename));
+      SET_STATUS_Success(len);
+      *curproc_file = True;
+   }
+}
+
 // SYS_readlink   58
-// generic
+// ssize_t  readlink(const char *restrict path, char *restrict buf, size_t bufsiz);
+PRE(sys_readlink)
+{
+   FUSE_COMPATIBLE_MAY_BLOCK();
+   Word saved = SYSNO;
+   Bool curproc_file = False;
+
+   PRINT("sys_readlink ( %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %llu )",
+         ARG1, (char*)(Addr)ARG1, ARG2, (ULong)ARG3);
+   PRE_REG_READ3(long, "readlink",
+                 const char *, path, char *, buf, int, bufsiz);
+   PRE_MEM_RASCIIZ( "readlink(path)", ARG1 );
+   PRE_MEM_WRITE( "readlink(buf)", ARG2,ARG3 );
+
+   if (VG_(have_slash_proc) == True)
+   {
+      /*
+       * Handle the case where readlink is looking at /proc/curproc/file or
+       * /proc/<pid>/file
+       */
+      do_readlink((const HChar *)ARG1, (HChar *)ARG2, (SizeT)ARG3, status, &curproc_file);
+   }
+
+   if (!curproc_file) {
+      /* Normal case */
+      SET_STATUS_from_SysRes( VG_(do_syscall3)(saved, ARG1, ARG2, ARG3));
+   }
+   if (SUCCESS && RES > 0) {
+      POST_MEM_WRITE( ARG2, RES );
+   }
+}
 
 // SYS_execve  59
 // generic
@@ -1941,8 +1985,7 @@ static void sysctl_kern_usrstack(SizeT* out, SizeT* outlen)
 
 static Bool sysctl_kern_proc_pathname(HChar *out, SizeT *len)
 {
-   // is this stashed somewhere?
-   const HChar *exe_name = VG_(find_executable)(VG_(args_the_exename));
+   const HChar *exe_name = VG_(resolved_exename);
 
    if (!VG_(realpath)(exe_name, out)) {
       return False;
@@ -5325,8 +5368,8 @@ POST(sys_openat)
 //                    size_t bufsize);
 PRE(sys_readlinkat)
 {
-   HChar name[25];
    Word  saved = SYSNO;
+   Bool curproc_file = False;
 
    PRINT("sys_readlinkat ( %" FMT_REGWORD "u, %#" FMT_REGWORD "x(%s), %#" FMT_REGWORD "x, %llu )", ARG1,ARG2,(char*)ARG2,ARG3,(ULong)ARG4);
    PRE_REG_READ4(ssize_t, "readlinkat",
@@ -5334,23 +5377,24 @@ PRE(sys_readlinkat)
    PRE_MEM_RASCIIZ( "readlinkat(path)", ARG2 );
    PRE_MEM_WRITE( "readlinkat(buf)", ARG3,ARG4 );
 
-   if (VG_(have_slash_proc) == True) {
+   if (VG_(have_slash_proc) == True && (Int)ARG1 == VKI_AT_FDCWD) {
       /*
        * Handle the case where readlinkat is looking at /proc/curproc/file or
        * /proc/<pid>/file.
        */
-      VG_(sprintf)(name, "/proc/%d/file", VG_(getpid)());
-      if (ML_(safe_to_deref)((void*)ARG2, 1)
-            && (VG_(strcmp)((HChar *)ARG2, name) == 0
-                || VG_(strcmp)((HChar *)ARG2, "/proc/curproc/file") == 0)) {
-         VG_(sprintf)(name, "/proc/curproc/fd/%d", VG_(cl_exec_fd));
-         SET_STATUS_from_SysRes( VG_(do_syscall4)(saved, ARG1, (UWord)name,
-                                 ARG3, ARG4));
-         return;
-      }
+      do_readlink((const HChar *)ARG2, (HChar *)ARG3, (SizeT)ARG4, status, &curproc_file);
+   }
+
+   // @todo PJF there is still the case where fd refers to /proc or /proc/pid
+   // or /proc/curproc and path is relative pid/file, curptoc/file or just file
+
+   if (!curproc_file) {
+      /* Normal case */
+      SET_STATUS_from_SysRes( VG_(do_syscall4)(saved, ARG1, ARG2, ARG3, ARG4));
+   }
+   if (SUCCESS && RES > 0) {
+      POST_MEM_WRITE( ARG3, RES );
    }
-   /* Normal case */
-   SET_STATUS_from_SysRes( VG_(do_syscall4)(saved, ARG1, ARG2, ARG3, ARG4));
 }
 
 POST(sys_readlinkat)
@@ -6696,7 +6740,7 @@ const SyscallTableEntry ML_(syscall_table)[] = {
 
    BSDX_(__NR_revoke,           sys_revoke),            // 56
    GENX_(__NR_symlink,          sys_symlink),           // 57
-   GENX_(__NR_readlink,         sys_readlink),          // 58
+   BSDX_(__NR_readlink,         sys_readlink),          // 58
    GENX_(__NR_execve,           sys_execve),            // 59
 
    GENX_(__NR_umask,            sys_umask),             // 60

diff --git a/include/pub_tool_clientstate.h b/include/pub_tool_clientstate.h
index 556ab03dd5dbba517ffcdfe563dd142bfb38b133..e25a59702ae186368adffc9c1c12d721facca4aa 100644 (file)
--- a/include/pub_tool_clientstate.h
+++ b/include/pub_tool_clientstate.h
@@ -63,6 +63,7 @@ extern Int VG_(args_for_valgrind_noexecpass);
    line. */
 extern const HChar* VG_(args_the_exename);
 
+extern const HChar* VG_(resolved_exename);
 
 #endif   // __PUB_TOOL_CLIENTSTATE_H
 

diff --git a/none/tests/freebsd/Makefile.am b/none/tests/freebsd/Makefile.am
index 5fe5de3627779588b3016c9bcb94e97bd26e4ad2..f956078d684f551db20565b3c6db57150c58b3ec 100644 (file)
--- a/none/tests/freebsd/Makefile.am
+++ b/none/tests/freebsd/Makefile.am
@@ -35,10 +35,13 @@ EXTRA_DIST = \
        452275.stderr.exp \
        usrstack.vgtest \
        usrstack.stderr.exp \
-       usrstack.stdout.exp
+       usrstack.stdout.exp \
+       proc_pid_file.vgtest \
+       proc_pid_file.stderr.exp
 
 check_PROGRAMS = \
-       auxv osrel swapcontext hello_world fexecve 452275 usrstack
+       auxv osrel swapcontext hello_world fexecve 452275 usrstack \
+       proc_pid_file
 
 AM_CFLAGS   += $(AM_FLAG_M3264_PRI)
 AM_CXXFLAGS += $(AM_FLAG_M3264_PRI)
@@ -48,3 +51,5 @@ osrel_CFLAGS = ${AM_CFLAGS}
 swapcontext_CFLAGS = ${AM_CFLAGS}
 
 hello_world_SOURCES = hello_world.cpp
+
+proc_pid_file_SOURCES = proc_pid_file.cpp

diff --git a/none/tests/freebsd/proc_pid_file.cpp b/none/tests/freebsd/proc_pid_file.cpp
new file mode 100644 (file)
index 0000000..cf9d185
--- /dev/null
+++ b/none/tests/freebsd/proc_pid_file.cpp
@@ -0,0 +1,27 @@
+#include <iostream>
+//#include <fstream>
+#include <string>
+//#include <cstdlib>
+#include <unistd.h>
+#include <fcntl.h>
+#include <cassert>
+
+int main()
+{
+   char resolvedPath[PATH_MAX];
+   auto count = readlink("/proc/curproc/file", resolvedPath, PATH_MAX);
+   resolvedPath[count] = '\0';
+   //std::cout << "resolvedPath: " << resolvedPath << '\n';
+   char resolvedPath2[PATH_MAX];
+   auto count2 = readlinkat(AT_FDCWD, "/proc/curproc/file", resolvedPath2, PATH_MAX);
+   resolvedPath2[count2] = '\0';
+   //std::cout << "resolvedPath2: " << resolvedPath2 << '\n';
+   std::string rp(resolvedPath);
+   assert(rp == resolvedPath2);
+   
+   auto n = rp.rfind("proc_pid_file");
+   
+   std::string filename(rp.substr(n));
+   //std::cout << "filename: " << filename << '\n';
+   assert(filename == "proc_pid_file");
+}

diff --git a/none/tests/freebsd/proc_pid_file.stderr.exp b/none/tests/freebsd/proc_pid_file.stderr.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/none/tests/freebsd/proc_pid_file.vgtest b/none/tests/freebsd/proc_pid_file.vgtest
new file mode 100644 (file)
index 0000000..0361a59
--- /dev/null
+++ b/none/tests/freebsd/proc_pid_file.vgtest
@@ -0,0 +1,3 @@
+prog: proc_pid_file
+vgopts: -q
+