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

Refresh election-guard package #550

Merged
merged 2 commits into from
May 26, 2022
Merged

Refresh election-guard package #550

merged 2 commits into from
May 26, 2022

Conversation

msprotz
Copy link
Contributor

@msprotz msprotz commented May 2, 2022

Election Guard recently started using the Montgomery form API.

Copy link
Contributor

@polubelova polubelova left a comment

Choose a reason for hiding this comment

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

Thanks!

Is it ok that GenericField is not specialized by the length of bignums as it was done for Bignum4096 and Bignum256 (for both 32-bit and 64-bit versions)?

@msprotz
Copy link
Contributor Author

msprotz commented May 3, 2022

My understanding is that this is likely going to have no noticeable performance impact. If I hear back otherwise, I'll send another PR for more specialization.

There is one unfortunate outcome of generating the code for Election Guard in MSVC-compatible mode. Consider this array declaration: https://github.com/project-everest/hacl-star/blob/master/dist/gcc-compatible/Hacl_GenericField64.c#L386

This is a VLA array, i.e. an array of variable-length allocated on the stack (len1 is not known at compile-time). Normally (gcc & clang), this is supported, and the behavior is that the array is "freed" at the end of its scope. In this case, the array becomes freed at the end of the loop body (C11 6.2.4§7). So all is good and the stack doesn't explode.

However, MSVC chose not to support VLA. This means that krml cannot emit VLAs for MSVC-compatible code. In this case, krml emits a call to alloca, which unfortunately does not free the underlying resource until the whole function returns. (So, the stack grows for each loop iteration.)

This is quite an unpleasant side-effect, and I feel like we have encountered this before. One solution is to hoist this allocation out of the loop by pre-allocating it. I don't know how much work that would be, though.

@aseemr aseemr self-requested a review May 26, 2022 06:37
@msprotz msprotz merged commit 445ab28 into master May 26, 2022
@karthikbhargavan karthikbhargavan deleted the protz_election_guard branch September 6, 2022 14:47
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.

None yet

3 participants