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

Move const time memory comparison utils to ct_utils.h #3760

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

randombit
Copy link
Owner

No description provided.

@coveralls
Copy link

coveralls commented Oct 15, 2023

Coverage Status

coverage: 91.707% (+0.003%) from 91.704% when pulling e872668 on jack/ct-refactor into 5073121 on master.

src/lib/utils/ct_utils.h Show resolved Hide resolved
Comment on lines +334 to +347
/**
* Compare two arrays of equal size and return a Mask indicating if
* they are equal or not. The mask is set if they are identical.
*/
template <typename T>
inline CT::Mask<T> is_equal(const T x[], const T y[], size_t len) {
volatile T difference = 0;

for(size_t i = 0; i != len; ++i) {
difference = difference | (x[i] ^ y[i]);
}

return CT::Mask<T>::is_zero(difference);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be based on ranges, no? Something along these lines, with CT::Mask<> fairy-dust sprinkled in:

Suggested change
/**
* Compare two arrays of equal size and return a Mask indicating if
* they are equal or not. The mask is set if they are identical.
*/
template <typename T>
inline CT::Mask<T> is_equal(const T x[], const T y[], size_t len) {
volatile T difference = 0;
for(size_t i = 0; i != len; ++i) {
difference = difference | (x[i] ^ y[i]);
}
return CT::Mask<T>::is_zero(difference);
}
template <std::ranges::contiguous_range X, std::ranges::contiguous_range Y>
requires std::same_as<std::ranges::range_value_t<X>, std::ranges::range_value_t<Y>>
constexpr auto is_equal(X&& x, Y&& y)
{
volatile auto difference = static_cast<std::ranges::range_value_t<X>>(std::ranges::size(x) != std::ranges::size(y));
auto ix = std::ranges::begin(x);
auto iy = std::ranges::begin(y);
// TODO C++23: std::views::zip(x, y)
// future-me, please make sure that zip() is constant-time
while(ix != std::ranges::end(x) && iy != std::ranges::end(y))
{
difference |= *ix ^ *iy;
++ix;
++iy;
}
return difference;
}```

Copy link
Owner Author

Choose a reason for hiding this comment

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

Maybe so. But tbh for this kind of code I like writing things in the most brain-dead way possible because it's usually (fairly) obvious what the compiler is going to do with it.

Some span based overloads here might be nice though. As with #3757 the goal here is primarily to remove usage of functions in mem_ops.h especially when they were already redundant with other functionality.

@randombit randombit merged commit 4c6612c into master Oct 17, 2023
37 checks passed
@randombit randombit deleted the jack/ct-refactor branch October 17, 2023 12:02
This was referenced Oct 18, 2023
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.

3 participants