Fix race condition in page fault notification

When a page fault cannot be resolved, the GDB monitor can get a hint about
which thread faulted by evaluating the thread state object returned by
'Cpu_session::state()'. Unfortunately, with the current implementation,
the signal which informs GDB monitor about the page fault is sent before
the thread state object of the faulted thread has been updated, so it
can happen that the faulted thread cannot be determined immediately
after receiving the signal.

With this commit, the thread state gets updated before the signal is sent.

At least on base-nova it can also happen that the thread state is not
accessible yet after receiving the page fault notification. For this
reason, GDB monitor needs to retry its query until the state is
accessible.

Fixes #1206.

repos/base-foc/src/base/pager/pager.cc

@@ -81,8 +81,6 @@ void Pager_activation_base::entry()
 				/* handle request */
 				if (obj->pager(pager)) {
 					/* could not resolv - leave thread in pagefault */
-					Lock::Guard guard(obj->state.lock);
-					obj->state.unresolved_page_fault = true;
 					PDBG("Could not resolve pf=%p ip=%p",
 					     (void*)pager.fault_addr(), (void*)pager.fault_ip());
 				} else {

repos/base-hw/include/base/pager.h

@@ -258,6 +258,13 @@ class Genode::Pager_object : public Object_pool<Pager_object>::Entry,
 		void thread_cap(Thread_capability const & c);
 
 		unsigned signal_context_id() const;
+
+
+		/*************
+		 ** Dummies **
+		 *************/
+
+		void unresolved_page_fault_occurred() { PDBG("not implemented"); }
 };
 
 class Genode::Pager_activation_base : public Thread_base,

repos/base-nova/include/base/pager.h

@@ -220,6 +220,15 @@ namespace Genode {
 			Thread_capability thread_cap() { return _thread_cap; } const
 			void thread_cap(Thread_capability cap) { _thread_cap = cap; }
 
+			/*
+			 * Note in the thread state that an unresolved page
+			 * fault occurred.
+			 */
+			void unresolved_page_fault_occurred()
+			{
+				_state.thread.unresolved_page_fault = true;
+			}
+
 			/**
 			 * Make sure nobody is in the handler anymore by doing an IPC to a
 			 * local cap pointing to same serving thread (if not running in the

repos/base-nova/src/base/pager/pager.cc

@@ -79,8 +79,6 @@ void Pager_object::_page_fault_handler()
 		}
 
 		if (ret == 1) {
-			obj->_state.thread.unresolved_page_fault = true;
-
 			char client_name[Context::NAME_LEN];
 			myself->name(client_name, sizeof(client_name));
 

repos/base-nova/src/core/platform_thread.cc

@@ -226,15 +226,18 @@ Thread_state Platform_thread::state()
 	if (!_pager) throw Cpu_session::State_access_failed();
 
 	Thread_state s;
+
 	if (_pager->copy_thread_state(&s))
 		return s;
 
-	if (is_worker())
+	if (is_worker()) {
 		s.sp = _pager->initial_esp();
-
 		return s;
 	}
 
+	throw Cpu_session::State_access_failed();
+}
+
 
 void Platform_thread::state(Thread_state s)
 {

repos/base/include/base/pager.h

@@ -109,6 +109,12 @@ namespace Genode {
 			 */
 			Thread_capability thread_cap() { return _thread_cap; } const
 			void thread_cap(Thread_capability cap) { _thread_cap = cap; }
+
+			/*
+			 * Note in the thread state that an unresolved page
+			 * fault occurred.
+			 */
+			void unresolved_page_fault_occurred();
 	};
 
 	/**

repos/base/src/base/pager/common.cc

@@ -25,3 +25,9 @@ void Pager_object::wake_up()
 	Ipc_client ipc_client(pager, &snd, &rcv);
 	ipc_client << this << IPC_CALL;
 }
+
+
+void Pager_object::unresolved_page_fault_occurred()
+{
+	state.unresolved_page_fault = true;
+}

repos/base/src/core/rm_session_component.cc

@@ -313,6 +313,8 @@ void Rm_faulter::fault(Rm_session_component *faulting_rm_session,
 
 	_faulting_rm_session = faulting_rm_session;
 	_fault_state         = fault_state;
+
+	_pager_object->unresolved_page_fault_occurred();
 }
 
 

repos/ports/src/app/gdb_monitor/gdbserver/genode-low.cc

@@ -181,21 +181,49 @@ void genode_continue_thread(unsigned long lwpid, int single_step)
 }
 
 
+/*
+ * This function returns the first thread with a page fault that it finds.
+ * Multiple page-faulted threads are currently not supported.
+ */
+
 unsigned long genode_find_segfault_lwpid()
 {
+
 	Cpu_session_component *csc = gdb_stub_thread()->cpu_session_component();
 
+	/*
+	 * It can happen that the thread state of the thread which caused the
+	 * page fault is not accessible yet. In that case, we'll retry until
+	 * it is accessible.
+	 */
+
+	while (1) {
+
 		Thread_capability thread_cap = csc->first();
 
 		while (thread_cap.valid()) {
+
+			try {
+
 				Thread_state thread_state = csc->state(thread_cap);
-		if (thread_state.unresolved_page_fault)
+
+				if (thread_state.unresolved_page_fault) {
+
+					/*
+					 * On base-foc it is necessary to pause the thread before
+					 * IP and SP are available in the thread state.
+					 */
+					csc->pause(thread_cap);
+
 					return csc->lwpid(thread_cap);
+				}
+
+			} catch (Cpu_session::State_access_failed) { }
+
 			thread_cap = csc->next(thread_cap);
 		}
 
-	PDBG("could not determine thread which caused the page fault");
-	return 1;
+	}
 }