monitor: add memory write support

Fixes #4947

repos/os/run/monitor_gdb.inc

@@ -59,7 +59,7 @@ install_config {
 			<default caps="100"/>
 
 			<monitor>
-				<policy label="first-test-log"/>
+				<policy label="first-test-log" wx="yes"/>
 			</monitor>
 
 			<start name="first-test-log">

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"
+}

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 };
 };
 

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> _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>  _read_job { };
+			Constructible<Write_job> _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> _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))
+			return _with_curr_view_at(pd, at,
-				_curr_view.destruct();
+			                          [&] (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);
+		 * Write memory from buffer 'src' into inferior 'pd' at address 'at'
-				try { _curr_view.construct(_env.rm(), pd, offset); }
+		 *
-				catch (Region_map::Region_conflict) {
+		 * The 'src.num_bytes' value denotes the number of bytes to write.
-					warning("attempt to read outside the virtual address space: ",
+		 *
-					        Hex(at.value));
+		 * \return  number of successfully written bytes, which can be smaller
-					return 0;
+		 *          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)
-			/* give up after 100 milliseconds */
+		{
-			_watchdog_timer.trigger_once(1000*100);
+			return _with_curr_view_at(pd, at,
-
+			                          [&] (Curr_view &curr_view) -> size_t {
-			/* drain pending signals to avoid spurious watchdog timeouts */
+				return _with_watched_page_faults(pd,
-			while (_env.ep().dispatch_pending_io_signal());
+				                                 [&] (auto const &block_fn) -> size_t {
-
+					return _probe->write(curr_view, at, src, block_fn);
-			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;
 		}
 };
 

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);

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_ */