Run builds on both Mac OS and Linux

[mgeisler]

This would have helped us catch #570.

[mgeisler]

@schultetwin1 or @ericye16, would you be able to help debug why the tests don't work on Mac OS? I was hoping they would run after #572, but now that I actually try running them, I see this error:

thread 'tests::test_nonempty_directory' panicked at 'panic in a function that cannot unwind', library/core/src/panicking.rs:126:5
[src/exercises/day-3/safe-ffi-wrapper.rs:74] path = "/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/.tmppiCrcN"
stack backtrace:
[src/exercises/day-3/safe-ffi-wrapper.rs:78] dir = 0x0000600000bf00a0
   0:        0x10c454e56 - std::backtrace_rs::backtrace::libunwind::trace::h0a908cd09b5a35f9
thread 'tests::test_nonempty_directory' panicked at 'misaligned pointer dereference: address must be a multiple of 0x8 but is 0x7f8e5e808a0c', src/exercises/day-3/safe-ffi-wrapper.rs:101:46
                               at /rustc/90c541806f23a127002de5b4038be731ba[145](https://github.com/google/comprehensive-rust/actions/runs/5320663153/jobs/9634738882?pr=830#step:5:146)8ca/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
   1:        0x10c454e56 - std::backtrace_rs::backtrace::trace_unsynchronized::hc0e7d5d16c14a788
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
   2:        0x10c454e56 - std::sys_common::backtrace::_print_fmt::hcf9ca6805c7eb2fe
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/sys_common/backtrace.rs:65:5
   3:        0x10c454e56 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hae51cb91d407e2ef
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/sys_common/backtrace.rs:44:22
   4:        0x10c4715ab - core::fmt::write::h746bc0969202388b
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/core/src/fmt/mod.rs:1254:17
   5:        0x10c451d8c - std::io::Write::write_fmt::h4098c2c7437a0bd7
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/io/mod.rs:1698:15
   6:        0x10c454c2a - std::sys_common::backtrace::_print::he6d3aef1f6c73e2d
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/sys_common/backtrace.rs:47:5
   7:        0x10c454c2a - std::sys_common::backtrace::print::h8360bf0158e89b36
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/sys_common/backtrace.rs:34:9
   8:        0x10c456c30 - std::panicking::default_hook::{{closure}}::hedf04c568eb6e0bc
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:269:22
   9:        0x10c4569e0 - std::panicking::default_hook::h62889b2c29e2347d
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:288:9
  10:        0x10c4247e6 - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::hace1e67d0d01d78f
stack backtrace:
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/alloc/src/boxed.rs:1987:9
  11:        0x10c4247e6 - test::test_main::{{closure}}::h421[153](https://github.com/google/comprehensive-rust/actions/runs/5320663153/jobs/9634738882?pr=830#step:5:154)d21ccc3479
   0: rust_begin_unwind
                               at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/test/src/lib.rs:134:21
             at /rustc/90c541806f23a127002de5b4038be731ba1458ca/library/std/src/panicking.rs:578:5
  12:        0x10c45730a - <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call::h5ccd129639ee877c

[pwnall]

I'm not repro-ing these on my M1 Mac. However, based on some searching, I think this is rust-lang/rust#111487

On my local machine, the dirent struct seems to have an 8-byte alignment. The panic is misaligned pointer dereference: address must be a multiple of 0x8 but is 0x7f8e5e808a0c and the code at src/exercises/day-3/safe-ffi-wrapper.rs:101:46 is let d_name = unsafe { CStr::from_ptr((*dirent).d_name.as_ptr()) }; where dirent comes from a readdir() call.

[pwnall]

I tried a few workarounds and they're all failing.

These comments in the stdc equivalent are a bit disheartening -- https://github.com/rust-lang/rust/blob/master/library/std/src/sys/unix/fs.rs#L694-L741

[mgeisler]

Hey @pwnall, thank you so much for jumping in with #843! This is much harder than I had imagined... If nothing else, this will be a cautionary tale for the next time I give the class 😄

@rbehjati, you also poked at this — I guess you're seeing the same crashes as @pwnall?

[domenukk]

First (stupid) question, why are you creating d_name as a CStr and then OsStr::from_bytes on d_name.to_bytes() and not create os_str from d_name directly?
Next, if you have alignment issues, the safest option might be to create a local let d_name = [0_u8; 1024] and copy the values into that local array, before using it in other rust APIs. The local variable should be aligned properly.

[mgeisler]

First (stupid) question, why are you creating d_name as a CStr and then OsStr::from_bytes on d_name.to_bytes() and not create os_str from d_name directly?

I don't know how to create an OsStr directly from a pointer. I had the impression that CStr was supposed to be used since it knows about NULL terminated strings, and then a OsStr can cheaply be constructed from it.

[domenukk]

Ok looks like you are correct, OsStr can't deal with 0 terminated strings by itself..

[domenukk]

These might be nice to have, else cargo +nightly miri test fails (at least on MacOS)

[mgeisler]

Yes, the change is not bad in itself — I would just prefer to have it in a different PR and with an explanation for the students who end up reading it later.

[rbehjati]

Hey @pwnall, thank you so much for jumping in with #843! This is much harder than I had imagined... If nothing else, this will be a cautionary tale for the next time I give the class 😄

@rbehjati, you also poked at this — I guess you're seeing the same crashes as @pwnall?

Sorry! I arrived late for the party! Glad to see that it is fixed now. I was actually getting a failed assertion error!!

failures:

---- tests::test_nonempty_directory stdout ----
[src/exercises/day-3/safe-ffi-wrapper.rs:74] path = "/var/folders/4l/r2r03g8s1d933_tplz2_6d1800qfjw/T/.tmprOZX3c"
[src/exercises/day-3/safe-ffi-wrapper.rs:78] dir = 0x0000600002a94140
thread 'tests::test_nonempty_directory' panicked at 'assertion failed: `(left == right)`
  left: `["", "", "", ".", "rab.rs"]`,
 right: `[".", "..", "bar.png", "crab.rs", "foo.txt"]`', src/exercises/day-3/safe-ffi-wrapper.rs:163:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[mgeisler]

Interesting! @domenukk showed me the same error in chat... looks like an off by one error to me, which is consistent with using the wrong layout.

[rbehjati]

LGTM :)

[rbehjati]

Looks like the CI is stuck. Not sure if re-running all jobs will fix that. We usually push a superficial commit to fix that.

[mgeisler]

Looks like the CI is stuck. Not sure if re-running all jobs will fix that. We usually push a superficial commit to fix that.

It's a bit more annoying here: I've marked the old "cargo" check as required — but this check is now replaced with two checks: "cargo (ubuntu-latest)" and "cargo (macos-latest)". Those checks are not yet required. Because we have all the translations (which also run mdbook test and thus indirectly build the Rust code), I should be able to make the "cargo" check non-required for a while until the PRs have picked up this change.

[domenukk]

Interesting! @domenukk showed me the same error in chat... looks like an off by one error to me, which is consistent with using the wrong layout.

More like an "off-by-a-lot" error with accidental memory laying around