Odin
Odin copied to clipboard
More os2
This pull request puts a dent into os2 for Linux:
- env
- pipe
- errors
- process
For process, I changed the API based on my best guess of the original intention.
For process_signal, I changed the signature to not take a ^Process argument. My guess was this was meant to act like the signal function that can be found on Linux and Windows where it installs signal handlers. Neither of these platforms (as far as I am aware), can install signal handlers to other processes. I defined a common set of signals and signal handlers that I believe both systems support. Also, I changed out the Process_Error for regular os2.Error.
Old:
Signal :: #type proc()
Kill: Signal = nil
Interrupt: Signal = nil
process_signal :: proc(p: ^Process, sig: Signal) -> Process_Error {
return .None
}
New:
Signal :: enum {
Abort,
Floating_Point_Exception,
Illegal_Instruction,
Interrupt,
Segmentation_Fault,
Termination,
}
Signal_Handler_Proc :: #type proc "c" (c.int)
Signal_Handler_Special :: enum {
Default,
Ignore,
}
Signal_Handler :: union {
Signal_Handler_Proc,
Signal_Handler_Special,
}
process_signal :: proc(sig: Signal, h: Signal_Handler) -> Error {
return _process_signal(sig, h)
}
I added a process_get_state proc:
process_get_state :: proc(p: Process) -> (Process_State, Error) {
return _process_get_state(p)
}
I also added a Duration to process_wait as it seems like a common ability between different systems.
I changed the Process_Attributes struct to contain explicit pointers to stdin, stdout and stderr instead of []^File. These get populated if they are passed to process_start as non-nil.
Process_Attributes :: struct {
dir: string,
env: []string,
stdin: ^File,
stdout: ^File,
stderr: ^File,
sys: ^Process_Attributes_OS_Specific,
}
As far as it goes, I'm not married to any of the API decisions I made. For example, to open a child process with pipes to stdout and stdin, the code looks like this:
child_stdout: os2.File
child_stdin: os2.File
attr: os2.Process_Attributes = {
stdin = &child_stdin,
stdout = &child_stdout,
}
os2.process_start("cat", nil, &attr)
I don't know.. it feels clunky to me.
Just noticed some issues concerning the Stat struct. The definition of the struct is platform-dependent, meaning this won't necessarily compile on x86-32 and arm linucees.
I'll link some definitions of these structs from the linux kernel. You can find them in the following files (relative to repository root):
- x86:
/arch/x86/include/uapi/asm/stat.h - arm32:
/arch/arm/include/uapi/asm/stat.h - arm64:
/include/uapi/asm-generic
Quick explanation of the structure: uapi means user api, the files in the arch-specific folders are bindings that old architectures used, before linux decided to make the definition of stat work similar on the different architectures. x86-64, x86-32 and arm32 use the old way, whereas arm64 uses the new way.
I think we only care about struct stat and struct stat64, struct old_stat is pretty much obsolete.
Some noteable observations about this struct definition's differences:
- x86-64 and x86-32 have the order of
st_nlinkandst_modefields flipped - The new stat deprecates register-sized inode and puts the new inode at the end of the struct
I'm currently working on binding more syscalls in the sys package, potentially adding Odin's types, so that it's a bit easier to do cross-arch stuff, but I'll provide the definition of Stat that I have
when ARCH == "amd64" {
Stat :: struct {
st_dev: c_ulong,
st_ino: c_ulong,
st_nlink: c_ulong,
st_mode: c_uint,
st_uid: c_uint, // 32-bit uid
st_gid: c_uint, // 32-bit uid
_: u32,
st_rdev: c_ulong,
st_size: c_ulong,
st_blksize: c_ulong,
st_blocks: c_ulong,
st_atim: c_timespec,
st_mtim: c_timespec,
st_ctim: c_timespec,
_: [3]c_ulong,
}
} else {
Stat :: struct {
st_dev: c_ulong,
st_ino: c_ulong,
st_mode: c_ushort,
st_nlink: c_ushort,
st_uid: c_ushort, // 16-bit gid
st_gid: c_ushort, // 16-bit uid
st_rdev: c_ulong,
st_size: c_ulong,
st_blksize: c_ulong,
st_blocks: c_ulong,
st_atim: c_timespec,
st_mtim: c_timespec,
st_ctim: c_timespec,
_: c_ulong,
_: c_ulong,
}
}
This doesn't have the ARM bindings yet (because I'm sure mine are incorrect), but that should at least point out the problem with stat.
@flysand7
Thanks dude. I just copied the stat structure out of core/os/os_linux.odin. So this change should probably be back-ported. Looks like I jumped
Haha. I fat fingered the close button while typing in that reply. Looks like I jumped the gun not making CI pass first anyway.
I'm currently working on binding more syscalls in the sys package, potentially adding Odin's types, so that it's a bit easier to do cross-arch stuff
Definitely looking forward to this!
Please don't merge yet. I want to go the buildah/podman route and get everything working across architectures. Should I just close?
In the top-right corner of the page there's a button that makes PR a draft which prevents it from being merged prematurely. I think you can undraft once you're finished
Any updates?
Any updates?
=] I don't have as much free time as you it would seem (full-time job + 4 kids). Note how the first commit on this was in May =] I'm sure you are right about the stat struct, but I want to make sure there are not similar issues with other structs. I'm going to write a bash script to run my tests across all architectures automatically using buildah/podman. I have a private repo for it that I will make public when that part works.
Ooh that's interesting! I didn't know about buildah/podman, so thanks for name dropping. Something that executes tests in a VM sounds like it could also be useful for my current work with syscalls, as I'm not sure about whether I've done a correct job on arch-dependent stuff in ARM/x86-32
Something I've realised is that there's a difference between handle inheritance between linux and windows that should get addressed. When you create a child process some handles may or may not be inherited by that process. On linux that is controlled by O_CLOEXEC flag. On windows that is controlled by bInheritHandles value in the security attributes struct.
The difference lies in the default behavior (source). On linux by default (unless the flag is specified) the handle is inherited. On windows it is not inherited. I think the easiest thing to do here is to use the windows behavior as Odin's default, at least for pipes. Could you pass O_CLOEXEC flag when creating linux pipes, or at least set both ends of the pipe to close on exec?
@flysand7 Will do! I just changed some things in sys/linux. I wanted to ask you about your Sig_Set definition. I thought sigset_t was a u64 as it's just a bit set of the signals. This is going to be 16 uints?
@private _SIGSET_NWORDS :: (1024 / (8 * size_of(uint)))
Sig_Set :: [_SIGSET_NWORDS]uint
@flysand7 Will do! I just changed some things in sys/linux. I wanted to ask you about your
Sig_Setdefinition. I thoughtsigset_twas au64as it's just a bit set of the signals. This is going to be 16 uints?
I'm not totally sure about it but it should probably be verified with a C compiler, but when I was making this I simply copied the typedefs from libc. It certaintly is not incorrect, but it might be wasteful to allocate that much space.
Should be good to go. This passes these tests on amd64 and arm64 with the aid of Puklaj's fork fix (#3476 ).
I made a local CI-like runner bash script thing to emulate other architectures in case anyone wants to do that on their own machine.