Session::recreate_shared_mmap fixes #3863
Conversation
|
(I think my fix is correct -- however do think if there could be some scenarios where my |
src/replay_syscall.cc
Outdated
| @@ -300,6 +300,8 @@ template <typename Arch> static void prepare_clone(ReplayTask* t) { | |||
| new_task->vm()->remove_all_watchpoints(); | |||
|
|
|||
| AutoRemoteSyscalls remote(new_task); | |||
| // Note that iteration is on Task `t` while any syscalls | |||
| // via `remote` will be issued on Task `new_task` | |||
| for (const auto& m : t->vm()->maps()) { | |||
There was a problem hiding this comment.
This loop is essentially the culprit for the fix in this PR
| @@ -575,9 +585,16 @@ const AddressSpace::Mapping Session::recreate_shared_mmap( | |||
| new_map = remote.task()->vm()->mapping_of(new_addr); | |||
| if (preserved_data) { | |||
| memcpy(new_map.local_addr, preserved_data, size); | |||
| munmap(preserved_data, size); | |||
| } | |||
There was a problem hiding this comment.
This here is very problematic as it could unmap the local memory for the wrong task. We want to unmap for remote.task()Mapping's local_addr and not for Mapping m's local_addr
While unmapping a Mapping local_addr, take into account that the Mapping `m` passed in as a method parameter may come from a Task that differs from the remote.task() so the Mapping at m.map.start() for remote.task() may be different to `m`.
sidkshatriya
force-pushed
the
recreate-shared-mmap
branch
from
0da8828
to
bd80c42
Compare
|
The build failure on aarch64 is due to something unrelated. See #3865 (comment) |
|
Done -- ready for further review/merge. There is a single failure in x86_64 And then rr itself crashes due to SIGSEGV (see test transcript). |
While unmapping a Mapping
local_addr, take into account that the Mappingmpassed in as a method parameter may come from a Task that differs from the remote.task() so the Mapping atm.map.start()forremote.task()may be different tom.