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

Sha1 and Sha2 cleanups & optimizations, take 2 #8174

Closed
wants to merge 9 commits into from

Conversation

DaGenix
Copy link

@DaGenix DaGenix commented Aug 1, 2013

Same content as #8097, but bors had an issue with that pull request. Opening a new one.

@brson
Copy link
Contributor

brson commented Aug 2, 2013

@DaGenix Needs rebase or merge.

@DaGenix
Copy link
Author

DaGenix commented Aug 2, 2013

@brson - rebased

Palmer Cox added 9 commits August 2, 2013 18:46
The code was arranged so that the core Sha2 code came first, and then
all of the various implementation of Digest followed along later. The
problem is that the Sha512 compression function code is far away from
the Sha512 Digest implementation, so, if you are trying to read over
the code, you need to scroll all around the file for no good reason. The
code was rearranged so that all of the Sha512 code is in one place and
all of the Sha256 code is in another and so that all impls for a struct
are near the definition of that struct.
The result_X() methods just calculate an output of a fixed size. They don't
really have much to do with running the actually hash algorithm until the very
last step - the output. It makes much more sense to put all this logic into
the Digest impls for each specific variation on the hash function.
…f it.

There are 2 main pieces of functionality in cryptoutil.rs:
* A set of unsafe function for efficiently reading and writing u32 and u64
  values. All of these functions are fairly easy to audit to confirm that
  they do what they are supposed to.
* A FixedBuffer struct. This struct keeps track of input data until there
  is enough of it to execute the a function on it which expects a fixed
  block of data.

The Sha2 module was rewritten to take advantage of the new functions in
cryptoutil as well as FixedBuffer. The result is that the duplicate code
for maintaining a buffer of input data is removed from the Sha512 and
Sha256 implementation. Additionally, the FixedBuffer code is much more
efficient than the previous code was.
The Sha2 compression functions were re-written to execute the message
scheduling calculations in the same loop as the rest of the compression
function. The compiler is able to generate much better code. Additionally,
innermost part of the compression functions were turned into macros to
reduce code duplicate and to make the functions more concise.
Create a helper function in cryptoutil.rs which feeds 1,000,000 'a's into
a Digest with varying input sizes and then checks the result. This is
essentially the same as one of Sha1's existing tests, so, that test was
re-implemented using this method. New tests were added using this method for
Sha512 and Sha256.
Added functions to cryptoutil.rs that perform an addition after shifting
the 2nd parameter by a specified constant. These function fail!() if integer
overflow will result. Updated the Sha2 implementation to use these functions.
@DaGenix
Copy link
Author

DaGenix commented Aug 2, 2013

@brson - rebased


// Putting the message schedule inside the same loop as the round calculations allows for
// the compiler to generate better code.
for uint::range_step(0, 64, 8) |t| {
Copy link
Contributor

Choose a reason for hiding this comment

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

These two loops will need to be changed to do blocks with a true at the end, because for is being denied by lint.

bors added a commit that referenced this pull request Aug 3, 2013
Same content as #8097, but bors had an issue with that pull request. Opening a new one.
@bors bors closed this Aug 3, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Mar 14, 2022
new lint: `missing-spin-loop`

This fixes rust-lang#7809. I went with the shorter name because the function is called `std::hint::spin_loop`. It doesn't yet detect `while let` loops. I left that for a follow-up PR.

---

changelog: new lint: [`missing_spin_loop`]
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.

5 participants