Don't normalize paths in backtraces to be relative to the cwd.

[EdSchouten]

Though this code is well intended, it is the only place in the backtrace
code where we depend on local file system interaction. By adding this
normalization, it becomes harder to use this code in environments where
there is no current working directory (i.e., freestanding environments
or sandboxed environments, such as CloudABI).

I suspect that printing absolute paths here is also useful to prevent
confusion and to make it easier to copy-paste and use paths between
separate terminal sessions.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[sfackler]

Why is this a problem on targets without a filesystem? Wouldn't env::current_dir just return an error and everything would work fine?

[EdSchouten]

Hi Steven,

That is a good question!

With CloudABI's C/C++ environment, we've so far taken the path of omitting features that cannot be implemented sanely in our environment. For example, we don't provide open() to open files, but we do provide openat() (also part of POSIX). The latter allows for capability-based sandboxing, whereas the former does not. Though removing this tends to break a lot of existing code out there, we've chosen this approach for two reasons:

1. It makes the task of porting software to CloudABI a lot more guided. Instead of being able to build easily but ending up with a non-functional binary, the compiler errors of absent features will act as a todo list of what needs to be altered for the application to work. It is easier to deal with build errors than to require tracers and debuggers to figure out why your application does not work. I suspect people in the Rust community share that sentiment.

   People on the CloudABI project have first-hand experience that this speeds up the process of porting software to CloudABI significantly.

2. Even though you could argue that it should, software doesn't tend to be implemented to deal with the absence of features gracefully. One example that we've seen in the C/C++ ecosystem more than once:

   int fd = open("/dev/urandom", O_RDONLY);
   if (fd >= 0) {
       // Use strong PRNG based on data from the random device.
   } else {
       // Use weak PRNG based on getpid() and gettimeofday().
   }
   

   It would be catastrophic if open() were implemented as a function that always returns ENOSYS.

As Rust is a compiled language, just like C/C++, I would strongly suggest to continue with this approach with Rust+CloudABI as well. Unfortunately, it seems that libstd's file system bits have not been designed with POSIX *at() support in mind, which is why I've resorted to disabling std::path, std::fs, etc. for the time being, hence this pull request.

My hope is that this work will eventually spawn a discussion about what an *at()-based file system API for Rust may look like. Work in this area will not only benefit users of CloudABI, it will also make race-free file system traversal, injectability of file system access, etc. easier in general.

Back to the change at hand: if this feature is considered important enough to justify a dependency on file systems, I can also make this conditional on cfg(target_os = "cloudabi"). Would that be preferred? If so, just let me know and I'll update the patch.

[sfackler]

It is totally unacceptable to make the behavior of mainstream targets worse because it's more convenient to implement something for a new experimental target.

[EdSchouten]

Whether this change makes things better or worse is purely a matter of taste, right? Regardless of supporting CloudABI, I always prefer if full paths are displayed, as this makes working with multiple terminals, a separate editor/IDE, etc. easier. I tend to copy-paste paths around a lot: open new terminal, vim <paste path from trace>. This is why I sent out this pull request in its current form.

Do you want me to make this conditional on cfg(target_os = "cloudabi")?

[sfackler]

That is a way to do it, yes.

[EdSchouten]

Done. Thanks for the quick replies so far!

[EdSchouten]

Oh, wait. We're not allowed to mix generic code with target specific code in sys_common/backtrace.rs.

Now I'm starting to get a bit puzzled about what's the nicest way to support this. Would it make sense to push this logic into foreach_symbol_fileline(), that its callback gains an additional argument? As in, let it provide |short_file, long_file, line| instead of |file, line|.

[shepmaster]

Happy new year from triage, @sfackler! Will you be able to finish checking this out sometime?

[EdSchouten]

Let's close this pull request, as this change will become (temporarily) unnecessary after #47268 lands. Thanks for your input anyway!