STD Cleanup and Test

Question

STD Cleanup and Test

Closed this issue 3 years ago · 51 comments

We have worked with most of the std while updating ctru-rs's modules. However, we now have to merge our changes into the rustlang tree, and this will require cleanup and a lot of testing. We must make sure to never have to make PR's to std ever again, since the effort for the contributor and the reviewer is great.

Since very basic/already tested multiple times, I'd skip testing of fundamental datatypes and of these modules:

alloc
any
array
ascii
borrow
boxed
cell
char
clone
cmp
collections
convert
default
error
f32
f64
ffi
fmt
hint
io
iter
marker
mem
num
ops
option
os (it's empty for our target)
panic
prelude
ptr
rc
result
slice
str
string
vec

TESTED AND NOW WORKING MODULES:

hash
future
pin
time
env
process
sync
thread

Modules we should test, though we have already worked with:

fs
path

Other than testing, cleaning up lints and other problems is needed.

@AzureMarker @ian-h-chamberlain
Please, a combined effort is the best and fastest way.

Answer 1 · 2022-01-30T18:37:15.000Z

I'm interested to see what kind of futures executor I can get running on the 3DS. Then again, there isn't much in the futures module that could be broken.

Answer 2 · 2022-01-30T18:54:39.000Z

Maybe there's a way to run the runtime std tests on-device?

Answer 3 · 2022-01-30T19:02:41.000Z

At one point I actually was able to build libc-test for the 3DS but ran into some issues when running it. I didn't spend much time to debug the issue, but based on that I believe it should be possible to build and run tests on the device (not as part of CI or anything, but perhaps with Citra there's something possible...)

I can start taking a look at cleaning up lints and warnings, I noticed that std does not seem to build using the default RUSTFLAGS in x.py, so I think there's some cleanup to do there, which should help make submitting upstream easier in the future.

Answer 4 · 2022-01-30T19:25:13.000Z

I noticed that std does not seem to build using the default RUSTFLAGS in x.py

x.py build -i library/std --stage 1 doesn't work for you?

Answer 5 · 2022-01-30T19:31:40.000Z

Maybe there's a way to run the runtime std tests on-device?

rust3ds/ctru-rs#25 Maybe we could do something with this, or Citra. Though I suggest on-hardware tests (as I also can't run Citra).

Answer 6 · 2022-01-30T19:32:33.000Z

I'm interested to see what kind of futures executor I can get running on the 3DS. Then again, there isn't much in the futures module that could be broken.

This was me before noticing HashMap are broken. Yes. A collection type (it's actually an issue with randoms).

Answer 7 · 2022-01-30T20:08:33.000Z

x.py build -i library/std --stage 1 doesn't work for you?

This was a while ago, so it might have been a combination of libc changes and my own setup, but IIRC there were some warnings which compiled (but warned) with -Zbuild-std but failed with x.py build library/std --target armv6k-nintendo-3ds because of -D warnings or something like that. I'll double check, right now I've been trying various x.py check, x.py clippy just to see what comes up. A bunch of test stuff doesn't pass the check due to the getrandom issue @Meziu mentioned, which looks like something we might be able to easily fix by adding some target_os = "horizon" in the getrandom crate.

Answer 8 · 2022-01-30T20:13:37.000Z

IIRC there were some warnings which compiled (but warned) with -Zbuild-std but failed with x.py build library/std --target armv6k-nintendo-3ds

I personally never built the std separately, and always built the stage 1 or 2 compiler.

A bunch of test stuff doesn't pass the check due to the getrandom issue @Meziu mentioned, which looks like something we might be able to easily fix by adding some target_os = "horizon" in the getrandom crate.

It's just a small change in sys/unix/rand.rs to use rand() instead of non-available syscalls.

Answer 9 · 2022-01-30T20:25:40.000Z

I haven't looked into it, but there's some randomness code in libctru used by ctru-rs which we might be able to use in the linker fix crate to supply a libc call?

Answer 10 · 2022-01-30T21:30:13.000Z

I haven't looked into it, but there's some randomness code in libctru used by ctru-rs which we might be able to use in the linker fix crate to supply a libc call?

Is there? I haven't found anything other than rand

Answer 11 · 2022-01-30T22:01:18.000Z

This API is the only thing I've found, but it includes this note at the top of the module:

This is normally not accessible to userland apps

So I'm not sure if we could use that.

BTW, I've found that searching with Google and site:libctru.devkitpro.org is a bit more effective than the builtin search on the generated documention

Edit: there are a few other APIs, sslcGenerateRandomData which is probably more for cryptographically secure randomness and I guess uses PS_GenerateRandomBytes internally. I guess the hosted docs haven't been updated to include these, since I can only find them in the headers on my system and not online, except for the wiki.

Answer 12 · 2022-01-30T23:17:30.000Z

We could implement a call to sslcGenerateRandomData into getrandom from Newlib, but that would require another libc declaration. We can make all the changes and once testing the std is over, we'll open a new PR to libc.

Answer 13 · 2022-01-31T19:04:38.000Z

Turns out time::Instant doesn't work because the impl uses libc::CLOCK_MONOTONIC, which is either wrong or nonexistent for our platform. I searched for a WHILE but didn't find anything about it. Resorted to Meziu/libc@8744910 in case you also can't find anything.

Answer 14 · 2022-01-31T20:22:34.000Z

Founded worries...
Still, I wonder if using only the REALTIME clock is a good idea or we should go with something else.

Answer 15 · 2022-01-31T20:50:00.000Z

Looking at the implementation of osGetTime (which is used by gettimeofday, which in turn is used by our clock_gettime impl for CLOCK_REALTIME), they appear to do some adjustment for clock drift in an attempt to get monotonic values, but I'm not sure if that implies the value from svcGetSystemTick() is not normally monotonic? There is also an epoch adjustment from 1900 to 1970 here for gettimeofday, but I don't think that would be strictly needed for a monotonic clock_gettime...

So maybe we could try our own impl that basically looks like this?

let now = svcGetSystemTick();
// Not sure if the math is quite right but you get the idea...
(*tp).tv_sec = now / SYSCLOCK_ARM11;
(*tp).tv_nsec = 1000 * (now - SYSCLOCK_ARM11*(*tp).tv_sec) / CPU_TICKS_PER_USEC;

I have to imagine that svcGetSystemTick() is monotonic, since I think it's supposed to be the number of clock cycles since boot, based on this docstring

Answer 16 · 2022-01-31T21:01:28.000Z

Fantastic. If it's ticks since boot, it's by definition monotonic. Though now I wonder if we may have link problems (this is the first normally available function we are actively going to reimplement).

Answer 17 · 2022-01-31T21:46:49.000Z

first normally available function we are actively going to reimplement

I'm not sure I understand, wouldn't this just go into a different match arm of our existing clock_gettime implementation? Or does this need to replace some other newlib function, or a syscall or something?

Answer 18 · 2022-01-31T21:53:24.000Z

first normally available function we are actively going to reimplement

I'm not sure I understand, wouldn't this just go into a different match arm of our existing clock_gettime implementation? Or does this need to replace some other newlib function, or a syscall or something?

Yeah, no other syscall. We just need a CLOCK_MONOTONIC match

Answer 19 · 2022-02-02T06:46:53.000Z

Fixed HashMaps in bbaf4bf

Answer 20 · 2022-02-12T04:46:52.000Z

Based on https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/std.3A.3Athread.20support.20for.20armv6k-nintendo-3ds/near/271654826, I think we should consider making a PR with what we have (or split it up into more chunks) without the thread changes in #10, and then make PRs for that and other changes later.

Answer 21 · 2022-02-12T06:43:20.000Z

We can totally do that. But first we need a couple of things. 1) We have to make a PR to libc (this includes renaming the newly added pthread functions) and make sure we won’t need much of libc in the future. 2) Check if our repo passes CI checks. GitHub doesn’t let us run them, but I wonder if we can try them anyways via CLI.

@AzureMarker @ian-h-chamberlain

Answer 22 · 2022-02-12T20:08:24.000Z

Putting this out there... A lot of the stuff we'd like to test may work "well enough" if we can use the same mechanism as ctru::test_runner, and provide a custom test runner to run std's existing tests against, on the device. (probably an unmergeable PR due to the linking to ctru required, but perhaps useful nonetheless).

I don't imagine it would be easy, especially since std uses x.py test instead of the usual cargo test, but it may be worth pursuing in relation to this issue.

Answer 23 · 2022-02-12T23:15:16.000Z

Yeah we could try running std tests, that would simplify things a lot, but I have no idea where to start implementing it. These kinds of tests are usually done for the host target.

Answer 24 · 2022-02-15T15:49:46.000Z

Priorities on the first STD PR:

Check out errors in any libc/pthread related code (this includes zombies ate my 3ds)
Test functionality of remaining modules (mainly fs and sync)
Close off unimplementable functionality (mainly process and env related functionality
Try running std tests. (This isn’t about making the testing stable or anything, but being able to try out the whole std systems in one go would be great)

Answer 25 · 2022-02-16T20:53:51.000Z

Try running std tests. (This isn’t about making the testing stable or anything, but being able to try out the whole std systems in one go would be great)

I tried this a bit yesterday, but didn't get super far along. I have a patch to getrandom, which was required for building, and now am seeing a bunch of link errors as expected for stuff like the stubs in ctru::test_runner. Hoping to make some further progress on it later this week, to at least get an executable built.

Answer 26 · 2022-02-16T22:29:07.000Z

Try running std tests. (This isn’t about making the testing stable or anything, but being able to try out the whole std systems in one go would be great)

I tried this a bit yesterday, but didn't get super far along. I have a patch to getrandom, which was required for building, and now am seeing a bunch of link errors as expected for stuff like the stubs in ctru::test_runner. Hoping to make some further progress on it later this week, to at least get an executable built.

Honestly compiling an executable would be more than enough. Getting to run them even once would give us immense info on the status of our progress.

Answer 27 · 2022-03-01T08:10:56.000Z

What’s left doing:

#16
Find a solution for #13

@AzureMarker @ian-h-chamberlain

Answer 28 · 2022-03-06T00:14:13.000Z

I think both of those are nice-to-haves, especially the path one. We could start opening a PR to libc and eventually std in the meantime. We can open follow-up PRs as needed.

Answer 29 · 2022-03-06T12:16:25.000Z

I think both of those are nice-to-haves, especially the path one. We could start opening a PR to libc and eventually std in the meantime. We can open follow-up PRs as needed.

We need to squash some commits both in libc and std (though std commits also needs to be separated in two PR’s). I don’t have much time these days, if you could do that (in a new branch) I’ll open the PR to libc.

Answer 30 · 2022-03-06T20:43:08.000Z

I made two branches for libc. I can merge them into one, but I think the first might go in easier than the second (to unblock the getrandom PR quicker):
horizon-getrandom-and-fixes - This contains fixes and the getrandom call by @ian-h-chamberlain, which unblocks rust-random/getrandom#248.
feature/horizon-pthread - This contains the changes needed for std threads support.

Feel free to push these to your repo and open the PR from there if you want. Or, I could handle the PR if you think you won't have time.

Answer 31 · 2022-03-06T22:14:11.000Z

Thank you! They look good already. You can open the PR’s yourself if you want it to go through faster, I don’t have my pc right now.

Answer 32 · 2022-03-08T01:41:43.000Z

Opened rust-lang/libc#2714 and rust-lang/libc#2715

Answer 33 · 2022-03-15T02:43:52.000Z

Well, that was pretty easy :). Both PRs are merged now and released in 0.2.120. We should update https://github.com/Meziu/libc to match the upstream (I definitely don't have permission to force-push master, so I'll leave that to @Meziu). We may want to make more changes later (I'm still eyeing tokio-console, which requires some more networking changes).

rust-random/getrandom#248 is published as well now, which isn't needed for std but is well used in third party crates (ex. through rand).

Answer 34 · 2022-03-15T03:45:43.000Z

Little update about what I've been doing: I've made a bit of progress getting std tests to compile, but not 100% there yet (some linker issue with compiler_builtins, I've asked for help on Zulip meanwhile). I can try to open a draft PR to show the changes, but it's pretty hacky and I've had to use a lot of local patches to get past compilation issues.

Side note: I also found out there is a built-in way to run std tests on a remote device, which seems like it would be useful for us, but the implementation seems to rely on forking new processes to run the tests, which makes sense but probably makes it unusable for us. Maybe it could be hacked around to work without forking.

Re #16 (comment) - I need to test again and can probably do so whenever I figure out or give up on the std testing.

Answer 35 · 2022-03-16T03:41:57.000Z

I rebased our commits on the latest rust-lang/rust changes, and split it into two branches:

feature/horizon-std: All of the changes we have for std support, besides threads.
feature/horizon-threads: std thread support, based on the feature/horizon-std branch.

This rebase also cleaned up the Cargo.lock situation (upgraded to libc 0.2.120 in the first commit) and tidied up the history slightly (removed some reverted commits, squashed/fixed-up a commit).

I can open a PR to rust-lang/rust soon for feature/horizon-std if that sounds good with everyone, @Meziu & @ian-h-chamberlain. There might still be some cleanups we want to do before that (ex. there might be something missing in MetadataExt's docs?).
Edit: looking at the diff, there's definitely some small cleanups we can do.

Once that merges we can rebase the threads branch and make another PR. In the meantime, @Meziu you could reset your rust-horizon fork's horizon-std branch to feature/horizon-threads if you'd like (or we just reset to rust-lang's master when everything merges).

Note: when testing I did run into rust-lang#94983, which I worked around by setting static-libstdcpp and consequently download-ci-llvm in my config.toml to false (there's a warning if you only set the first field)... which meant I had to build LLVM :/.

Answer 36 · 2022-03-16T06:05:05.000Z

Wait, does libc 0.2.120 Have all of our changes? Our merges happened after their bump.

Edit: I also wonder how the automatic building process works for std. Can we ship a prebuilt std only needing to link symbols afterwards (with our polyfills)?
Can a Tier 3 target even ship a prebuilt std?

Answer 37 · 2022-03-16T13:30:33.000Z

@AzureMarker - sounds good, I can help with cleanup if there is enough worth splitting the work.

@Meziu it looks like rust-lang/libc#2725 did not make it in to 0.2.120 but everything else did, based on the commit order. So perhaps we will want to bump again and update before a PR to std.

As far as I know, tier 3 platforms do not have prebuilt std (that would be tier 2 i think based on https://doc.rust-lang.org/nightly/rustc/platform-support.html#tier-2 )

Also, should we add a documentation page for 3ds as in https://doc.rust-lang.org/nightly/rustc/platform-support/TEMPLATE.html ?

Seems like the policy to start adding these happened right around the time of rustc support for 3ds was added so it wasn't required in review, but with std coming in I think it would make sense. I can write that up if you'd like and put the three of us on as maintainers?

Answer 38 · 2022-03-16T14:18:35.000Z

Seems like the policy to start adding these happened right around the time of rustc support for 3ds was added so it wasn't required in review, but with std coming in I think it would make sense. I can write that up if you'd like and put the three of us on as maintainers?

I didn’t know about those. Yeah, we three can be maintainers (maybe I should put up a GitHub group now that I think about it). About specific info, like build information, specify the need of system libraries, and that we automatically link them via cargo-3ds, without getting too deep into devkitPRO-specific features (though mentioning the need), like tools and such. The toolchain changes a lot, and I still prefer caution about what we post on Rust official repositories. Nintendo has no mercy😂.

Answer 39 · 2022-03-16T14:20:06.000Z

Once that merges we can rebase the threads branch and make another PR. In the meantime, @Meziu you could reset your rust-horizon fork's horizon-std branch to feature/horizon-threads if you'd like (or we just reset to rust-lang's master when everything merges).

Let’s just wait for Rust to pull everything and then sync with them. That tree is screwed, thanks for the cleanup.

Answer 40 · 2022-03-17T04:45:21.000Z

(maybe I should put up a GitHub group now that I think about it)

There is the https://github.com/Rust3DS group, but we'd have to talk to them and see if they're still interested or are able to transfer ownership (we should let them know of our progress in any case).

Answer 41 · 2022-03-17T13:16:34.000Z

(maybe I should put up a GitHub group now that I think about it)

There is the https://github.com/Rust3DS group, but we'd have to talk to them and see if they're still interested or are able to transfer ownership (we should let them know of our progress in any case).

I can contact @FenrirWolf later (doesn't look active on GitHub).

Answer 42 · 2022-03-17T16:22:13.000Z

Hiya folks, I'm still alive. Just haven't been very active in the 3DS space. I'll definitely have to look over everything you guys have been doing though!

I have some control over the Rust3DS github group, but iirc @HybridEidolon is the one who has full ownership and permissions over it.

Answer 43 · 2022-03-18T03:10:56.000Z

Yeah, I played around with things and I don't have very many permissions in the org. Don't think I can even add people to it myself. But hopefully we can get Eidolon's help with that.

Also do you guys coordinate on discord or something, or has it mainly been through github issues?

Answer 44 · 2022-03-18T06:01:52.000Z

Also do you guys coordinate on discord or something, or has it mainly been through github issues?

We have only communicated via GitHub for now.

Answer 45 · 2022-03-20T22:51:05.000Z

Also do you guys coordinate on discord or something, or has it mainly been through github issues?

We have only communicated via GitHub for now.

We could enable GitHub Discussions on some of the repositories to make non-issue communication easier (I don't have that permission).

Answer 46 · 2022-03-20T22:52:38.000Z

@Meziu it looks like rust-lang/libc#2725 did not make it in to 0.2.120 but everything else did, based on the commit order. So perhaps we will want to bump again and update before a PR to std.

This is now in 0.2.121. I'll bump it in the rustc fork.

Answer 47 · 2022-03-24T23:58:53.000Z

I enabled Discussions on my rustc fork at least (where we have some branches staged for the upstream PRs), if anyone's interested:
https://github.com/AzureMarker/rust-horizon/discussions

@FenrirWolf if you have any issues with getting the toolchain working, feel free to open a thread there.

Answer 48 · 2022-04-08T05:38:40.000Z

@HybridEidolon we’ve noticed people still stumble (most definitely because of google search results) in your rust3ds GitHub org, which is quite outdated. Are you willing to pass the ownership to us or make us contributors so we can port our changes to that repo and re-organize it?

Answer 49 · 2022-04-10T20:09:54.000Z

Thanks for the organization invites @HybridEidolon!

@Meziu @ian-h-chamberlain we should discuss how we should transfer and organize repos in the org. Let's talk here:
https://github.com/orgs/rust3ds/teams/group-members/discussions/3

Answer 50 · 2022-04-10T21:08:39.000Z

Opened the std PR! rust-lang#95897

Answer 51 · 2022-04-13T02:30:18.000Z

I think with that, we can close this issue? We will still need to open the std::thread PR, but that's straightforward.