Skip to content

Commit

Permalink
Kernel: PID/TID typing
Browse files Browse the repository at this point in the history
This compiles, and contains exactly the same bugs as before.
The regex 'FIXME: PID/' should reveal all markers that I left behind, including:
- Incomplete conversion
- Issues or things that look fishy
- Actual bugs that will go wrong during runtime
  • Loading branch information
BenWiederhake authored and awesomekling committed Aug 10, 2020
1 parent f225321 commit f5744a6
Show file tree
Hide file tree
Showing 26 changed files with 136 additions and 111 deletions.
22 changes: 11 additions & 11 deletions Kernel/FileSystem/ProcFS.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ enum ProcFileType {
FI_MaxStaticFileIndex,
};

static inline pid_t to_pid(const InodeIdentifier& identifier)
static inline ProcessID to_pid(const InodeIdentifier& identifier)
{
#ifdef PROCFS_DEBUG
dbg() << "to_pid, index=" << String::format("%08x", identifier.index()) << " -> " << (identifier.index() >> 16);
Expand Down Expand Up @@ -154,14 +154,14 @@ static inline size_t to_sys_index(const InodeIdentifier& identifier)
return identifier.index() >> 16u;
}

static inline InodeIdentifier to_identifier(unsigned fsid, ProcParentDirectory parent, pid_t pid, ProcFileType proc_file_type)
static inline InodeIdentifier to_identifier(unsigned fsid, ProcParentDirectory parent, ProcessID pid, ProcFileType proc_file_type)
{
return { fsid, ((unsigned)parent << 12u) | ((unsigned)pid << 16u) | (unsigned)proc_file_type };
return { fsid, ((unsigned)parent << 12u) | ((unsigned)pid.value() << 16u) | (unsigned)proc_file_type };
}

static inline InodeIdentifier to_identifier_with_fd(unsigned fsid, pid_t pid, int fd)
static inline InodeIdentifier to_identifier_with_fd(unsigned fsid, ProcessID pid, int fd)
{
return { fsid, (PDI_PID_fd << 12u) | ((unsigned)pid << 16u) | (FI_MaxStaticFileIndex + fd) };
return { fsid, (PDI_PID_fd << 12u) | ((unsigned)pid.value() << 16u) | (FI_MaxStaticFileIndex + fd) };
}

static inline InodeIdentifier sys_var_to_identifier(unsigned fsid, unsigned index)
Expand Down Expand Up @@ -663,7 +663,7 @@ Optional<KBuffer> procfs$pid_root(InodeIdentifier identifier)
Optional<KBuffer> procfs$self(InodeIdentifier)
{
char buffer[16];
sprintf(buffer, "%u", Process::current()->pid());
sprintf(buffer, "%d", Process::current()->pid().value());
return KBuffer::copy((const u8*)buffer, strlen(buffer));
}

Expand Down Expand Up @@ -821,13 +821,13 @@ Optional<KBuffer> procfs$all(InodeIdentifier)
break;
}

process_object.add("pid", process.pid());
process_object.add("pid", process.pid().value());
process_object.add("pgid", process.tty() ? process.tty()->pgid() : 0);
process_object.add("pgp", process.pgid());
process_object.add("sid", process.sid());
process_object.add("uid", process.uid());
process_object.add("gid", process.gid());
process_object.add("ppid", process.ppid());
process_object.add("ppid", process.ppid().value());
process_object.add("nfds", process.number_of_open_file_descriptors());
process_object.add("name", process.name());
process_object.add("tty", process.tty() ? process.tty()->tty_name() : "notty");
Expand All @@ -842,7 +842,7 @@ Optional<KBuffer> procfs$all(InodeIdentifier)
auto thread_array = process_object.add_array("threads");
process.for_each_thread([&](const Thread& thread) {
auto thread_object = thread_array.add_object();
thread_object.add("tid", thread.tid());
thread_object.add("tid", thread.tid().value());
thread_object.add("name", thread.name());
thread_object.add("times_scheduled", thread.times_scheduled());
thread_object.add("ticks", thread.ticks());
Expand Down Expand Up @@ -1227,7 +1227,7 @@ KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntr
}
for (auto pid_child : Process::all_pids()) {
char name[16];
size_t name_length = (size_t)sprintf(name, "%u", pid_child);
size_t name_length = (size_t)sprintf(name, "%d", pid_child.value());
callback({ name, name_length, to_identifier(fsid(), PDI_Root, pid_child, FI_PID), 0 });
}
break;
Expand Down Expand Up @@ -1270,7 +1270,7 @@ KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntr
if (!description)
continue;
char name[16];
size_t name_length = (size_t)sprintf(name, "%u", i);
size_t name_length = (size_t)sprintf(name, "%d", i);
callback({ name, name_length, to_identifier_with_fd(fsid(), pid, i), 0 });
}
} break;
Expand Down
4 changes: 2 additions & 2 deletions Kernel/Net/Socket.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Socket::Socket(int domain, int type, int protocol)
, m_protocol(protocol)
{
auto& process = *Process::current();
m_origin = { process.pid(), process.uid(), process.gid() };
m_origin = { process.pid().value(), process.uid(), process.gid() };
}

Socket::~Socket()
Expand All @@ -83,7 +83,7 @@ RefPtr<Socket> Socket::accept()
auto client = m_pending.take_first();
ASSERT(!client->is_connected());
auto& process = *Process::current();
client->m_acceptor = { process.pid(), process.uid(), process.gid() };
client->m_acceptor = { process.pid().value(), process.uid(), process.gid() };
client->m_connected = true;
client->m_role = Role::Accepted;
return client;
Expand Down
4 changes: 2 additions & 2 deletions Kernel/PerformanceEventBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ PerformanceEvent& PerformanceEventBuffer::at(size_t index)
return events[index];
}

KBuffer PerformanceEventBuffer::to_json(pid_t pid, const String& executable_path) const
KBuffer PerformanceEventBuffer::to_json(ProcessID pid, const String& executable_path) const
{
KBufferBuilder builder;

JsonObjectSerializer object(builder);
object.add("pid", pid);
object.add("pid", pid.value());
object.add("executable", executable_path);

auto array = object.add_array("events");
Expand Down
2 changes: 1 addition & 1 deletion Kernel/PerformanceEventBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class PerformanceEventBuffer {
return const_cast<PerformanceEventBuffer&>(*this).at(index);
}

KBuffer to_json(pid_t, const String& executable_path) const;
KBuffer to_json(ProcessID, const String& executable_path) const;

private:
PerformanceEvent& at(size_t index);
Expand Down
29 changes: 18 additions & 11 deletions Kernel/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,12 @@ Lock* g_hostname_lock;
VirtualAddress g_return_to_ring3_from_signal_trampoline;
HashMap<String, OwnPtr<Module>>* g_modules;

pid_t Process::allocate_pid()
ProcessID Process::allocate_pid()
{
// Overflow is UB, and negative PIDs wreck havoc.
// TODO: Handle PID overflow
// For example: Use an Atomic<u32>, mask the most significant bit,
// retry if PID is already taken as a PID, taken as a TID, or zero.
return next_pid.fetch_add(1, AK::MemoryOrder::memory_order_acq_rel);
}

Expand All @@ -112,9 +116,9 @@ void Process::initialize()
create_signal_trampolines();
}

Vector<pid_t> Process::all_pids()
Vector<ProcessID> Process::all_pids()
{
Vector<pid_t> pids;
Vector<ProcessID> pids;
ScopedSpinLock lock(g_processes_lock);
pids.ensure_capacity((int)g_processes->size_slow());
for (auto& process : *g_processes)
Expand Down Expand Up @@ -286,7 +290,7 @@ void Process::kill_all_threads()
});
}

RefPtr<Process> Process::create_user_process(Thread*& first_thread, const String& path, uid_t uid, gid_t gid, pid_t parent_pid, int& error, Vector<String>&& arguments, Vector<String>&& environment, TTY* tty)
RefPtr<Process> Process::create_user_process(Thread*& first_thread, const String& path, uid_t uid, gid_t gid, ProcessID parent_pid, int& error, Vector<String>&& arguments, Vector<String>&& environment, TTY* tty)
{
auto parts = path.split('/');
if (arguments.is_empty()) {
Expand Down Expand Up @@ -334,7 +338,7 @@ RefPtr<Process> Process::create_user_process(Thread*& first_thread, const String

NonnullRefPtr<Process> Process::create_kernel_process(Thread*& first_thread, String&& name, void (*e)(), u32 affinity)
{
auto process = adopt(*new Process(first_thread, move(name), (uid_t)0, (gid_t)0, (pid_t)0, Ring0));
auto process = adopt(*new Process(first_thread, move(name), (uid_t)0, (gid_t)0, ProcessID(0), Ring0));
first_thread->tss().eip = (FlatPtr)e;

if (process->pid() != 0) {
Expand All @@ -348,7 +352,7 @@ NonnullRefPtr<Process> Process::create_kernel_process(Thread*& first_thread, Str
return process;
}

Process::Process(Thread*& first_thread, const String& name, uid_t uid, gid_t gid, pid_t ppid, RingLevel ring, RefPtr<Custody> cwd, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
Process::Process(Thread*& first_thread, const String& name, uid_t uid, gid_t gid, ProcessID ppid, RingLevel ring, RefPtr<Custody> cwd, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
: m_name(move(name))
, m_pid(allocate_pid())
, m_euid(uid)
Expand Down Expand Up @@ -473,10 +477,11 @@ void Process::crash(int signal, u32 eip, bool out_of_memory)
ASSERT_NOT_REACHED();
}

RefPtr<Process> Process::from_pid(pid_t pid)
RefPtr<Process> Process::from_pid(ProcessID pid)
{
ScopedSpinLock lock(g_processes_lock);
for (auto& process : *g_processes) {
process.pid();
if (process.pid() == pid)
return &process;
}
Expand Down Expand Up @@ -553,7 +558,7 @@ siginfo_t Process::reap(Process& process)
siginfo_t siginfo;
memset(&siginfo, 0, sizeof(siginfo));
siginfo.si_signo = SIGCHLD;
siginfo.si_pid = process.pid();
siginfo.si_pid = process.pid().value();
siginfo.si_uid = process.uid();

if (process.m_termination_signal) {
Expand All @@ -566,7 +571,7 @@ siginfo_t Process::reap(Process& process)

ASSERT(g_processes_lock.is_locked());

if (process.ppid()) {
if (!!process.ppid()) {
auto parent = Process::from_pid(process.ppid());
if (parent) {
parent->m_ticks_in_user_for_dead_children += process.m_ticks_in_user + process.m_ticks_in_user_for_dead_children;
Expand Down Expand Up @@ -654,7 +659,8 @@ void Process::finalize()
disown_all_shared_buffers();
{
InterruptDisabler disabler;
if (auto* parent_thread = Thread::from_tid(m_ppid)) {
// FIXME: PID/TID BUG
if (auto* parent_thread = Thread::from_tid(m_ppid.value())) {
if (parent_thread->m_signal_action_data[SIGCHLD].flags & SA_NOCLDWAIT) {
// NOTE: If the parent doesn't care about this process, let it go.
m_ppid = 0;
Expand Down Expand Up @@ -783,7 +789,8 @@ void Process::terminate_due_to_signal(u8 signal)
KResult Process::send_signal(u8 signal, Process* sender)
{
InterruptDisabler disabler;
if (auto* thread = Thread::from_tid(m_pid)) {
// FIXME: PID/TID BUG
if (auto* thread = Thread::from_tid(m_pid.value())) {
thread->send_signal(signal, sender);
return KSuccess;
}
Expand Down
41 changes: 21 additions & 20 deletions Kernel/Process.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ class Process
}

static NonnullRefPtr<Process> create_kernel_process(Thread*& first_thread, String&& name, void (*entry)(), u32 affinity = THREAD_AFFINITY_DEFAULT);
static RefPtr<Process> create_user_process(Thread*& first_thread, const String& path, uid_t, gid_t, pid_t ppid, int& error, Vector<String>&& arguments = Vector<String>(), Vector<String>&& environment = Vector<String>(), TTY* = nullptr);
static RefPtr<Process> create_user_process(Thread*& first_thread, const String& path, uid_t, gid_t, ProcessID ppid, int& error, Vector<String>&& arguments = Vector<String>(), Vector<String>&& environment = Vector<String>(), TTY* = nullptr);
~Process();

static Vector<pid_t> all_pids();
static Vector<ProcessID> all_pids();
static AK::NonnullRefPtrVector<Process> all_processes();

Thread* create_kernel_thread(void (*entry)(), u32 priority, const String& name, u32 affinity = THREAD_AFFINITY_DEFAULT, bool joinable = true);
Expand All @@ -152,10 +152,10 @@ class Process
PageDirectory& page_directory() { return *m_page_directory; }
const PageDirectory& page_directory() const { return *m_page_directory; }

static RefPtr<Process> from_pid(pid_t);
static RefPtr<Process> from_pid(ProcessID);

const String& name() const { return m_name; }
pid_t pid() const { return m_pid; }
ProcessID pid() const { return m_pid; }
pid_t sid() const { return m_sid; }
pid_t pgid() const { return m_pgid; }
const FixedArray<gid_t>& extra_gids() const { return m_extra_gids; }
Expand All @@ -165,9 +165,9 @@ class Process
gid_t gid() const { return m_gid; }
uid_t suid() const { return m_suid; }
gid_t sgid() const { return m_sgid; }
pid_t ppid() const { return m_ppid; }
ProcessID ppid() const { return m_ppid; }

pid_t exec_tid() const { return m_exec_tid; }
ThreadID exec_tid() const { return m_exec_tid; }

mode_t umask() const { return m_umask; }

Expand Down Expand Up @@ -224,7 +224,7 @@ class Process
int sys$fstat(int fd, stat*);
int sys$stat(Userspace<const Syscall::SC_stat_params*>);
int sys$lseek(int fd, off_t, int whence);
int sys$kill(pid_t pid, int sig);
int sys$kill(pid_t pid_or_pgid, int sig);
[[noreturn]] void sys$exit(int status);
int sys$sigreturn(RegisterState& registers);
pid_t sys$waitid(Userspace<const Syscall::SC_waitid_params*>);
Expand Down Expand Up @@ -263,7 +263,7 @@ class Process
int sys$getgroups(ssize_t, gid_t*);
int sys$setgroups(ssize_t, const gid_t*);
int sys$pipe(int pipefd[2], int flags);
int sys$killpg(int pgrp, int sig);
int sys$killpg(pid_t pgrp, int sig);
int sys$seteuid(uid_t);
int sys$setegid(gid_t);
int sys$setuid(uid_t);
Expand Down Expand Up @@ -338,7 +338,7 @@ class Process
int sys$sendfd(int sockfd, int fd);
int sys$recvfd(int sockfd);
long sys$sysconf(int name);
int sys$disown(pid_t);
int sys$disown(ProcessID);

template<bool sockname, typename Params>
int get_sock_or_peer_name(const Params&);
Expand Down Expand Up @@ -574,8 +574,8 @@ class Process
friend class Scheduler;
friend class Region;

Process(Thread*& first_thread, const String& name, uid_t, gid_t, pid_t ppid, RingLevel, RefPtr<Custody> cwd = nullptr, RefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
static pid_t allocate_pid();
Process(Thread*& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, RingLevel, RefPtr<Custody> cwd = nullptr, RefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
static ProcessID allocate_pid();

Range allocate_range(VirtualAddress, size_t, size_t alignment = PAGE_SIZE);

Expand Down Expand Up @@ -607,7 +607,7 @@ class Process
}
KResultOr<String> get_syscall_path_argument(const Syscall::StringArgument&) const;

bool has_tracee_thread(int tracer_pid) const;
bool has_tracee_thread(ProcessID tracer_pid) const;

RefPtr<PageDirectory> m_page_directory;

Expand All @@ -616,7 +616,7 @@ class Process

String m_name;

pid_t m_pid { 0 };
ProcessID m_pid { 0 };
pid_t m_sid { 0 };
pid_t m_pgid { 0 };

Expand All @@ -627,7 +627,7 @@ class Process
uid_t m_suid { 0 };
gid_t m_sgid { 0 };

pid_t m_exec_tid { 0 };
ThreadID m_exec_tid { 0 };
FlatPtr m_load_offset { 0U };
FlatPtr m_entry_eip { 0U };

Expand Down Expand Up @@ -677,7 +677,7 @@ class Process
};
RegionLookupCache m_region_lookup_cache;

pid_t m_ppid { 0 };
ProcessID m_ppid { 0 };
mode_t m_umask { 022 };

FixedArray<gid_t> m_extra_gids;
Expand Down Expand Up @@ -732,11 +732,12 @@ template<typename Callback>
inline void Process::for_each_child(Callback callback)
{
ASSERT_INTERRUPTS_DISABLED();
pid_t my_pid = pid();
ProcessID my_pid = pid();
ScopedSpinLock lock(g_processes_lock);
for (auto* process = g_processes->head(); process;) {
auto* next_process = process->next();
if (process->ppid() == my_pid || process->has_tracee_thread(m_pid)) {
// FIXME: PID/TID BUG
if (process->ppid() == my_pid || process->has_tracee_thread(m_pid.value())) {
if (callback(*process) == IterationDecision::Break)
break;
}
Expand All @@ -748,7 +749,7 @@ template<typename Callback>
inline void Process::for_each_thread(Callback callback) const
{
InterruptDisabler disabler;
pid_t my_pid = pid();
ProcessID my_pid = pid();

if (my_pid == 0) {
// NOTE: Special case the colonel process, since its main thread is not in the global thread table.
Expand Down Expand Up @@ -800,14 +801,14 @@ inline bool InodeMetadata::may_execute(const Process& process) const
return may_execute(process.euid(), process.egid(), process.extra_gids());
}

inline int Thread::pid() const
inline ProcessID Thread::pid() const
{
return m_process->pid();
}

inline const LogStream& operator<<(const LogStream& stream, const Process& process)
{
return stream << process.name() << '(' << process.pid() << ')';
return stream << process.name() << '(' << process.pid().value() << ')';
}

inline u32 Thread::effective_priority() const
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Profiling.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void start(Process& process)
executable_path() = process.executable()->absolute_path().impl();
else
executable_path() = {};
s_pid = process.pid();
s_pid = process.pid().value(); // FIXME: PID/TID INCOMPLETE

if (!s_profiling_buffer) {
s_profiling_buffer = RefPtr<KBufferImpl>(KBuffer::create_with_size(8 * MB).impl()).leak_ref();
Expand Down
2 changes: 1 addition & 1 deletion Kernel/Ptrace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ KResultOr<u32> handle_syscall(const Kernel::Syscall::SC_ptrace_params& params, P
return KSuccess;
}

if (params.pid == caller.pid())
if (params.pid == caller.pid().value())
return KResult(-EINVAL);

Thread* peer = nullptr;
Expand Down
Loading

0 comments on commit f5744a6

Please sign in to comment.