From 02209e5455634f7c2afd8034ce13e384d626e14c Mon Sep 17 00:00:00 2001 From: Christian Prochaska Date: Thu, 29 Jun 2023 16:01:06 +0200 Subject: [PATCH] monitor: add memory write support Fixes #4947 --- repos/os/run/monitor_gdb.inc | 2 +- repos/os/run/monitor_gdb.run | 14 ++ repos/os/src/monitor/gdb_stub.h | 49 ++++- repos/os/src/monitor/memory_accessor.h | 183 +++++++++++++----- repos/os/src/test/monitor/main.cc | 22 +++ .../os/src/test/monitor/monitor_controller.h | 37 ++++ 6 files changed, 258 insertions(+), 49 deletions(-) diff --git a/repos/os/run/monitor_gdb.inc b/repos/os/run/monitor_gdb.inc index 47a97cbfe0..5d75820a83 100644 --- a/repos/os/run/monitor_gdb.inc +++ b/repos/os/run/monitor_gdb.inc @@ -59,7 +59,7 @@ install_config { - + diff --git a/repos/os/run/monitor_gdb.run b/repos/os/run/monitor_gdb.run index 05e541149c..922159ddf9 100644 --- a/repos/os/run/monitor_gdb.run +++ b/repos/os/run/monitor_gdb.run @@ -21,3 +21,17 @@ run_genode_until {\(gdb\)} 20 $gdb_id if {![regexp {0x1003e8b:\t0x6f636e6f} $output]} { puts stderr "*** Error: Dumped memory is not as expected" } + +puts "" +puts "----- test: write memory -----" +puts "" + +send "set {int} 0x1003e8b = 0x12345678\n" +run_genode_until {\(gdb\)} 20 $gdb_id + +send "x/x 0x1003e8b\n" +run_genode_until {\(gdb\)} 20 $gdb_id + +if {![regexp {0x1003e8b:\t0x12345678} $output]} { + puts stderr "*** Error: Modified memory is not as expected" +} diff --git a/repos/os/src/monitor/gdb_stub.h b/repos/os/src/monitor/gdb_stub.h index 849b1116d3..3cb978cbca 100644 --- a/repos/os/src/monitor/gdb_stub.h +++ b/repos/os/src/monitor/gdb_stub.h @@ -88,6 +88,15 @@ struct Monitor::Gdb::State : Noncopyable return 0; } + size_t write_memory(Memory_accessor::Virt_addr at, Const_byte_range_ptr const &src) + { + if (_current.constructed()) + return _memory_accessor.write(_current->pd, at, src); + + warning("attempt to write memory without a current target"); + return 0; + } + bool current_defined() const { return _current.constructed(); } void current(Inferiors::Id pid, Threads::Id tid) @@ -454,6 +463,43 @@ struct m : Command_without_separator }; +/** + * Write memory (binary data) + */ +struct X : Command_without_separator +{ + X(Commands &c) : Command_without_separator(c, "X") { } + + void execute(State &state, Const_byte_range_ptr const &args, Output &out) const override + { + addr_t const addr = comma_separated_hex_value(args, 0, addr_t(0)); + size_t const len = comma_separated_hex_value(args, 1, 0UL); + + if (len == 0) { + /* packet support probing */ + gdb_ok(out); + return; + } + + size_t written_num_bytes { 0 }; + + with_argument(args, Sep {':'}, 1, [&] (Const_byte_range_ptr const &arg) { + + if (arg.num_bytes != len) + return; + + written_num_bytes = + state.write_memory(Memory_accessor::Virt_addr { addr }, arg); + }); + + if (written_num_bytes == len) + gdb_ok(out); + else + gdb_error(out, 1); + } +}; + + /** * Query liveliness of thread ID */ @@ -528,7 +574,8 @@ struct Monitor::Gdb::Supported_commands : Commands Cmd::m, Cmd::D, Cmd::T, - Cmd::ask + Cmd::ask, + Cmd::X > _instances { *this }; }; diff --git a/repos/os/src/monitor/memory_accessor.h b/repos/os/src/monitor/memory_accessor.h index 8bc3da58bf..a122cdb55c 100644 --- a/repos/os/src/monitor/memory_accessor.h +++ b/repos/os/src/monitor/memory_accessor.h @@ -50,7 +50,7 @@ class Monitor::Memory_accessor : Noncopyable Inferior_pd &_pd; addr_t const _offset; - struct { uint8_t const *_local_ptr; }; + struct { uint8_t * const _local_ptr; }; Curr_view(Region_map &local_rm, Inferior_pd &pd, addr_t offset) : @@ -119,7 +119,40 @@ class Monitor::Memory_accessor : Noncopyable } }; - Constructible _read_job { }; + /* exists only temporary during a 'Memory_accessor::write' call */ + struct Write_job + { + Curr_view &curr_view; + Virt_addr at; + Const_byte_range_ptr const &src; + + size_t pos = 0; /* number of completed bytes */ + bool done = 0; /* true if 'execute_may_fault' survived */ + + Write_job(Curr_view &curr_view, Virt_addr at, Const_byte_range_ptr const &src) + : + curr_view(curr_view), at(at), src(src) + { } + + /* called only once */ + void execute_may_fault() + { + /* offset from the start of the window */ + addr_t const window_pos = at.value - curr_view._offset; + addr_t const num_bytes_at_window_pos = WINDOW_SIZE - window_pos; + size_t const len = min(src.num_bytes, num_bytes_at_window_pos); + + uint8_t * const dst_ptr = curr_view._local_ptr; + + for (pos = 0; pos < len; pos++) + dst_ptr[window_pos + pos] = src.start[pos]; + + done = true; + } + }; + + Constructible _read_job { }; + Constructible _write_job { }; /** * Thread interface @@ -134,6 +167,11 @@ class Monitor::Memory_accessor : Noncopyable /* wakeup 'Memory_accessor::read' via I/O signal */ _probe_response_handler.local_submit(); + } else if (_write_job.constructed()) { + _write_job->execute_may_fault(); + + /* wakeup 'Memory_accessor::write' via I/O signal */ + _probe_response_handler.local_submit(); } } } @@ -161,6 +199,22 @@ class Monitor::Memory_accessor : Noncopyable return result; } + + size_t write(Curr_view &curr_view, + Virt_addr at, Const_byte_range_ptr const &src, auto const &block_fn) + { + _write_job.construct(curr_view, at, src); + _blockade.wakeup(); + + /* block until write is done or a page fault occurred */ + while (!_write_job->done && block_fn()); + + size_t const result = _write_job->pos; + + _write_job.destruct(); + + return result; + } }; Constructible _probe { }; @@ -174,6 +228,60 @@ class Monitor::Memory_accessor : Noncopyable void _handle_timeout() { _timeout_count++; } + size_t _with_watched_page_faults(Inferior_pd &pd, auto const &fn) + { + /* give up after 100 milliseconds */ + _watchdog_timer.trigger_once(1000*100); + + /* drain pending signals to avoid spurious watchdog timeouts */ + while (_env.ep().dispatch_pending_io_signal()); + + unsigned const orig_page_fault_count = pd.page_fault_count(); + unsigned const orig_timeout_count = _timeout_count; + + if (!_probe.constructed()) + _probe.construct(_env, _probe_response_handler); + + auto fault_or_timeout_occurred = [&] { + return (orig_page_fault_count != pd.page_fault_count()) + || (orig_timeout_count != _timeout_count); }; + + auto block_fn = [&] + { + if (fault_or_timeout_occurred()) + return false; /* cancel operation */ + + _env.ep().wait_and_dispatch_one_io_signal(); + return true; /* keep trying */ + }; + + size_t const result = fn(block_fn); + + /* wind down the faulted probe thread, spawn a fresh one on next call */ + if (fault_or_timeout_occurred()) + _probe.destruct(); + + return result; + } + + size_t _with_curr_view_at(Inferior_pd &pd, Virt_addr at, auto const &fn) + { + if (_curr_view.constructed() && !_curr_view->contains(pd, at)) + _curr_view.destruct(); + + if (!_curr_view.constructed()) { + addr_t const offset = at.value & ~(WINDOW_SIZE - 1); + try { _curr_view.construct(_env.rm(), pd, offset); } + catch (Region_map::Region_conflict) { + warning("attempt to access memory outside the virtual address space: ", + Hex(at.value)); + return 0; + } + } + + return fn(*_curr_view); + } + public: Memory_accessor(Env &env) : _env(env) @@ -194,52 +302,33 @@ class Monitor::Memory_accessor : Noncopyable */ size_t read(Inferior_pd &pd, Virt_addr at, Byte_range_ptr const &dst) { - if (_curr_view.constructed() && !_curr_view->contains(pd, at)) - _curr_view.destruct(); + return _with_curr_view_at(pd, at, + [&] (Curr_view &curr_view) -> size_t { + return _with_watched_page_faults(pd, + [&] (auto const &block_fn) -> size_t { + return _probe->read(curr_view, at, dst, block_fn); + }); + }); + } - if (!_curr_view.constructed()) { - addr_t const offset = at.value & ~(WINDOW_SIZE - 1); - try { _curr_view.construct(_env.rm(), pd, offset); } - catch (Region_map::Region_conflict) { - warning("attempt to read outside the virtual address space: ", - Hex(at.value)); - return 0; - } - } - - /* give up after 100 milliseconds */ - _watchdog_timer.trigger_once(1000*100); - - /* drain pending signals to avoid spurious watchdog timeouts */ - while (_env.ep().dispatch_pending_io_signal()); - - unsigned const orig_page_fault_count = pd.page_fault_count(); - unsigned const orig_timeout_count = _timeout_count; - - if (!_probe.constructed()) - _probe.construct(_env, _probe_response_handler); - - auto fault_or_timeout_occurred = [&] { - return (orig_page_fault_count != pd.page_fault_count()) - || (orig_timeout_count != _timeout_count); }; - - auto block_fn = [&] - { - if (fault_or_timeout_occurred()) - return false; /* cancel read */ - - _env.ep().wait_and_dispatch_one_io_signal(); - return true; /* keep trying */ - }; - - size_t const read_num_bytes = - _probe->read(*_curr_view, at, dst, block_fn); - - /* wind down the faulted probe thread, spawn a fresh one on next call */ - if (fault_or_timeout_occurred()) - _probe.destruct(); - - return read_num_bytes; + /** + * Write memory from buffer 'src' into inferior 'pd' at address 'at' + * + * The 'src.num_bytes' value denotes the number of bytes to write. + * + * \return number of successfully written bytes, which can be smaller + * than 'src.num_bytes' when encountering the bounds of + * mapped memory in the inferior's address space. + */ + size_t write(Inferior_pd &pd, Virt_addr at, Const_byte_range_ptr const &src) + { + return _with_curr_view_at(pd, at, + [&] (Curr_view &curr_view) -> size_t { + return _with_watched_page_faults(pd, + [&] (auto const &block_fn) -> size_t { + return _probe->write(curr_view, at, src, block_fn); + }); + }); } }; diff --git a/repos/os/src/test/monitor/main.cc b/repos/os/src/test/monitor/main.cc index 5894d512f9..ee88b57d58 100644 --- a/repos/os/src/test/monitor/main.cc +++ b/repos/os/src/test/monitor/main.cc @@ -185,6 +185,27 @@ struct Test::Main "unexpected content at patched address"); } + void test_write_memory() + { + log("-- test_write_memory --"); + char const * const s = "Trying to modify this pattern "; + char const * const s_new = "Trying to read back this pattern"; + size_t const num_bytes = strlen(s); + char buffer[num_bytes] { }; + + assert(_monitor.write_memory((addr_t)s, + Const_byte_range_ptr(s_new, num_bytes)), + "unable to write string of ", num_bytes, " bytes"); + + size_t const read_bytes = + _monitor.read_memory((addr_t)s, Byte_range_ptr(buffer, sizeof(buffer))); + + assert(read_bytes == num_bytes, + "unable to read string of ", num_bytes, " bytes"); + assert(equal(Const_byte_range_ptr(buffer, read_bytes), s_new), + "read bytes don't match expected pattern"); + } + Main(Env &env) : _env(env) { test_query_threads(); @@ -192,6 +213,7 @@ struct Test::Main test_read_memory(); test_truncated_mapping(); test_writeable_text_segment(); + test_write_memory(); test_bench(); _env.parent().exit(0); diff --git a/repos/os/src/test/monitor/monitor_controller.h b/repos/os/src/test/monitor/monitor_controller.h index e2cf77fc0a..f1ed38fc7d 100644 --- a/repos/os/src/test/monitor/monitor_controller.h +++ b/repos/os/src/test/monitor/monitor_controller.h @@ -109,6 +109,32 @@ class Monitor::Controller : Noncopyable } } + bool _response_ok() + { + bool ok = false; + _with_response([&] (Const_byte_range_ptr const &response) { + if ((response.num_bytes == 2) && + (response.start[0] == 'O') && + (response.start[1] == 'K')) + ok = true; + }); + return ok; + } + + struct Gdb_binary_buffer + { + Const_byte_range_ptr const &src; + + Gdb_binary_buffer(Const_byte_range_ptr const &src) + : src(src) { } + + void print(Output &output) const + { + for (size_t i = 0; i < src.num_bytes; i++) + Genode::print(output, Char(src.start[i])); + } + }; + public: Controller(Env &env) : _env(env) @@ -187,6 +213,17 @@ class Monitor::Controller : Noncopyable }); return read_bytes; } + + /** + * Write memory 'at' of current inferior + */ + bool write_memory(addr_t at, Const_byte_range_ptr const &src) + { + _request("X", Gdb_hex(at), ",", Gdb_hex(src.num_bytes), ":", + Gdb_binary_buffer(src)); + + return _response_ok(); + } }; #endif /* _MONITOR_CONTROLLER_H_ */