Make backtraces work with #[global_allocator]

[beepster4096]

Currently, backtraces break when the global allocator is overridden because the allocator will attempt to deallocate memory allocated directly by Miri.

This PR fixes that by using a new memory kind and providing a function to deallocate it. We can't call the custom allocator to allocate because it's not possible to call a function in the middle of a shim.

This PR fixes that by adding a new version of the backtrace API accessible by setting flags to 1. Existing code still functions.

backtrace-rs PR: rust-lang/backtrace-rs#462

Fixes #1996

[oli-obk]

So, afaict in order to land this you'll need to remove the backtrace memorykind again, then land it here, then update backtrace-rs, then bump everything in rustc, then add the memory kind again.

Does that sound about right?

[beepster4096]

Either that or special-casing this new memory kind so that it can be deallocated by the default allocator and removing that later, I think.

[RalfJung]

We can't call the custom allocator to allocate because it's not possible to call a function in the middle of a shim.

Well, it is possible in principle, but it is really rather annoying...

Is there any way that we could change the backtrace API so that allocation is done by the user? Like, the user has to first call miri_get_backtrace_size, then create an allocation of that size, and then hand that to miri_get_backtrace?

[beepster4096]

Is there any way that we could change the backtrace API so that allocation is done by the user? Like, the user has to first call miri_get_backtrace_size, then create an allocation of that size, and then hand that to miri_get_backtrace?

Yeah, that's a much better idea actually. Since this would involve completely rewriting the PR, should I open a new PR or just force push over this one?

[oli-obk]

Just force pushing is fine

[RalfJung]

In terms of migrating to the new API, we do have a flags argument -- so we could define our first flag, which indicates a pre-allocated buffer, and then only expect a 2nd argument if the flag is set appropriately. That way Miri can support both old and new clients of this API.

(open does something similar in POSIX, so such "optional arguments" actually seem possible in practice. And anyway we fully control the 'calling convention' here.)

[beepster4096]

When updating the readme, should I keep documentation for the old api, or replace it completely with the new api?

[RalfJung]

When updating the readme, should I keep documentation for the old api, or replace it completely with the new api?

We want people to use the new one so probably it makes sense to only document that.

[beepster4096]

Sorry that I took so long. The changes are done.

[RalfJung]

Sorry that I took so long. The changes are done.

No worries. :)
I left a few more comments, just some nits. This is almost ready to go!

[RalfJung]

Awesome, looks good. :) Can you squash the commits together?

[beepster4096]

Whoops, forgot about the CI failure

[RalfJung]

Thanks a lot for this PR. :-)
@bors r+

[bors]

📌 Commit 2c670b1 has been approved by RalfJung

[bors]

⌛ Testing commit 2c670b1 with merge 5778667...

[bors]

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing 5778667 to master...