Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bindings, types, and constants for some mach vm apis #4357

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

beaumccartney
Copy link
Contributor

@beaumccartney beaumccartney commented Oct 7, 2024

From my own bindings, cleaned up a bit to resemble what I see here.

I added:

  • vm_allocate()
  • vm_deallocate()
  • vm_map()
  • mach_make_memory_entry()
  • vm_page_size (constant)
  • types for each of the functions

I've reproduced the signatures, types/typedefs, and constants as exactly as
possible from the headers. I also kept doc comments from the original headers
(but they can go of course).

To that end, I wanted to ask about some of the types and procedure already in
mach_darwin.odin, as they don't match the original C api exactly. There is
a note about aligning the types to 16 bytes so I wanted to check if there was
a reason for the changes (e.g. in case I need to do something similar for the
type aliases I've given).

type aliases:

type mach_darwin.odin c api
task_t u64 mach_port_t
semaphore_t u64 mach_port_t
kern_return_t u64 int
thread_act_t u64 mach_port_t

procedures:

procedure mach_darwin.odin c api
mach_task_self() proc() -> task_t proc() -> mach_port_t
semaphore_signal_thread() proc(semaphore: semaphore_t, thread: thread_t) -> kern_return_t proc(semaphore: semaphore_t, thread: thread_t) -> kern_return_t

Note that mach_port_t is normally an unsigned int

Additionally, semaphore_signal_thread() takes a thread_t, not
a thread_act_t (though they're both typedef-ed to mach_port_t)

Also regarding aligning the types, the note referenced another note that said
that aligning a single-field struct didn't work at the time of writing. This
information was committed in 2019, so perhaps the issue is resolved now?

I made a vm-backed ring that maps multiple virtual pages onto the same physical
page to prove that the bindings are working. (I didn't want to clutter here so
I made a gist).
https://gist.github.com/beaumccartney/60cc84799dbdc1e4890ab1ec0a128fcb

I've made a thin wrapper to do make the api usable with more "odin-y" types,
it's included in the gist I linked above. Is a wrapper like mine worth
including?

  • enums and bitsets
  • [^]byte to reference a c pointer that represents the base of a region of
    memory

I didn't wrap VM_FLAGS_* in anything cause you can pack more information than
just toggling stuff ("superpage" stuff - its a bit weird)

@gingerBill gingerBill requested a review from laytan October 7, 2024 22:57
@laytan
Copy link
Sponsor Collaborator

laytan commented Oct 8, 2024

To that end, I wanted to ask about some of the types and procedure already in
mach_darwin.odin, as they don't match the original C api exactly. There is
a note about aligning the types to 16 bytes so I wanted to check if there was
a reason for the changes (e.g. in case I need to do something similar for the
type aliases I've given).

I don't think any of these types should be different from those in the headers, I don't see anything going on there about aligning, additionally the comment references a linux implementation of pthread? It's all a bit weird.

I would like to see the usage of bit sets and enums here, but you don't need wrappers for that. You can just make the binding return the enums and take the bit sets.

procs:
| type          | old  | new (matching c api)|
| ---           | ---- | ------------------- |
| kern_return_t | u64  | c.int               |
| thread_t      | u64  | mach_port_t         |
| task_t        | u64  | mach_port_t         |
| semaphore_t   | u64  | mach_port_t         |

for mach_task_self(), return mach_port_t instead of task_t

for semaphore_signal_thread(), accept a thread_t instead of a thread_act_t
@beaumccartney
Copy link
Contributor Author

I've changed all the types to match the headers, and used enums and bitsets for everything except VM_FLAGS_*

I'm going to figure something out for VM_FLAGS cause there's a little bit of weirdness with stuff e.g. packing data inside the int as well as the flags. For reference the stuff I'm talking about is VM_FLAGS_* in mach/vm_statistics.h.

I'll try and have something for it done today.

@beaumccartney
Copy link
Contributor Author

@laytan I've added enum and bitset wrappers for everything. Let me know if there's anything else I can do.

@beaumccartney
Copy link
Contributor Author

https://github.com/beaumccartney/Odin/blob/b0ff41e673f8b788706488fb061a13a5862f5e96/core/sys/darwin/mach_darwin.odin#L77

is it possible to put that in rodata? when I prefix it with @rodata I can still compile it, but I can also just edit the value. Is that a bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants