Skip to content

Make Miri backtraces work with #[global_allocator] #462

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 19, 2022
Merged

Make Miri backtraces work with #[global_allocator] #462

merged 1 commit into from
May 19, 2022

Conversation

beepster4096
Copy link
Contributor

Supports changes in rust-lang/miri#1975

@beepster4096 beepster4096 mentioned this pull request Feb 13, 2022
Merged
alexcrichton
alexcrichton reviewed Feb 14, 2022
View reviewed changes
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable to me, but how would you like this to get landed? The CI here is failing presumably because miri needs an update, but does the lack of changes here block a miri update?

@beepster4096
Copy link
Contributor Author

The CI here is failing presumably because miri needs an update, but does the lack of changes here block a miri update?

The new approach should avoid this blocking problem by being backwards compatible with old code

bors added a commit to rust-lang/miri that referenced this pull request Mar 20, 2022
@bors
Auto merge of #1975 - DrMeepster:backtrace_fix, r=RalfJung
5778667 
Make backtraces work with #[global_allocator]

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
@beepster4096
Copy link
Contributor Author

Alright, should be updated for the final version of rust-lang/miri#1975

@beepster4096
Copy link
Contributor Author

But I forgot that this won't pass yet until next nightly with miri on it 🙃

@beepster4096 beepster4096 mentioned this pull request Mar 20, 2022
Closed
@RalfJung RalfJung mentioned this pull request Mar 20, 2022
Merged
@RalfJung
Copy link
Member

First the Miri submodule needs to be updated. :) rust-lang/rust#95144 does that.

@RalfJung
Copy link
Member

@drmeepster you could try rebasing so that CI picks up the new Miri? Then we'll see if this works as it should.

@beepster4096
Copy link
Contributor Author

Whoops! I forgot about this completely, sorry.

@RalfJung
Copy link
Member

Miri CI is green, so LGTM. :) But I cannot land PRs here.

@alexcrichton alexcrichton merged commit 4e5a3f7 into rust-lang:master May 19, 2022
@RalfJung
Copy link
Member

🎉 :-)

@alexcrichton what does it take to get this into rustc?

@alexcrichton
Copy link
Member

I believe the library/backtrace needs to be updated in the repository and that's probably it

@RalfJung RalfJung mentioned this pull request May 20, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request May 28, 2022
@bors
Auto merge of rust-lang#97207 - RalfJung:backtrace, r=Mark-Simulacrum
1fede17 
update libbacktrace

It seems like previously we were on a tag of the backtrace repo; not sure if there is a policy that it should always be a tag?

Cc rust-lang/backtrace-rs#462 `@alexcrichton` `@DrMeepster`
workingjubilee pushed a commit to tcdi/postgrestd that referenced this pull request Sep 15, 2022
@bors
Auto merge of #97207 - RalfJung:backtrace, r=Mark-Simulacrum
b47a6b3 
update libbacktrace

It seems like previously we were on a tag of the backtrace repo; not sure if there is a policy that it should always be a tag?

Cc rust-lang/backtrace-rs#462 `@alexcrichton` `@DrMeepster`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@beepster4096 @RalfJung @alexcrichton