changeset cf07f8bf58db in /z/repo/gem5
details: http://repo.gem5.org/gem5?cmd=changeset;node=cf07f8bf58db
description:
        syscall_emul: Bandage readlink /proc/self/exe

        The recent changeset to readlink() to handle reading the /proc/self/exe 
link
        introduces a number of problems. This patch fixes two:

        1) Because readlink() called on /proc/self/exe now uses 
LiveProcess::progName()
        to find the binary path, it will only get the zeroth parameter of the 
simulated
        system command line. However, if a config script also specifies the 
process'
        executable, the executable parameter is used to create the LiveProcess 
rather
        than the zeroth command line parameter. Thus, the zeroth command line 
parameter
        is not necessarily the correct path to the binary executing in the 
simulated
        system. To fix this, add a LiveProcess data member, 'executable', which 
is
        correctly set during instantiation and returned from progName().

        2) If a config script allows a user to pass a relative path as the 
zeroth
        simulated system command line parameter or process executable, 
readlink() will
        incorrecly return a relative path when called on '/proc/self/exe'.
        /proc/self/exe is always set to a full path, so running benchmarks can 
fail if
        a relative path is returned. To fix this, clean up the handling of
        LiveProcess::progName() within readlink() to get the full binary path.

        NOTE: This patch still leaves the potential problem that host full path 
to the
        binary bleeds into the simulated system, potentially causing the 
appearance of
        non-deterministic simulated system execution.

diffstat:

 src/sim/process.cc      |  13 +++++++++----
 src/sim/process.hh      |   3 ++-
 src/sim/syscall_emul.cc |  41 +++++++++++++++++++++++++++--------------
 3 files changed, 38 insertions(+), 19 deletions(-)

diffs (107 lines):

diff -r bd894d2bdd7c -r cf07f8bf58db src/sim/process.cc
--- a/src/sim/process.cc        Fri Sep 25 13:25:34 2015 -0400
+++ b/src/sim/process.cc        Tue Sep 29 09:25:20 2015 -0500
@@ -472,6 +472,7 @@
 LiveProcess::LiveProcess(LiveProcessParams *params, ObjectFile *_objFile)
     : Process(params), objFile(_objFile),
       argv(params->cmd), envp(params->env), cwd(params->cwd),
+      executable(params->executable),
       __uid(params->uid), __euid(params->euid),
       __gid(params->gid), __egid(params->egid),
       __pid(params->pid), __ppid(params->ppid),
@@ -528,11 +529,15 @@
 {
     LiveProcess *process = NULL;
 
-    string executable =
-        params->executable == "" ? params->cmd[0] : params->executable;
-    ObjectFile *objFile = createObjectFile(executable);
+    // If not specified, set the executable parameter equal to the
+    // simulated system's zeroth command line parameter
+    if (params->executable == "") {
+        params->executable = params->cmd[0];
+    }
+
+    ObjectFile *objFile = createObjectFile(params->executable);
     if (objFile == NULL) {
-        fatal("Can't load object file %s", executable);
+        fatal("Can't load object file %s", params->executable);
     }
 
     if (objFile->isDynamic())
diff -r bd894d2bdd7c -r cf07f8bf58db src/sim/process.hh
--- a/src/sim/process.hh        Fri Sep 25 13:25:34 2015 -0400
+++ b/src/sim/process.hh        Tue Sep 29 09:25:20 2015 -0500
@@ -238,6 +238,7 @@
     std::vector<std::string> argv;
     std::vector<std::string> envp;
     std::string cwd;
+    std::string executable;
 
     LiveProcess(LiveProcessParams *params, ObjectFile *objFile);
 
@@ -294,7 +295,7 @@
     inline uint64_t ppid() {return __ppid;}
 
     // provide program name for debug messages
-    virtual const char *progName() const { return argv[0].c_str(); }
+    virtual const char *progName() const { return executable.c_str(); }
 
     std::string
     fullPath(const std::string &filename)
diff -r bd894d2bdd7c -r cf07f8bf58db src/sim/syscall_emul.cc
--- a/src/sim/syscall_emul.cc   Fri Sep 25 13:25:34 2015 -0400
+++ b/src/sim/syscall_emul.cc   Tue Sep 29 09:25:20 2015 -0500
@@ -407,25 +407,38 @@
     if (path != "/proc/self/exe") {
         result = readlink(path.c_str(), (char *)buf.bufferPtr(), bufsiz);
     } else {
-        // readlink() will return the path of the binary given
-        // with the -c option, however it is possible that this
-        // will still result in incorrect behavior if one binary
-        // runs another, e.g., -c time -o "my_binary" where
-        // my_binary calls readlink(). this is a very unlikely case,
-        // so we issue a warning.
-        warn_once("readlink may yield unexpected results if multiple "
-                  "binaries are used\n");
-        if (strlen(p->progName()) > bufsiz) {
+        // Emulate readlink() called on '/proc/self/exe' should return the
+        // absolute path of the binary running in the simulated system (the
+        // LiveProcess' executable). It is possible that using this path in
+        // the simulated system will result in unexpected behavior if:
+        //  1) One binary runs another (e.g., -c time -o "my_binary"), and
+        //     called binary calls readlink().
+        //  2) The host's full path to the running benchmark changes from one
+        //     simulation to another. This can result in different simulated
+        //     performance since the simulated system will process the binary
+        //     path differently, even if the binary itself does not change.
+
+        // Get the absolute canonical path to the running application
+        char real_path[PATH_MAX];
+        char *check_real_path = realpath(p->progName(), real_path);
+        if (!check_real_path) {
+            fatal("readlink('/proc/self/exe') unable to resolve path to "
+                  "executable: %s", p->progName());
+        }
+        strncpy((char*)buf.bufferPtr(), real_path, bufsiz);
+        size_t real_path_len = strlen(real_path);
+        if (real_path_len > bufsiz) {
             // readlink will truncate the contents of the
             // path to ensure it is no more than bufsiz
-            strncpy((char*)buf.bufferPtr(), p->progName(), bufsiz);
             result = bufsiz;
         } else {
-            // return the program's working path rather
-            // than the one for the gem5 binary itself.
-            strcpy((char*)buf.bufferPtr(), p->progName());
-            result = strlen((char*)buf.bufferPtr());
+            result = real_path_len;
         }
+
+        // Issue a warning about potential unexpected results
+        warn_once("readlink() called on '/proc/self/exe' may yield unexpected "
+                  "results in various settings.\n      Returning '%s'\n",
+                  (char*)buf.bufferPtr());
     }
 
     buf.copyOut(tc->getMemProxy());
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to