Skip to content
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.

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

test cleanup for nightly #641

Merged
merged 5 commits into from
Jul 28, 2024
Merged

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Jul 15, 2024

A collage of random bugfixes.

@workingjubilee workingjubilee force-pushed the appease-miri branch 2 times, most recently from e70fddd to ae89807 Compare July 15, 2024 04:07
@ChrisDenton
Copy link
Member

failures:

---- backtrace_new_should_start_with_call_site_trace stdout ----
  0: backtrace_new_should_start_with_call_site_trace

...they're the same picture???

@workingjubilee workingjubilee changed the title Try to fix the miri CI test cleanup for miri and windows Jul 27, 2024
@workingjubilee workingjubilee changed the title test cleanup for miri and windows test cleanup for miri Jul 27, 2024
@workingjubilee
Copy link
Member Author

...yeah I have no idea why this is an unused_unsafe now :ferrisClueless:

            pub fn dbghelp(&self) -> *mut Dbghelp {
                unsafe {
                    ptr::addr_of_mut!(DBGHELP)
                }
            }

certainly someone didn't actually implement and merge rust-lang/rust#125834

@workingjubilee workingjubilee force-pushed the appease-miri branch 2 times, most recently from 6c6b1cb to ddcbd97 Compare July 27, 2024 22:16
use a few less sus casts for skip_inner_frames and use inline(never)
to appease miri. miri now tries to pretend this frame may differ.
stable doesn't know yet, so just allow the lint.
@workingjubilee workingjubilee force-pushed the appease-miri branch 2 times, most recently from 1d56ec5 to b64e37f Compare July 27, 2024 22:33
This allows us to both
- retain provenance throughout
- describe why these are Send and Sync
- progressively reduce errors by reducing the conflation of pointers
@workingjubilee workingjubilee changed the title test cleanup for miri test cleanup for nightly Jul 27, 2024
Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

Looks good to me. One nit but r=me even without it.

src/dbghelp.rs Show resolved Hide resolved
Comment on lines -48 to +50
let a = thread::spawn(move || {
while !done2.load(SeqCst) {
format!("{:?}", backtrace::Backtrace::new());
let a = thread::spawn(move || loop {
if done2.load(SeqCst) {
break format!("{:?}", backtrace::Backtrace::new());
Copy link
Member

Choose a reason for hiding this comment

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

The new version looks right to me. Or at least I do not understand why the old version was doing that.

@ChrisDenton
Copy link
Member

Ok after re-reviewing with coffee in hand, let's do this...

@ChrisDenton ChrisDenton merged commit 3ad7f66 into rust-lang:master Jul 28, 2024
41 checks passed
@workingjubilee workingjubilee deleted the appease-miri branch July 28, 2024 21:51
@workingjubilee
Copy link
Member Author

Completely understandable. Thank you!

if sp == 0 {
let sp = frame.sp();
eprintln!("sp = {sp:p}");
if sp as usize == 0 {
Copy link
Member Author

Choose a reason for hiding this comment

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

...why didn't I just write is_null() lol

Copy link
Member

Choose a reason for hiding this comment

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

eugh, I really shouldn't have reviewed that day even with a dose of caffeine

ConradIrwin referenced this pull request in zed-industries/zed Sep 19, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [backtrace](https://redirect.github.com/rust-lang/backtrace-rs) |
dependencies | patch | `0.3.73` -> `0.3.74` |
| [backtrace](https://redirect.github.com/rust-lang/backtrace-rs) |
dev-dependencies | patch | `0.3.73` -> `0.3.74` |

---

### Release Notes

<details>
<summary>rust-lang/backtrace-rs (backtrace)</summary>

###
[`v0.3.74`](https://redirect.github.com/rust-lang/backtrace-rs/releases/tag/0.3.74)

[Compare
Source](https://redirect.github.com/rust-lang/backtrace-rs/compare/0.3.73...0.3.74)

#### What's Changed

- QNX Neutrino 7.0 support, thanks to
[@&#8203;nyurik](https://redirect.github.com/nyurik) in
[https://github.com/rust-lang/backtrace-rs/pull/648](https://redirect.github.com/rust-lang/backtrace-rs/pull/648)
- Cleaned up our Android support. This should massively improve
backtraces for ones with the API level sufficient to ship with
libunwind, etc. Unfortunately, it comes at the cost of dropping support
for older ones! Thanks to
[@&#8203;fengys](https://redirect.github.com/fengys) in
[https://github.com/rust-lang/backtrace-rs/pull/656](https://redirect.github.com/rust-lang/backtrace-rs/pull/656)
- Made PrintFmt, which was using the `Enum::__NonExhaustiveVariant`
pattern, use `#[non_exhaustive]` for real. Don't @&#8203; me if you were
matching on that! Thanks to
[@&#8203;nyurik](https://redirect.github.com/nyurik) in
[https://github.com/rust-lang/backtrace-rs/pull/651](https://redirect.github.com/rust-lang/backtrace-rs/pull/651)
- Massively cleaned up the windows code! We moved from winapi to
windows-sys with windows-targets thanks to
[@&#8203;CraftSpider](https://redirect.github.com/CraftSpider) and
[@&#8203;ChrisDenton](https://redirect.github.com/ChrisDenton) in
- Don't cast HANDLE to usize and back by
[@&#8203;CraftSpider](https://redirect.github.com/CraftSpider) in
[https://github.com/rust-lang/backtrace-rs/pull/635](https://redirect.github.com/rust-lang/backtrace-rs/pull/635)
- Switch from `winapi` to `windows-sys` by
[@&#8203;CraftSpider](https://redirect.github.com/CraftSpider) in
[https://github.com/rust-lang/backtrace-rs/pull/641](https://redirect.github.com/rust-lang/backtrace-rs/pull/641)
- Update windows bindings and use windows-targets by
[@&#8203;ChrisDenton](https://redirect.github.com/ChrisDenton) in
[https://github.com/rust-lang/backtrace-rs/pull/653](https://redirect.github.com/rust-lang/backtrace-rs/pull/653)
- A bunch of updated dependencies. Thanks
[@&#8203;djc](https://redirect.github.com/djc) and
[@&#8203;khuey](https://redirect.github.com/khuey)!
- Sorry if you were testing this code in miri! It started yelling about
sussy casts. A lot. We did a bunch of internal cleanups that should make
it quiet down, thanks to
[@&#8203;workingjubilee](https://redirect.github.com/workingjubilee) in
[https://github.com/rust-lang/backtrace-rs/pull/641](https://redirect.github.com/rust-lang/backtrace-rs/pull/641)
- Uhhh we had to tweak `dl_iterate_phdr` in
[https://github.com/rust-lang/backtrace-rs/pull/660](https://redirect.github.com/rust-lang/backtrace-rs/pull/660)
after Android revealed it was... kind of unsound actually and not doing
things like checking for null pointers before making slices! WHOOPS!
Thanks to [@&#8203;saethlin](https://redirect.github.com/saethlin) for
implementing detection for precisely that in rustc! It's really hard to
find soundness issues in inherited codebases like this one...

#### New Contributors

- [@&#8203;CraftSpider](https://redirect.github.com/CraftSpider) made
their first contribution in
[https://github.com/rust-lang/backtrace-rs/pull/635](https://redirect.github.com/rust-lang/backtrace-rs/pull/635)
- [@&#8203;fengys1996](https://redirect.github.com/fengys1996) made
their first contribution in
[https://github.com/rust-lang/backtrace-rs/pull/656](https://redirect.github.com/rust-lang/backtrace-rs/pull/656)
- [@&#8203;djc](https://redirect.github.com/djc) made their first
contribution in
[https://github.com/rust-lang/backtrace-rs/pull/657](https://redirect.github.com/rust-lang/backtrace-rs/pull/657)

**Full Changelog**:
rust-lang/backtrace-rs@0.3.73...0.3.74

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 3pm on Wednesday" in timezone
America/New_York, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

Release Notes:

- N/A

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants