Attention is currently required from: Matt Sinclair.
Hello kokoro, Matt Sinclair,

I'd like you to do a code review.
Please visit

    https://gem5-review.googlesource.com/c/public/gem5/+/48805

to review the following change.


Change subject: sim-se: Fix execve syscall
......................................................................

sim-se: Fix execve syscall

There were three things preventing execve from working

Firstly, the entrypoint for the new program wasn't correct. This was
fixed by calling Process::init, which adds a bias to the entrypoint.

Secondly, the uname string wasn't being copied over. This meant when the
new executable tried to run, it would think the kernel was too old to
run on, and would error out. This was fixed by copying over the uname
string (the `release` string in Process) when creating the new process.

Additionally, this patch also ensures we copy over the uname string in
the clone implementation, as otherwise a cloned thread that called
execve would crash.

Finally, we choose to not delete the new ProcessParams or the old
Process. This is done both because it matches what is done in cloneFunc,
but also because deleting the old process results in a segfault later
on.

Change-Id: I4ca201da689e9e37671b4cb477dc76fa12eecf69
Reviewed-on: https://gem5-review.googlesource.com/c/public/gem5/+/48345
Reviewed-by: Matt Sinclair <[email protected]>
Reviewed-by: Bobby R. Bruce <[email protected]>
Maintainer: Matt Sinclair <[email protected]>
Tested-by: kokoro <[email protected]>
---
M src/sim/syscall_emul.hh
1 file changed, 6 insertions(+), 2 deletions(-)



diff --git a/src/sim/syscall_emul.hh b/src/sim/syscall_emul.hh
index 522fb1b..c450cad 100644
--- a/src/sim/syscall_emul.hh
+++ b/src/sim/syscall_emul.hh
@@ -1453,6 +1453,7 @@
     pp->euid = p->euid();
     pp->gid = p->gid();
     pp->egid = p->egid();
+    pp->release = p->release;

     /* Find the first free PID that's less than the maximum */
     std::set<int> const& pids = p->system->PIDs;
@@ -2018,6 +2019,7 @@
     pp->errout.assign("cerr");
     pp->cwd.assign(p->tgtCwd);
     pp->system = p->system;
+    pp->release = p->release;
     /**
      * Prevent process object creation with identical PIDs (which will trip
* a fatal check in Process constructor). The execve call is supposed to
@@ -2028,7 +2030,9 @@
      */
     p->system->PIDs.erase(p->pid());
     Process *new_p = pp->create();
-    delete pp;
+ // TODO: there is no way to know when the Process SimObject is done with
+    // the params pointer. Both the params pointer (pp) and the process
+    // pointer (p) are normally managed in python and are never cleaned up.

     /**
      * Work through the file descriptor array and close any files marked
@@ -2043,10 +2047,10 @@

     *new_p->sigchld = true;

-    delete p;
     tc->clearArchRegs();
     tc->setProcessPtr(new_p);
     new_p->assignThreadContext(tc->contextId());
+    new_p->init();
     new_p->initState();
     tc->activate();
     TheISA::PCState pcState = tc->pcState();

--
To view, visit https://gem5-review.googlesource.com/c/public/gem5/+/48805
To unsubscribe, or for help writing mail filters, visit https://gem5-review.googlesource.com/settings

Gerrit-Project: public/gem5
Gerrit-Branch: develop
Gerrit-Change-Id: I4ca201da689e9e37671b4cb477dc76fa12eecf69
Gerrit-Change-Number: 48805
Gerrit-PatchSet: 1
Gerrit-Owner: Bobby R. Bruce <[email protected]>
Gerrit-Reviewer: Matt Sinclair <[email protected]>
Gerrit-Reviewer: kokoro <[email protected]>
Gerrit-CC: Kyle Roarty <[email protected]>
Gerrit-Attention: Matt Sinclair <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
gem5-dev mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

Reply via email to