Kernel: Threads cannot take arguments anymore

kernel/arch/i386/Thread.S

@@ -5,23 +5,18 @@ read_rip:
 	jmp *%eax
 
 exit_thread_trampoline:
-	addl $16, %esp
-	popl %eax
-	pushl $0x696969
-	pushl %eax
+	addl $4, %esp
+	pushl (%esp)
 	ret
 
-# void start_thread(uint32_t arg0, uint32_t arg1, uint32_t arg2, uint32_t arg3, uint32_t rsp, uint32_t rip)
+# void start_thread(uint32_t function, uint32_t esp, uint32_t eip)
 .global start_thread
 start_thread:
-	movl %esp, %eax
-	movl 24(%eax), %ecx
-	movl 20(%eax), %esp
+	movl 4(%esp), %eax
+	movl 12(%esp), %ecx
+	movl 8(%esp), %esp
 
-	pushl 16(%eax)
-	pushl 12(%eax)
-	pushl 8(%eax)
-	pushl 4(%eax)
+	pushl %eax
 	pushl $exit_thread_trampoline
 
 	movl $0, %ebp

kernel/arch/x86_64/Thread.S

@@ -8,14 +8,14 @@ exit_thread_trampoline:
 	movq 8(%rsp), %rdi
 	ret
 
-# void start_thread(uint64_t arg0, uint64_t arg1, uint64_t arg2, uint64_t arg3, uint64_t rsp, uint64_t rip)
+# void start_thread(uint64_t function, uint64_t rsp, uint64_t rip)
 .global start_thread
 start_thread:
-	movq %r8, %rsp
+	movq %rsi, %rsp
 	movq $0, %rbp
 	pushq $exit_thread_trampoline
 	sti
-	jmp *%r9
+	jmp *%rdx
 
 # void continue_thread(uint64_t rsp, uint64_t rip)
 .global continue_thread

kernel/include/kernel/Scheduler.h

@@ -17,18 +17,8 @@ namespace Kernel
 		static Scheduler& get();
 
 		const Thread& current_thread() const;
-
-		template<typename... Args>
-		BAN::ErrorOr<void> add_thread(const BAN::Function<void(Args...)>& func, Args... args)
-		{
-			uintptr_t flags;
-			asm volatile("pushf; pop %0" : "=r"(flags));
-			asm volatile("cli");
-			TRY(m_threads.emplace_back(func, BAN::forward<Args>(args)...));
-			if (flags & (1 << 9))
-				asm volatile("sti");
-			return {};
-		}
+		
+		BAN::ErrorOr<void> add_thread(const BAN::Function<void()>& function);
 
 		void reschedule();
 		void set_current_thread_sleeping();

kernel/include/kernel/Thread.h

@@ -22,19 +22,10 @@ namespace Kernel
 		};
 
 	public:
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wpmf-conversions"
-		template<typename... Args>
-		Thread(const BAN::Function<void(Args...)>& func, Args... args)
-			: Thread((uintptr_t)(void*)&BAN::Function<void(Args...)>::operator(), (uintptr_t)&func, ((uintptr_t)args)...)
-		{
-			static_assert(((BAN::is_integral_v<Args> || BAN::is_pointer_v<Args>) && ...));
-		}
-#pragma GCC diagnostic pop
-
+		Thread(const BAN::Function<void()>&);
 		~Thread();
 
-		uint32_t id() const { return m_id; }
+		uint32_t tid() const { return m_tid; }
 
 		void set_rsp(uintptr_t rsp) { m_rsp = rsp; }
 		void set_rip(uintptr_t rip) { m_rip = rip; }
@@ -43,21 +34,19 @@ namespace Kernel
 		uintptr_t rip() const { return m_rip; }
 		State state() const { return m_state; }
 
-		const uintptr_t* args() const { return m_args; }
+		const BAN::Function<void()>* function() const { return &m_function; }
 
 	private:
-		Thread(uintptr_t rip, uintptr_t func, uintptr_t arg1 = 0, uintptr_t arg2 = 0, uintptr_t arg3 = 0);
 		void on_exit();
 		
 	private:
 		void*			m_stack_base	= nullptr;
 		State			m_state			= State::NotStarted;
-		uintptr_t		m_args[4]		= {};
 		uintptr_t		m_rip			= 0;
 		uintptr_t		m_rsp			= 0;
-		const uint32_t	m_id			= 0;
+		const uint32_t	m_tid			= 0;
 		
-		alignas(max_align_t) uint8_t m_function[BAN::Function<void()>::size()] { 0 };
+		BAN::Function<void()> m_function;
 	};
 
 }

kernel/kernel/Scheduler.cpp

@@ -7,7 +7,7 @@ namespace Kernel
 
 	static Scheduler* s_instance = nullptr;
 
-	extern "C" void start_thread(uintptr_t arg0, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3, uintptr_t rsp, uintptr_t rip);
+	extern "C" void start_thread(const BAN::Function<void()>* function, uintptr_t rsp, uintptr_t rip);
 	extern "C" void continue_thread(uintptr_t rsp, uintptr_t rip);
 	extern "C" uintptr_t read_rip();
 
@@ -30,6 +30,18 @@ namespace Kernel
 		return *m_current_iterator;
 	}
 
+
+	BAN::ErrorOr<void> Scheduler::add_thread(const BAN::Function<void()>& function)
+	{
+		uintptr_t flags;
+		asm volatile("pushf; pop %0" : "=r"(flags));
+		asm volatile("cli");
+		TRY(m_threads.emplace_back(function));
+		if (flags & (1 << 9))
+			asm volatile("sti");
+		return {};
+	}
+
 	void Scheduler::reschedule()
 	{
 		ASSERT(InterruptController::get().is_in_service(PIT_IRQ));
@@ -62,9 +74,6 @@ namespace Kernel
 		
 		Thread& current = *m_current_iterator;
 
-		//if (m_threads.size() == 2 && current.id() != 0 && current.state() == Thread::State::Running)
-		//	return;
-
 		if (current.state() == Thread::State::Done)
 		{
 			m_threads.remove(m_current_iterator);
@@ -91,7 +100,7 @@ namespace Kernel
 		{
 			case Thread::State::NotStarted:
 				next.set_state(Thread::State::Running);
-				start_thread(next.args()[0], next.args()[1], next.args()[2], next.args()[3], next.rsp(), next.rip());
+				start_thread(next.function(), next.rsp(), next.rip());
 				break;
 			case Thread::State::Paused:
 				next.set_state(Thread::State::Running);
@@ -123,8 +132,7 @@ namespace Kernel
 		ASSERT(current.state() == Thread::State::NotStarted);
 		current.set_state(Thread::State::Running);
 
-		const uintptr_t* args = current.args();
-		start_thread(args[0], args[1], args[2], args[3], current.rsp(), current.rip());
+		start_thread(current.function(), current.rsp(), current.rip());
 
 		ASSERT(false);
 	}

kernel/kernel/Shell.cpp

@@ -180,21 +180,18 @@ argument_done:
 		{
 			static SpinLock s_thread_spinlock;
 
-			// NOTE: This is a workaround to pass values as copies to threads.
-			//       I have only implemented passing integer and pointers.
-			//       We don't continue execution until the thread has unlocked
-			//       the spinlock.
 			s_thread_spinlock.lock();
-			MUST(Scheduler::get().add_thread(Function<void(const Vector<String>*)>(
-				[this] (const Vector<String>* args_ptr)
+
+			MUST(Scheduler::get().add_thread(Function<void()>(
+				[this, &arguments]
 				{
-					auto args = *args_ptr;
-					s_thread_spinlock.unlock();
+					auto args = arguments;
 					args.remove(0);
+					s_thread_spinlock.unlock();
 					PIT::sleep(5000);
 					process_command(args);
 				}
-			), &arguments));
+			)));
 
 			while (s_thread_spinlock.is_locked());
 		}

kernel/kernel/Thread.cpp

@@ -10,7 +10,7 @@
 namespace Kernel
 {
 
-	static uint32_t s_next_id = 0;
+	static uint32_t s_next_tid = 0;
 
 	static constexpr size_t thread_stack_size = 16384;
 
@@ -21,26 +21,19 @@ namespace Kernel
 		memcpy((void*)rsp, (void*)&value, size);
 	}
 
-	Thread::Thread(uintptr_t rip, uintptr_t func, uintptr_t arg1, uintptr_t arg2, uintptr_t arg3)
-		: m_id(s_next_id++)
+	Thread::Thread(const BAN::Function<void()>& function)
+		: m_tid(s_next_tid++)
+		, m_function(function)
 	{
 		m_stack_base = kmalloc(thread_stack_size, PAGE_SIZE);
 		ASSERT(m_stack_base);
-	
-		m_rsp = (uintptr_t)m_stack_base + thread_stack_size;
-		m_rip = rip;
-		m_args[1] = arg1;
-		m_args[2] = arg2;
-		m_args[3] = arg3;
 
-		// NOTE: in System V ABI arg0 is the pointer to 'this'
-		//       we copy the function object to Thread object
-		//       so we can ensure the lifetime of it. We store
-		//       it as raw bytes so that Thread can be non-templated.
-		//       This requires BAN::Function to be trivially copyable
-		//       but for now it should be.
-		memcpy(m_function, (void*)func, sizeof(m_function));
-		m_args[0] = (uintptr_t)m_function;
+		m_rsp = (uintptr_t)m_stack_base + thread_stack_size;
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wpmf-conversions"
+		m_rip = (uintptr_t)(void*)&BAN::Function<void()>::operator();
+#pragma GCC diagnostic pop
 
 		write_to_stack<sizeof(void*)>(m_rsp, this);
 		write_to_stack<sizeof(void*)>(m_rsp, &Thread::on_exit);