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

Getting STATUS_ILLEGAL_INSTRUCTION on Windows #40

Closed
ImmemorConsultrixContrarie opened this issue Jan 19, 2020 · 11 comments
Closed

Getting STATUS_ILLEGAL_INSTRUCTION on Windows #40

ImmemorConsultrixContrarie opened this issue Jan 19, 2020 · 11 comments
Assignees

Comments

@ImmemorConsultrixContrarie
Copy link
Contributor

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Jan 19, 2020

OS x64 Windows, toolchain stable-x86_64-pc-windows-gnu, rustc 1.40.0 (73528e339 2019-12-16).

A subtle bug appeared during tests of my no_std lib. And since this test meets no unsafe except for rust internals and bitvec internals, I suspect there's a problem with bitvec. Maybe it's Rust fault itself, but chances are low.
Comments in the test describe the problem and this is the only file where bitvec is used. There's only six bitvec operations: call to BitBox::default() through derive, BitVec::from(BitBox), BitVec.resize(), BitVec.into_boxed_bitslice(), BitBox.get(usize), and BitBox.get_mut(usize).

Forgot to mention: all tests pass on simple "cargo test" in debug mode, and only this one test crashes on "cargo test --release".

@ImmemorConsultrixContrarie
Copy link
Contributor Author

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Jan 19, 2020

Changed get_mut() into set(), now everything is fine.

@myrrlyn
Copy link
Collaborator

@myrrlyn myrrlyn commented Jan 20, 2020

Okay that's really weird, since set and *get_mut= should have identical behavior. It looks like something is hitting a ud2 that only exists in debug? Thanks for leaving this open; this fault is a bug even if it has an immediate workaround.

@myrrlyn myrrlyn self-assigned this Jan 20, 2020
myrrlyn added a commit that referenced this issue Jan 21, 2020
@myrrlyn
Copy link
Collaborator

@myrrlyn myrrlyn commented Jan 21, 2020

I cannot yet reproduce the test failure on my machines. I do not currently have access to a Windows machine, and running cargo test --release on my macOS and Linux machines, on the commit described, does not fail for me.

Can you try patching your worktree to use my WIP branch? You can do so in your manifest with

# Cargo.toml

[patch.crates-io.bitvec]
git = "https://github.com/myrrlyn/bitvec"
branch = "hotfix/40"

or, if that fails, by applying

diff --git a/src/slice/api.rs b/src/slice/api.rs
index e19d038..b7de50f 100644
--- a/src/slice/api.rs
+++ b/src/slice/api.rs
@@ -2254,7 +2254,11 @@ where O: 'a + BitOrder, T: 'a + BitStore {
 	) -> Self::Mut {
 		BitMut {
 			data: *slice.get_unchecked(self),
-			slot: slice.get_unchecked_mut(self ..= self),
+			slot: {
+				let mut bitptr = slice.get_unchecked_mut(self ..).bitptr();
+				bitptr.set_len(1);
+				bitptr.into_bitslice_mut()
+			},
 		}
 	}

to the copy of bitvec in your dependency tree? This is the worse option to actually do, however.

@ImmemorConsultrixContrarie
Copy link
Contributor Author

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Jan 21, 2020

if that fails

Still produce the same error. cargo clean, added patch, cargo test --release, same error.
Cargo clean again, deleted patch from toml, changed 2257 line of ...\bitvec-0.17.1\src\slice\api.rs into

slot: {
		let mut bitptr = slice.get_unchecked_mut(self ..).bitptr();
		bitptr.set_len(1);
		bitptr.into_bitslice_mut()
	},

still same error.

@myrrlyn
Copy link
Collaborator

@myrrlyn myrrlyn commented Jan 21, 2020

uhhh. okay. and using set avoids it? that's really strange

@ImmemorConsultrixContrarie
Copy link
Contributor Author

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Jan 21, 2020

uhhh. okay. and using set avoids it? that's really strange

Yep.

@myrrlyn
Copy link
Collaborator

@myrrlyn myrrlyn commented Jan 23, 2020

Would you mind testing if current master (as of b87a81e) resolves your problem when using get_mut instead of set? I have not been able to get a target x86_64-pc-windows-gnu operational yet, or I would check locally.

I unfortunately still don't know what's actually causing the behavior, so if your changes to BitMut don't affect it, I may have to "resolve" this with only a warning in the documentation. Thanks again for bringing it to my attention, even if I'm unable to get it closed.

@ImmemorConsultrixContrarie
Copy link
Contributor Author

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Jan 23, 2020

Okay, it's fixed. Hella magic!

@myrrlyn
Copy link
Collaborator

@myrrlyn myrrlyn commented Jan 23, 2020

I profoundly wish I understood why this solved it, but I'm glad it did. Thanks for the report and test!

@ImmemorConsultrixContrarie
Copy link
Contributor Author

@ImmemorConsultrixContrarie ImmemorConsultrixContrarie commented Jan 23, 2020

Okay, just retested, it is definetly about some change from .17.1, cuz 17.1 from crates still hangs with this error and master-branch passes tests. Yeah, whatever, this is already closed anyway.

Hella magic!

@myrrlyn
Copy link
Collaborator

@myrrlyn myrrlyn commented Jan 23, 2020

I've published 0.17.2 to crates.io with the changes we've made today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants