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

can we use upstream pthread instead of this library? #26

Open
ian-h-chamberlain opened this issue Apr 11, 2023 · 5 comments
Open

can we use upstream pthread instead of this library? #26

ian-h-chamberlain opened this issue Apr 11, 2023 · 5 comments

Comments

@ian-h-chamberlain
Copy link
Member

ian-h-chamberlain commented Apr 11, 2023

The new newlib update from devkitPro seems to include an actual libpthread!!!

$ arm-none-eabi-nm   /opt/devkitpro/devkitARM/arm-none-eabi/lib/libsysbase.a | grep pthread | wc -l 
86

I can't quite tell if it works correctly or at all, but there is definitely some new code in the libsysbase library and we might need to readjust how this library works to play nicely with it (link errors etc.)

The implementation looks to be similar to what we have done in this library, but perhaps the syscalls are still stubs (and we could implement them here, but let newlib handle the business logic):

https://github.com/devkitPro/buildscripts/blob/cd5e224d6a14f7d32712ab10cfc08e0c6a2daea3/dkarm-eabi/patches/newlib-4.3.0.20230120.patch#L7741

There might also be some other stuff that relates to shim-3ds, it seems like a lot of new library surface area. Probably will take a while to crawl through all of it and figure out what we can use or need to change.

Looking at libnx (where I am guessing some of these changes came from) it seems they just implement __syscall_* in terms of the device-specific libraries: https://github.com/switchbrew/libnx/blob/master/nx/source/runtime/newlib.c#L172

Perhaps we could do the same in this lib instead.

@Meziu
Copy link
Member

Meziu commented Apr 11, 2023

Those… don’t look like stubs 😳

Edit: funny how those features always come after our own implementations… clock_gettime is also a fundamental part of these pthread changes, and it came extremely shortly after our impl. Maybe someone is spying on us 👀

@ian-h-chamberlain
Copy link
Member Author

No, they're not stubs exactly, but I see a lot of stuff like

if (!__has_syscall(thread_create))
	return ENOSYS;

I don't think libctru is providing those syscalls currently, so we would need to define them ourselves to get stuff working.

From trying to build a quick example, I think something in Rust runtime startup got broken by these changes (possibly related to thread locals), so we probably have some work to do, lol!

@Meziu
Copy link
Member

Meziu commented Apr 11, 2023

To me the "hello-world" example works fine (updated ctru-sys to the latest bindings and all libraries). The only change is that those bindings don't include __getreent, which has now been moved to newlib, rather than libctru. Should we add that to ctru-sys or to libc?

Edit: I've tested multi-threading and mutexes as well but they don't seem to give any issues to me. The threading order is correctly run and the data is not corrupted. All this on pthread-3ds of course.

@WinterMute
Copy link

Those… don’t look like stubs 😳

Edit: funny how those features always come after our own implementations… clock_gettime is also a fundamental part of these pthread changes, and it came extremely shortly after our impl. Maybe someone is spying on us 👀

Those features were implemented for devkitA64 basically 4 years ago in 2019, see https://devkitpro.org/viewtopic.php?f=13&t=8891&p=16395&hilit=pthreads#p16395. We have simply consolidated a lot of the patching between devkitA64 and devkitARM to ease maintenance and feature parity in future releases.

I would be grateful if people could refrain from making thoughtless comments like this and have some respect for the work we do.

@Meziu
Copy link
Member

Meziu commented Apr 17, 2023

I would be grateful if people could refrain from making thoughtless comments like this and have some respect for the work we do.

We respect your work immensely. It’s only thanks to your and your collaborators’ work that our project could even exist. I only meant that comment as a joke, of course you aren’t copying from us (if anything, it’s the opposite). I often don’t act serious when responding, and I was pretty excited about the changes.

I’m sorry if I said anything you found disrespectful. 😓

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 a pull request may close this issue.

3 participants