----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://reviews.gem5.org/r/3106/#review7215 -----------------------------------------------------------
src/sim/syscall_emul.cc (line 411) <http://reviews.gem5.org/r/3106/#comment6130> should just be 'readlink() should return the path of the executable'... the stuff about argv[0] is not relevant here src/sim/syscall_emul.cc (line 425) <http://reviews.gem5.org/r/3106/#comment6131> say 'absolute' rather than 'full' src/sim/syscall_emul.cc (line 429) <http://reviews.gem5.org/r/3106/#comment6133> trailing slash on /proc/self/exe/ brings up an interesting point: the test above only compares with the version w/o the trailing slash. at the very least the error message should be consistent... src/sim/syscall_emul.cc (line 434) <http://reviews.gem5.org/r/3106/#comment6132> should be free() rather than delete? might be safer just to allocate a local array of PATH_MAX on the stack - Steve Reinhardt On Sept. 18, 2015, 8 a.m., Joel Hestness wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/3106/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2015, 8 a.m.) > > > Review request for Default. > > > Repository: gem5 > > > Description > ------- > > Changeset 11092:c0a2ea0a12b2 > --------------------------- > 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 apparent > non-deterministic simulated system execution. > > > Diffs > ----- > > src/sim/process.hh 62e1504b9c64 > src/sim/process.cc 62e1504b9c64 > src/sim/syscall_emul.cc 62e1504b9c64 > > Diff: http://reviews.gem5.org/r/3106/diff/ > > > Testing > ------- > > > Thanks, > > Joel Hestness > > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
