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

[stdlib] Add Libc and tests #3766

Open
wants to merge 92 commits into
base: nightly
Choose a base branch
from

Conversation

martinvuyk
Copy link
Contributor

Add Libc and tests

This will pave the way for better cross platform support.

Tests for the most used libc functionality were added. Some trivial functions were left as # TODO for later testing. ioctl was deactivated as it needs to be tested thoroughly.

On the networking side, I have functional socket code but it needs a counterpart to be used or use async code, so I left it "untested" but it already works. The next stage after this would be to bring in the socket package I've been working on for the last couple of months, but I'd rather wait for full async Mojo since many APIs would benefit from being async first.

I can keep developing this further in my repo, but I think this will benefit the stdlib greatly by centralizing the implementation and testing. And also the whole ecosystem will find it useful to have an already "cross platform" Libc wrapper. These tests also introduce a very easy way to figure out platform support problems.

Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
@JoeLoser JoeLoser added the needs-discussion Need discussion in order to move forward label Nov 14, 2024
@JoeLoser
Copy link
Collaborator

JoeLoser commented Nov 14, 2024

Hey Martin, thanks so much for pushing on the libc work! It's something the team has talked about briefly in the past (whether to include libc bindings in the stdlib or not). I'll add it again to the team's weekly design discussions this upcoming Monday and we'll circle back here based on what we discuss.

I'm sure @lsh has some thoughts on this space too 😄

@martinvuyk
Copy link
Contributor Author

Hi @JoeLoser , still working through some kinks in MacOS here 😢 . Was planning on wrapping those up before writing a more thorough explanation of my rationale. But here it goes:

I think this is very necessary; I've already seen bindings all around the stdlib and whether we want to or not is a piece of infrastructure everyone counts on. Having it tested and actually compliant with the standard is very important especially when extending to socket work. It's a real pain-point for anyone doing low level cross platform work. While developing this I actually came across some much needed information and documentation where I actually think we'll have Windows support very soon (I already added most of the macro constants and have a way to do error logging which is the most necessary part). Also something very cool is that I've managed to make it able to be dynamically linked on the 3 operating systems as well :) (need to test windows to be sure)

PS: Eventually I'll drop another 7k ish lines 😆 with my socket design that I hope also makes those issues something of the past. Because this implementation should only deal with translating constants and function calls to be compliant with the standard, every operating system deals with socket behavior differently at the control layer.

Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
Signed-off-by: martinvuyk <[email protected]>
@owenhilyard
Copy link

My main suggestion is to try to make the structure mirror the structure of POSIX, to make it easier to find things. Also, please remember /etc/protocols, there's a lot of useful things in there like PIM, mptcp (Linux only), IPv6 with no next header (nice for routers), kernel-managed TLS, and DCCP.

from .info import os_is_linux, os_is_windows, is_64bit, os_is_macos
from .intrinsics import _mlirtype_is_eq
from sys.info import os_is_linux, os_is_windows, is_64bit, os_is_macos
from sys.intrinsics import _mlirtype_is_eq

from sys._libc import dlerror, dlopen, dlclose, dlsym
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinvuyk what about the sys._libc module? are they going to merge at some point?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @msaelices, yes I imagine they will merge at some point. There is also some future cleanup to switch c_int to use C.int and other such variables (if they decide to merge this PR at all).

stdlib/src/sys/ffi/c/types.mojo Show resolved Hide resolved
stdlib/src/sys/ffi/c/libc.mojo Show resolved Hide resolved
var a = args.get_loaded_kgen_pack()

@parameter
if is_nvidia_gpu():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just is_gpu() work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this workaround in a stdlib function, I don't know if it's some NVIDIA particular thing or if it's that the API for GPUs needs to be different 🤷‍♂️. This vprintf signature is actually non standard AFAIK since it doesn't use the va_list type, but it could also be that the underlying VariadicPack uses such a structure internally and when a Pointer traverses though it it works fine, when I tested vprintf on Linux with the same setup it did not work 🤷‍♂️.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-discussion Need discussion in order to move forward
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants