Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Inline ASM sha1#450

Closed
laudiacay wants to merge 8 commits into
RustCrypto:master from
laudiacay:sha1-inlineasm
Closed

Inline ASM sha1 #450
laudiacay wants to merge 8 commits into
RustCrypto:master from
laudiacay:sha1-inlineasm

Conversation

@laudiacay

@laudiacay laudiacay commented Jan 29, 2023

Copy link
Copy Markdown

draft.....

Copy link
Copy Markdown
Author

partially will address RustCrypto/asm-hashes#45 per #449 (comment)

Copy link
Copy Markdown
Author

@newpavlov having a little trouble getting the q6 register back into state[0..4]. I have gotten everything else done for the aarch64 implementation (well, we'll find out for sure when I can run it).

Do you know the right syntax for this? Everything I've tried (state.as_mut_ptr(), state[0].as_mut_ptr(), state[0..4].as_mut_ptr().... several other iterations) says "cannot assign to this". I tried using the "v6.1s" syntax to individually assign u32s to indices and apparently those aren't valid registers for in/out.

problem line in question:

lateout("q6") state as *mut u32,

Copy link
Copy Markdown
Member

I am not very familiar with ARM assembly, so I can not comment on that. But lateout("q6") state as *mut u32, places pointer to the register, while I would expect you to place value from the state directly into it. The compress function should look roughly like this:

pub fn compress(state: &mut [u32; 5], blocks: &[Block<Sha1Core>]) {
 let [mut s0, mut s1, mut s2, mut s3, mut s4] = *state;
 unsafe {
 // you can use different register names
 asm!(
 // assembly impl
 inout("q0") s0,
 inout("q1") s1,
 inout("q2") s2,
 inout("q3") s3,
 inout("q4") s4,
 in("q5") blocks.as_ptr(),
 in("q6") blocks.len(),
 // clobbers
 options(pure, nostack),
 );
 }
 *state = [s0, s1, s2, s3, s4];
}

Also note that the ARM assembly relies on instructions from the crypto extension. You should add runtime detection using the cpufeatures crate and use the software backend as a fallback.

i don't know why the "do not enable by library crates" warning is in the asm feature

The idea is that the choice should be made by binary crates. Otherwise it will be hard for binary crates to change it if a library unconditionally enables such feature.

Copy link
Copy Markdown
Member

Closing as stale. Feel free to reopen it or create a new PR, if you would like to resume to work on this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /