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

Add iterator debugging to CI #4413

Merged
merged 4 commits into from
Nov 2, 2024
Merged

Add iterator debugging to CI #4413

merged 4 commits into from
Nov 2, 2024

Conversation

randombit
Copy link
Owner

@randombit randombit commented Oct 27, 2024

See #3737 #3840

@randombit
Copy link
Owner Author

randombit commented Oct 27, 2024

Can't repro the CI failures locally , this may be something in the TPM2 code. Will investigate more later.

Not TPM2, the fuzzers job is also failing and that doesn't run the TPM2 tests.

Seems to be ffi_aead test since skipping that test allows the main test suite to pass.

@randombit randombit force-pushed the jack/add-iterator-checks branch 3 times, most recently from a9f7357 to 73ef2ef Compare October 28, 2024 22:13
@coveralls
Copy link

coveralls commented Oct 28, 2024

Coverage Status

coverage: 91.074% (+0.002%) from 91.072%
when pulling 082bdea on jack/add-iterator-checks
into bd045b1 on master.

Copy link
Collaborator

@reneme reneme left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for picking this up. :)

Comment on lines +555 to +557
if(rng->next_byte() & 1) {
a.set_bit(i);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Off topic: should we introduce something like rng->next_bit() -> CT::Choice? Not sure whether that would be useful in production code.

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? I can't think of cases currently where we predicate on a single random bit (vs say the even-y bit of a EC point). Somewhere in the PQ algos perhaps?

Copy link
Collaborator

@reneme reneme Nov 1, 2024

Choose a reason for hiding this comment

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

Not really. They all just pull a seed and then sample from KDFs or XOFs. I didn't find something with an ad-hoc search either.

For some of those samplings, we have to pull a number of bits that isn't aligned by 8 from the XOF (without losing the other bits). For instance, this is generating uint16_ts that are comprised of 12 bits pulled from the XOF. This required some generator adapters to pull off.

const auto [d1, d2] = bounded_xof.next<3>([&](const auto bytes) {
const auto x = load_le3(bytes);
return std::pair{lowerthan_q(x & 0x0FFF), lowerthan_q(x >> 12)};
});

But introducing a bit-oriented API just for this? Not really, I think.

The iterator checks failed when we attempted to use first to extract
ideal_granularity bytes out of dummy_buffer, but on testing it failed
exactly when ideal_granularity was 256 bytes, the same size as
dummy_buffer itself.

Increase the size and add an assert that the buffer is large enough.
This is not helpful in any case, and I ran into some very confusing
situations attempting to replicate iterator errors on Ubuntu 24.04
apparently caused by ccache not noticing that the inputs had changed.
Even without iterator checks, running the tests with 8192 bits takes
35 seconds on a reasonably fast machine, and it takes over 5 minutes
with GCC iterator checks enabled.

While there also randomize the test input
@randombit randombit merged commit 3b8a73d into master Nov 2, 2024
65 checks passed
@randombit randombit deleted the jack/add-iterator-checks branch November 2, 2024 00:30
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