diff options
author | Ben Wiederhake <BenWiederhake.GitHub@gmx.de> | 2020-08-09 01:08:24 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2020-08-10 11:51:45 +0200 |
commit | bee08a4b9ff0f2c269a4e3a8159b689fbb8d1ab8 (patch) | |
tree | 7963eede850be6fcf18ac8b3aae30e72a8d30407 | |
parent | 7bdf54c8372d6951409fae17dd4cec7eea6573a2 (diff) | |
download | serenity-bee08a4b9ff0f2c269a4e3a8159b689fbb8d1ab8.zip |
Kernel: More PID/TID typing
-rw-r--r-- | Kernel/API/Syscall.h | 2 | ||||
-rw-r--r-- | Kernel/FileSystem/ProcFS.cpp | 4 | ||||
-rw-r--r-- | Kernel/Process.cpp | 2 | ||||
-rw-r--r-- | Kernel/Process.h | 19 | ||||
-rw-r--r-- | Kernel/Profiling.cpp | 7 | ||||
-rw-r--r-- | Kernel/Profiling.h | 6 | ||||
-rw-r--r-- | Kernel/Ptrace.cpp | 8 | ||||
-rw-r--r-- | Kernel/Scheduler.cpp | 5 | ||||
-rw-r--r-- | Kernel/Syscalls/execve.cpp | 2 | ||||
-rw-r--r-- | Kernel/Syscalls/kill.cpp | 5 | ||||
-rw-r--r-- | Kernel/Syscalls/sched.cpp | 22 | ||||
-rw-r--r-- | Kernel/Syscalls/thread.cpp | 10 | ||||
-rw-r--r-- | Kernel/Thread.cpp | 7 | ||||
-rw-r--r-- | Kernel/Thread.h | 2 | ||||
-rw-r--r-- | Kernel/init.cpp | 2 | ||||
-rw-r--r-- | Libraries/LibC/serenity.cpp | 5 | ||||
-rw-r--r-- | Libraries/LibC/serenity.h | 2 | ||||
-rw-r--r-- | Libraries/LibC/sys/ptrace.cpp | 4 | ||||
-rw-r--r-- | Libraries/LibC/sys/ptrace.h | 5 | ||||
-rw-r--r-- | Libraries/LibCore/ProcessStatisticsReader.h | 2 | ||||
-rw-r--r-- | Libraries/LibThread/Lock.h | 6 |
21 files changed, 67 insertions, 60 deletions
diff --git a/Kernel/API/Syscall.h b/Kernel/API/Syscall.h index 7fae203cdc..5598e609bd 100644 --- a/Kernel/API/Syscall.h +++ b/Kernel/API/Syscall.h @@ -440,7 +440,7 @@ struct SC_stat_params { struct SC_ptrace_params { int request; - pid_t pid; + pid_t tid; Userspace<u8*> addr; int data; }; diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 12250da5c5..02672ca828 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -445,7 +445,7 @@ Optional<KBuffer> procfs$profile(InodeIdentifier) KBufferBuilder builder; JsonObjectSerializer object(builder); - object.add("pid", Profiling::pid()); + object.add("pid", Profiling::pid().value()); object.add("executable", Profiling::executable_path()); auto array = object.add_array("events"); @@ -453,7 +453,7 @@ Optional<KBuffer> procfs$profile(InodeIdentifier) Profiling::for_each_sample([&](auto& sample) { auto object = array.add_object(); object.add("type", "sample"); - object.add("tid", sample.tid); + object.add("tid", sample.tid.value()); object.add("timestamp", sample.timestamp); auto frames_array = object.add_array("stack"); for (size_t i = 0; i < Profiling::max_stack_frame_count; ++i) { diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 4a6eefb07c..d286949cb0 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -834,7 +834,7 @@ KBuffer Process::backtrace() const { KBufferBuilder builder; for_each_thread([&](Thread& thread) { - builder.appendf("Thread %d (%s):\n", thread.tid(), thread.name().characters()); + builder.appendf("Thread %d (%s):\n", thread.tid().value(), thread.name().characters()); builder.append(thread.backtrace()); return IterationDecision::Continue; }); diff --git a/Kernel/Process.h b/Kernel/Process.h index f06af2732b..e84d140913 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -202,8 +202,8 @@ public: int sys$dbgputch(u8); int sys$dbgputstr(Userspace<const u8*>, int length); int sys$dump_backtrace(); - int sys$gettid(); - int sys$donate(int tid); + pid_t sys$gettid(); + int sys$donate(pid_t tid); int sys$ftruncate(int fd, off_t); pid_t sys$setsid(); pid_t sys$getsid(pid_t); @@ -306,10 +306,10 @@ public: int sys$sched_getparam(pid_t pid, Userspace<struct sched_param*>); int sys$create_thread(void* (*)(void*), Userspace<const Syscall::SC_create_thread_params*>); void sys$exit_thread(void*); - int sys$join_thread(int tid, void** exit_value); - int sys$detach_thread(int tid); - int sys$set_thread_name(int tid, const char* buffer, size_t buffer_size); - int sys$get_thread_name(int tid, char* buffer, size_t buffer_size); + int sys$join_thread(pid_t tid, void** exit_value); + int sys$detach_thread(pid_t tid); + int sys$set_thread_name(pid_t tid, const char* buffer, size_t buffer_size); + int sys$get_thread_name(pid_t tid, char* buffer, size_t buffer_size); int sys$rename(Userspace<const Syscall::SC_rename_params*>); int sys$mknod(Userspace<const Syscall::SC_mknod_params*>); int sys$shbuf_create(int, void** buffer); @@ -330,7 +330,7 @@ public: int sys$profiling_enable(pid_t); int sys$profiling_disable(pid_t); int sys$futex(Userspace<const Syscall::SC_futex_params*>); - int sys$set_thread_boost(int tid, int amount); + int sys$set_thread_boost(pid_t tid, int amount); int sys$set_process_boost(pid_t, int amount); int sys$chroot(const char* path, size_t path_length, int mount_flags); int sys$pledge(Userspace<const Syscall::SC_pledge_params*>); @@ -597,7 +597,7 @@ private: void disown_all_shared_buffers(); KResult do_kill(Process&, int signal); - KResult do_killpg(pid_t pgrp, int signal); + KResult do_killpg(ProcessGroupID pgrp, int signal); KResult do_killall(int signal); KResult do_killself(int signal); @@ -739,8 +739,7 @@ inline void Process::for_each_child(Callback callback) ScopedSpinLock lock(g_processes_lock); for (auto* process = g_processes->head(); process;) { auto* next_process = process->next(); - // FIXME: PID/TID BUG - if (process->ppid() == my_pid || process->has_tracee_thread(m_pid.value())) { + if (process->ppid() == my_pid || process->has_tracee_thread(m_pid)) { if (callback(*process) == IterationDecision::Break) break; } diff --git a/Kernel/Profiling.cpp b/Kernel/Profiling.cpp index ca817afa19..2765b59d34 100644 --- a/Kernel/Profiling.cpp +++ b/Kernel/Profiling.cpp @@ -40,7 +40,7 @@ namespace Profiling { static KBufferImpl* s_profiling_buffer; static size_t s_slot_count; static size_t s_next_slot_index; -static u32 s_pid; +static ProcessID s_pid { -1 }; String& executable_path() { @@ -50,7 +50,7 @@ String& executable_path() return *path; } -u32 pid() +ProcessID pid() { return s_pid; } @@ -61,7 +61,7 @@ void start(Process& process) executable_path() = process.executable()->absolute_path().impl(); else executable_path() = {}; - s_pid = process.pid().value(); // FIXME: PID/TID INCOMPLETE + s_pid = process.pid(); if (!s_profiling_buffer) { s_profiling_buffer = RefPtr<KBufferImpl>(KBuffer::create_with_size(8 * MB).impl()).leak_ref(); @@ -87,6 +87,7 @@ Sample& next_sample_slot() void stop() { + // FIXME: This probably shouldn't be empty. } void did_exec(const String& new_executable_path) diff --git a/Kernel/Profiling.h b/Kernel/Profiling.h index f3ffb441f4..fa7d682b68 100644 --- a/Kernel/Profiling.h +++ b/Kernel/Profiling.h @@ -39,13 +39,13 @@ namespace Profiling { constexpr size_t max_stack_frame_count = 50; struct Sample { - i32 pid; - i32 tid; + ProcessID pid; + ThreadID tid; u64 timestamp; u32 frames[max_stack_frame_count]; }; -extern u32 pid(); +extern ProcessID pid(); extern String& executable_path(); Sample& next_sample_slot(); diff --git a/Kernel/Ptrace.cpp b/Kernel/Ptrace.cpp index 2e38b48c00..ab085c3054 100644 --- a/Kernel/Ptrace.cpp +++ b/Kernel/Ptrace.cpp @@ -43,13 +43,17 @@ KResultOr<u32> handle_syscall(const Kernel::Syscall::SC_ptrace_params& params, P return KSuccess; } - if (params.pid == caller.pid().value()) + // FIXME: PID/TID BUG + // This bug allows to request PT_ATTACH (or anything else) the same process, as + // long it is not the main thread. Alternatively, if this is desired, then the + // bug is that this prevents PT_ATTACH to the main thread from another thread. + if (params.tid == caller.pid().value()) return KResult(-EINVAL); Thread* peer = nullptr; { InterruptDisabler disabler; - peer = Thread::from_tid(params.pid); + peer = Thread::from_tid(params.tid); } if (!peer) return KResult(-ESRCH); diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index bf84598b7a..dcfdd4074c 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -684,9 +684,8 @@ void Scheduler::timer_tick(const RegisterState& regs) SmapDisabler disabler; auto backtrace = current_thread->raw_backtrace(regs.ebp, regs.eip); auto& sample = Profiling::next_sample_slot(); - // FIXME: PID/TID INCOMPLETE - sample.pid = current_thread->process().pid().value(); - sample.tid = current_thread->tid().value(); + sample.pid = current_thread->process().pid(); + sample.tid = current_thread->tid(); sample.timestamp = g_uptime; for (size_t i = 0; i < min(backtrace.size(), Profiling::max_stack_frame_count); ++i) { sample.frames[i] = backtrace[i]; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 16ff3f231a..6e8b4ab5ab 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -282,7 +282,7 @@ int Process::do_exec(NonnullRefPtr<FileDescription> main_program_description, Ve m_master_tls_size = master_tls_size; m_master_tls_alignment = master_tls_alignment; - // FIXME: PID/TID BUG + // FIXME: PID/TID ISSUE m_pid = new_main_thread->tid().value(); new_main_thread->make_thread_specific_region({}); new_main_thread->reset_fpu_state(); diff --git a/Kernel/Syscalls/kill.cpp b/Kernel/Syscalls/kill.cpp index 6331280f3b..818ebf2a72 100644 --- a/Kernel/Syscalls/kill.cpp +++ b/Kernel/Syscalls/kill.cpp @@ -43,7 +43,7 @@ KResult Process::do_kill(Process& process, int signal) return KSuccess; } -KResult Process::do_killpg(pid_t pgrp, int signal) +KResult Process::do_killpg(ProcessGroupID pgrp, int signal) { InterruptDisabler disabler; @@ -52,8 +52,7 @@ KResult Process::do_killpg(pid_t pgrp, int signal) // Send the signal to all processes in the given group. if (pgrp == 0) { // Send the signal to our own pgrp. - // FIXME: PIF/PGID INCOMPLETE - pgrp = pgid().value(); + pgrp = pgid(); } bool group_was_empty = true; diff --git a/Kernel/Syscalls/sched.cpp b/Kernel/Syscalls/sched.cpp index b144f39792..7729a817b0 100644 --- a/Kernel/Syscalls/sched.cpp +++ b/Kernel/Syscalls/sched.cpp @@ -35,7 +35,7 @@ int Process::sys$yield() return 0; } -int Process::sys$donate(int tid) +int Process::sys$donate(pid_t tid) { REQUIRE_PROMISE(stdio); if (tid < 0) @@ -48,7 +48,7 @@ int Process::sys$donate(int tid) return 0; } -int Process::sys$sched_setparam(int tid, Userspace<const struct sched_param*> user_param) +int Process::sys$sched_setparam(int pid, Userspace<const struct sched_param*> user_param) { REQUIRE_PROMISE(proc); if (!validate_read_typed(user_param)) @@ -59,8 +59,8 @@ int Process::sys$sched_setparam(int tid, Userspace<const struct sched_param*> us InterruptDisabler disabler; auto* peer = Thread::current(); - if (tid != 0) - peer = Thread::from_tid(tid); + if (pid != 0) + peer = Thread::from_tid(pid); if (!peer) return -ESRCH; @@ -68,8 +68,7 @@ int Process::sys$sched_setparam(int tid, Userspace<const struct sched_param*> us if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid) return -EPERM; - if (desired_param.sched_priority < THREAD_PRIORITY_MIN || - desired_param.sched_priority > THREAD_PRIORITY_MAX) + if (desired_param.sched_priority < THREAD_PRIORITY_MIN || desired_param.sched_priority > THREAD_PRIORITY_MAX) return -EINVAL; peer->set_priority((u32)desired_param.sched_priority); @@ -84,8 +83,11 @@ int Process::sys$sched_getparam(pid_t pid, Userspace<struct sched_param*> user_p InterruptDisabler disabler; auto* peer = Thread::current(); - if (pid != 0) + if (pid != 0) { + // FIXME: PID/TID BUG + // The entire process is supposed to be affected. peer = Thread::from_tid(pid); + } if (!peer) return -ESRCH; @@ -93,12 +95,14 @@ int Process::sys$sched_getparam(pid_t pid, Userspace<struct sched_param*> user_p if (!is_superuser() && m_euid != peer->process().m_uid && m_uid != peer->process().m_uid) return -EPERM; - struct sched_param param { (int) peer->priority() }; + struct sched_param param { + (int)peer->priority() + }; copy_to_user(user_param, ¶m); return 0; } -int Process::sys$set_thread_boost(int tid, int amount) +int Process::sys$set_thread_boost(pid_t tid, int amount) { REQUIRE_PROMISE(proc); if (amount < 0 || amount > 20) diff --git a/Kernel/Syscalls/thread.cpp b/Kernel/Syscalls/thread.cpp index 1fe2d77192..35367bdd71 100644 --- a/Kernel/Syscalls/thread.cpp +++ b/Kernel/Syscalls/thread.cpp @@ -73,7 +73,7 @@ int Process::sys$create_thread(void* (*entry)(void*), Userspace<const Syscall::S // length + 4 to give space for our extra junk at the end StringBuilder builder(m_name.length() + 4); builder.append(m_name); - builder.appendf("[%d]", thread->tid()); + builder.appendf("[%d]", thread->tid().value()); thread->set_name(builder.to_string()); thread->set_priority(requested_thread_priority); @@ -102,7 +102,7 @@ void Process::sys$exit_thread(void* exit_value) ASSERT_NOT_REACHED(); } -int Process::sys$detach_thread(int tid) +int Process::sys$detach_thread(pid_t tid) { REQUIRE_PROMISE(thread); InterruptDisabler disabler; @@ -117,7 +117,7 @@ int Process::sys$detach_thread(int tid) return 0; } -int Process::sys$join_thread(int tid, void** exit_value) +int Process::sys$join_thread(pid_t tid, void** exit_value) { REQUIRE_PROMISE(thread); if (exit_value && !validate_write_typed(exit_value)) @@ -169,7 +169,7 @@ int Process::sys$join_thread(int tid, void** exit_value) return 0; } -int Process::sys$set_thread_name(int tid, const char* user_name, size_t user_name_length) +int Process::sys$set_thread_name(pid_t tid, const char* user_name, size_t user_name_length) { REQUIRE_PROMISE(thread); auto name = validate_and_copy_string_from_user(user_name, user_name_length); @@ -188,7 +188,7 @@ int Process::sys$set_thread_name(int tid, const char* user_name, size_t user_nam thread->set_name(name); return 0; } -int Process::sys$get_thread_name(int tid, char* buffer, size_t buffer_size) +int Process::sys$get_thread_name(pid_t tid, char* buffer, size_t buffer_size) { REQUIRE_PROMISE(thread); if (buffer_size == 0) diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index d04a4dcc1d..196c6befa4 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -54,11 +54,10 @@ Thread::Thread(NonnullRefPtr<Process> process) // First thread gets TID == PID m_tid = m_process->pid().value(); } else { - // TODO: Use separate counter? m_tid = Process::allocate_pid().value(); } #ifdef THREAD_DEBUG - dbg() << "Created new thread " << m_process->name() << "(" << m_process->pid() << ":" << m_tid << ")"; + dbg() << "Created new thread " << m_process->name() << "(" << m_process->pid().value() << ":" << m_tid.value() << ")"; #endif set_default_signal_dispositions(); m_fpu_state = (FPUState*)kmalloc_aligned(sizeof(FPUState), 16); @@ -87,7 +86,7 @@ Thread::Thread(NonnullRefPtr<Process> process) m_tss.cr3 = m_process->page_directory().cr3(); - m_kernel_stack_region = MM.allocate_kernel_region(default_kernel_stack_size, String::format("Kernel Stack (Thread %d)", m_tid), Region::Access::Read | Region::Access::Write, false, true); + m_kernel_stack_region = MM.allocate_kernel_region(default_kernel_stack_size, String::format("Kernel Stack (Thread %d)", m_tid.value()), Region::Access::Read | Region::Access::Write, false, true); m_kernel_stack_region->set_stack(true); m_kernel_stack_base = m_kernel_stack_region->vaddr().get(); m_kernel_stack_top = m_kernel_stack_region->vaddr().offset(default_kernel_stack_size).get() & 0xfffffff8u; @@ -963,7 +962,7 @@ void Thread::wake_from_queue() set_state(State::Running); } -Thread* Thread::from_tid(int tid) +Thread* Thread::from_tid(ThreadID tid) { InterruptDisabler disabler; Thread* found_thread = nullptr; diff --git a/Kernel/Thread.h b/Kernel/Thread.h index d869eac70c..0252333d9e 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -82,7 +82,7 @@ public: explicit Thread(NonnullRefPtr<Process>); ~Thread(); - static Thread* from_tid(pid_t); + static Thread* from_tid(ThreadID); static void finalize_dying_threads(); ThreadID tid() const { return m_tid; } diff --git a/Kernel/init.cpp b/Kernel/init.cpp index 09c75da774..758b1f32db 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -336,7 +336,7 @@ void init_stage2() tty0->set_graphical(!text_mode); Thread* thread = nullptr; auto userspace_init = kernel_command_line().lookup("init").value_or("/bin/SystemServer"); - Process::create_user_process(thread, userspace_init, (uid_t)0, (gid_t)0, (pid_t)0, error, {}, {}, tty0); + Process::create_user_process(thread, userspace_init, (uid_t)0, (gid_t)0, ProcessID(0), error, {}, {}, tty0); if (error != 0) { klog() << "init_stage2: error spawning SystemServer: " << error; Processor::halt(); diff --git a/Libraries/LibC/serenity.cpp b/Libraries/LibC/serenity.cpp index 5837aee15f..8c357d0828 100644 --- a/Libraries/LibC/serenity.cpp +++ b/Libraries/LibC/serenity.cpp @@ -60,13 +60,13 @@ int profiling_disable(pid_t pid) __RETURN_WITH_ERRNO(rc, rc, -1); } -int set_thread_boost(int tid, int amount) +int set_thread_boost(pid_t tid, int amount) { int rc = syscall(SC_set_thread_boost, tid, amount); __RETURN_WITH_ERRNO(rc, rc, -1); } -int set_process_boost(int tid, int amount) +int set_process_boost(pid_t tid, int amount) { int rc = syscall(SC_set_process_boost, tid, amount); __RETURN_WITH_ERRNO(rc, rc, -1); @@ -136,5 +136,4 @@ int get_stack_bounds(uintptr_t* user_stack_base, size_t* user_stack_size) int rc = syscall(SC_get_stack_bounds, user_stack_base, user_stack_size); __RETURN_WITH_ERRNO(rc, rc, -1); } - } diff --git a/Libraries/LibC/serenity.h b/Libraries/LibC/serenity.h index 55423f13c2..396cd34f8d 100644 --- a/Libraries/LibC/serenity.h +++ b/Libraries/LibC/serenity.h @@ -52,7 +52,7 @@ int profiling_disable(pid_t); #define THREAD_PRIORITY_HIGH 50 #define THREAD_PRIORITY_MAX 99 -int set_thread_boost(int tid, int amount); +int set_thread_boost(pid_t tid, int amount); int set_process_boost(pid_t, int amount); #define FUTEX_WAIT 1 diff --git a/Libraries/LibC/sys/ptrace.cpp b/Libraries/LibC/sys/ptrace.cpp index fd97a0ffca..a83d8ec79b 100644 --- a/Libraries/LibC/sys/ptrace.cpp +++ b/Libraries/LibC/sys/ptrace.cpp @@ -31,7 +31,7 @@ extern "C" { -int ptrace(int request, pid_t pid, void* addr, int data) +int ptrace(int request, pid_t tid, void* addr, int data) { // PT_PEEK needs special handling since the syscall wrapper @@ -49,7 +49,7 @@ int ptrace(int request, pid_t pid, void* addr, int data) Syscall::SC_ptrace_params params { request, - pid, + tid, reinterpret_cast<u8*>(addr), data }; diff --git a/Libraries/LibC/sys/ptrace.h b/Libraries/LibC/sys/ptrace.h index b1008f62ca..edb45aeec9 100644 --- a/Libraries/LibC/sys/ptrace.h +++ b/Libraries/LibC/sys/ptrace.h @@ -40,6 +40,9 @@ __BEGIN_DECLS #define PT_POKE 8 #define PT_SETREGS 9 -int ptrace(int request, pid_t pid, void* addr, int data); +// FIXME: PID/TID ISSUE +// Affects the entirety of LibDebug and Userland/strace.cpp. +// See also Kernel/Ptrace.cpp +int ptrace(int request, pid_t tid, void* addr, int data); __END_DECLS diff --git a/Libraries/LibCore/ProcessStatisticsReader.h b/Libraries/LibCore/ProcessStatisticsReader.h index c260735ffa..6620365d08 100644 --- a/Libraries/LibCore/ProcessStatisticsReader.h +++ b/Libraries/LibCore/ProcessStatisticsReader.h @@ -33,7 +33,7 @@ namespace Core { struct ThreadStatistics { - int tid; + pid_t tid; unsigned times_scheduled; unsigned ticks; unsigned syscall_count; diff --git a/Libraries/LibThread/Lock.h b/Libraries/LibThread/Lock.h index 4fe0bb88f5..b6a82c42a2 100644 --- a/Libraries/LibThread/Lock.h +++ b/Libraries/LibThread/Lock.h @@ -44,7 +44,7 @@ public: void unlock(); private: - Atomic<int> m_holder { 0 }; + Atomic<pid_t> m_holder { 0 }; u32 m_level { 0 }; }; @@ -65,14 +65,14 @@ private: ALWAYS_INLINE void Lock::lock() { - int tid = gettid(); + pid_t tid = gettid(); if (m_holder == tid) { ++m_level; return; } for (;;) { int expected = 0; - if (m_holder.compare_exchange_strong(expected, tid, AK::memory_order_acq_rel)) { + if (m_holder.compare_exchange_strong(expected, tid, AK::memory_order_acq_rel)) { m_level = 1; return; } |