Skip to content

Remove the redundant MDExceptionCodeLinux enum and use the values from minidump-common instead#62

Merged
Jake-Shadle merged 3 commits intorust-minidump:mainfrom
gabrielesvelto:remove-redundant-enum
Nov 4, 2022
Merged

Remove the redundant MDExceptionCodeLinux enum and use the values from minidump-common instead#62
Jake-Shadle merged 3 commits intorust-minidump:mainfrom
gabrielesvelto:remove-redundant-enum

Conversation

@gabrielesvelto
Copy link
Copy Markdown
Contributor

No description provided.

@Jake-Shadle
Copy link
Copy Markdown
Collaborator

So interesting, there seems to be a difference between 1.64 and 1.65 on Windows, I could build and test successfully on Windows with this PR on 1.64.0 but then switched to 1.65.0 and got the same crash as CI did, so at least it's fully reproducible.

@Jake-Shadle
Copy link
Copy Markdown
Collaborator

So it looks like the dump_current_process test crashes inside RtlCaptureContext, I've actually had issues with that function in the past, I think I've found a workaround for now which is to build and run test in release, but I'll add an issue to look into this for later, but considering that code path only gets hit in the not-recommended-use-at-your-own-risk dump_local_context I wouldn't consider it super high priority, though still worrying that (seemingly) a rust update changed...something.

@Jake-Shadle Jake-Shadle merged commit 42c74f1 into rust-minidump:main Nov 4, 2022
@gabrielesvelto
Copy link
Copy Markdown
Contributor Author

That function does magic stuff with the stack so it's possible LLVM is hitting some edge-case where it generates incompatible code. Anyway WDYT about that clippy warning? I suppose we could Box<> the (large) errors but it would change the external API so I didn't want to go there just to appease it (even though it would have other benefits).

@Jake-Shadle
Copy link
Copy Markdown
Collaborator

I just wanted to unblock this change for now so I silenced it, but I think before the next release of this crate it would be good to actually amp up the clippy lints and actually address all of them or at least have better reasoning for why certain ones get silenced since overall it makes things better.

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