Improve handling of failed operations in isolation mode

[Aaron1011]

Currently, we abort program execution when we try to access the external environment in isolation mode. However, libstd currently calls getcwd when printing out a panic backtrace. To ensure that panicking works in isolation mode, we've disabled the call to getcwd when libstd has cfg(miri) enabled.

Eventually, it would be good to remove this hack, and start returning proper error codes from disabled functions, rather than aborting the process.

See https://github.com/rust-lang/rust/pull/60026#discussion_r342257352 for more discussion

[RalfJung]

@oli-obk I'd be interested in your opinion here: right now we abort execution on shims that are unsupported due to isolation; if we instead continue execution pretending the function errored that might be unexpected. Though OTOH this is what "normal" sandboxes do so I think it's actually expected. We could emit a warning the first time this happens -- this might be a good test project for https://github.com/rust-lang/miri/issues/797.

[oli-obk]

hmm... we do this already for all the pthread code, so we do have precedent. I'm fine with it and I don't think we'd need a warning. I would expect a sandboxed open call to error after all.

[Aaron1011]

If we decide to make disabled functions return an error code, I think we should have a command-line flag to opt in to the old behavior. There should be a way of running Miri such that either:

1. The program output is identical to the output when run normally (i.e. compiled by rustc and executed), or:
2. We terminate the program with an explicit Miri error.

[oli-obk]

While I personally don't see a reason for such a flag, I won't block it if there is strong desire for it.

We've been doing quite well with just having unsupported functions return even success return values and do nothing. Going a step further and reporting error return values for unsupported functions where the caller has a fallback or otherwise useful code path seems totally alright to me.

[RalfJung]

While I personally don't see a reason for such a flag, I won't block it if there is strong desire for it.

Same.

The program output is identical to the output when run normally (i.e. compiled by rustc and executed)

Note that we cannot guarantee this due to things like pointer-integer casts, or libc functions that return the stack size (we just return some constant, 16k if I remember correctly). And we already don't try to be faithful for most things "implemented" in foreign_items.rs, we just make these good enough such that things work.

[RalfJung]

Here's another example where it would be good if isolated operations returned failure instead of aborting execution: https://github.com/seanmonstar/num_cpus/pull/96 makes num_cpus open a file (rejected with isolation) but falls back to another way of determining the CPU count (which works in isolation).

[camshaft]

This would be great to have for my usecase. I'm wanting to add miri support to https://github.com/camshaft/bolero and wanted to read the fuzz corpus from the filesystem, if possible, otherwise fall back to only generating some random tests cases with a warning saying to disable isolation if desired.

[camelid]

I'm going to try working on this.

[RalfJung]

@camelid that's great. :-)

[henryboisdequin]

@camelid Are you still working on this or can I take a shot at this?

[camelid]

@henryboisdequin I never ended up getting to this, so you can try!

[atsmtat]

Hi @RalfJung , I opened a PR fixing get and set cwd ops. Let me know what you think. Also, this is my first PR in rustc related packages. If you find something missing from the usual workflow, please let me know 🙂

[RalfJung]

With https://github.com/rust-lang/miri/pull/1797 we now support returning EPERM for some operations when isolation is enabled, instead of aborting execution. What is left to do is do this for as many operations as possible. :) It might be good to assemble a list.

[atsmtat]

I guess most of the fs operations can return EPERM.

[RalfJung]

Yes, I think that would make a lot of sense.

[atsmtat]

Yes, I think that would make a lot of sense.

#1838 takes care of fs shims.

[RalfJung]

With https://github.com/rust-lang/miri/pull/1838 landing soon, the remaining uses of check_no_isolation seem to be

• the shims in time.rs
• pthread_cond_timedwait
• FUTEX syscall with a timeout

[yunji-yunji]

I got the above error when trying to read the input of my fuzzer from a file. I think isolation is disabled using MIRIFLAGS, and the fs.rs file seems up to date. Can I still read a file in my main.rs?

• command ./x.py run miri --args ../../hello_world/src/main.rs

• Error message

• main.rs

• git log library/std/src/sys/unix/fs.rs (It seems up to date.)

• MIRIFLAGS

[saethlin]

Try adding -Zmiri-disable-isolation to your args.

The -Z flags for Miri just like the -Z and -C flags for rustc are command-line arguments (Miri is basically a hacked-up rustc). MIRIFLAGS lets you pass those arguments through cargo-miri just like RUSTFLAGS lets you pass them through cargo. You aren't going through Cargo so nothing is looking at that environment variable.

[yunji-yunji]

Thank you so much! The problem is solved with your support.

[RalfJung]

In the future, please open a new issue unless you are sure this is the same problem as an existing one. In this case your program had nothing to do with what we are discussing in this issue.