-
Notifications
You must be signed in to change notification settings - Fork 505
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
HQC failing on ALT Linux and gcc-12 #1244
Comments
The key line in http://git.altlinux.org/beehive/logs/Sisyphus/x86_64/archive/2022/0702/error/liboqs-0.7.1-alt2 is:
And it is very surprising: It seems the final (decaps) operation of this single algorithm (HQC-128) is failing. If nothing in the code base was changed (?) this should not happen. Can you manually reproduce this error by running these commands (and sharing your output):
If this also fails, please also run Finally, does the current |
Yes this is ALT Linux and it's daily rebuild of the package which previously built without errors. Package is build from the tag
This is not raw upstream repository, but a repo with RPM spec which builds this package, but you can see it's merged from
Will report that a bit later. |
Ok merged
Rerun of three |
Quick $ grep pqclean_hqc-rmrs-128_avx2/kem log2
|
Thanks for these logs and reconfirmations. So in summary: you do a daily build with 0.7.1 (unchanged for quite some time) and suddenly HQC started to fail?! The most prominent thing that changed recently was the OpenSSL update. Would HQC work OK if you built |
I just checked the logs, and we started getting re-build failures on June 2, the day we switched to gcc-12 as default compiler. I just tried to rebuild the package (with
Rebuild with |
I thought this may be related to LTO (
There is 3 errors like this in
This is perhaps the cause of test problems. |
Thanks very much for these additional checks. Yes, it seems to be related to that file and that compiler (version). Given there's not a lot of algorithm maintenance (fixing such bugs) going on until NIST decides which algorithm(s) are eliminated from consideration for standardization, here's my workaround suggestions until this "low maintenance mode" is lifted:
Edit/Add: Looking at #995 HQC seems to be in an exceptionally "deep" "low maintenance" mode .... If I were you, I'd go for option 1, thus. |
Yes let's wait July 5 for the NST announcement. I also produced my workaround: diff --git a/src/kem/hqc/pqclean_hqc-rmrs-128_avx2/reed_muller.c b/src/kem/hqc/pqclean_hqc-rmrs-128_avx2/reed_muller.c
index 85afd331..77339b33 100644
--- a/src/kem/hqc/pqclean_hqc-rmrs-128_avx2/reed_muller.c
+++ b/src/kem/hqc/pqclean_hqc-rmrs-128_avx2/reed_muller.c
@@ -331,6 +331,7 @@ inline uint32_t find_peaks(__m256i *transform) {
tmp = _mm256_or_si256(tmp, _mm256_and_si256(vect_mask, transform[i]));
}
result = 0;
+#pragma GCC unroll 16
for (size_t i = 0; i < 16; i++) {
mask = ~(uint32_t) ((-(int64_t)(i ^ message % 16)) >> 63);
result |= mask & ((uint16_t *)&tmp)[i];
diff --git a/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c b/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c
index dbfd6a29..8fb506cb 100644
--- a/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c
+++ b/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c
@@ -331,6 +331,7 @@ inline uint32_t find_peaks(__m256i *transform) {
tmp = _mm256_or_si256(tmp, _mm256_and_si256(vect_mask, transform[i]));
}
result = 0;
+#pragma GCC unroll 16
for (size_t i = 0; i < 16; i++) {
mask = ~(uint32_t) ((-(int64_t)(i ^ message % 16)) >> 63);
result |= mask & ((uint16_t *)&tmp)[i];
diff --git a/src/kem/hqc/pqclean_hqc-rmrs-256_avx2/reed_muller.c b/src/kem/hqc/pqclean_hqc-rmrs-256_avx2/reed_muller.c
index 22527b8a..48c2db4c 100644
--- a/src/kem/hqc/pqclean_hqc-rmrs-256_avx2/reed_muller.c
+++ b/src/kem/hqc/pqclean_hqc-rmrs-256_avx2/reed_muller.c
@@ -331,6 +331,7 @@ inline uint32_t find_peaks(__m256i *transform) {
tmp = _mm256_or_si256(tmp, _mm256_and_si256(vect_mask, transform[i]));
}
result = 0;
+#pragma GCC unroll 16
for (size_t i = 0; i < 16; i++) {
mask = ~(uint32_t) ((-(int64_t)(i ^ message % 16)) >> 63);
result |= mask & ((uint16_t *)&tmp)[i]; This GCC error is obscure but while experimenting with an idea of replacing this array indexed access with With this patch (and LTO disabled) it builds |
So HQC remains "in the running" for NIST. Do you prefer to
|
I would rather submit PR to liboqs since I'm not user of raw PQClean. |
Make index variable `i` immediate by unrolling the loop. This is just lucky guess that's worked, because similar in function intrinsic `_mm256_extract_epi16` requires immediate value for its index. Workaround to the (perhaps) GCC 12 bug: In function 'find_peaks', inlined from 'PQCLEAN_HQCRMRS192_AVX2_reed_muller_decode' at /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c:387:18: /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c:336:44: error: 'tmp' is used uninitialized [-Werror=uninitialized] 336 | result |= mask & ((uint16_t *)&tmp)[i]; | ~~~~~~~~~~~~~~~~~~^~~ /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c: In function 'PQCLEAN_HQCRMRS192_AVX2_reed_muller_decode': /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c:234:13: note: 'tmp' was declared here 234 | __m256i tmp = _mm256_setzero_si256(); | ^~~ If LTO is enabled this error message is not shown, but HQC-128 decaps test is failed. Link: open-quantum-safe#1244 Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
I submitted the PR. ps. BTW, our tests run on ARM32 ( [1]
|
Make index variable `i` immediate by unrolling the loop. This is just lucky guess that's worked, because similar in function intrinsic `_mm256_extract_epi16` requires immediate value for its index. Workaround to the (perhaps) GCC 12 bug: In function 'find_peaks', inlined from 'PQCLEAN_HQCRMRS192_AVX2_reed_muller_decode' at /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c:387:18: /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c:336:44: error: 'tmp' is used uninitialized [-Werror=uninitialized] 336 | result |= mask & ((uint16_t *)&tmp)[i]; | ~~~~~~~~~~~~~~~~~~^~~ /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c: In function 'PQCLEAN_HQCRMRS192_AVX2_reed_muller_decode': /usr/src/RPM/BUILD/liboqs-0.7.1/src/kem/hqc/pqclean_hqc-rmrs-192_avx2/reed_muller.c:234:13: note: 'tmp' was declared here 234 | __m256i tmp = _mm256_setzero_si256(); | ^~~ If LTO is enabled this error message is not shown, but HQC-128 decaps test is failed. Link: open-quantum-safe#1244 Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
Make index variable `i` immediate by unrolling the loop. Link: #1244 Signed-off-by: Vitaly Chikunov <vt@altlinux.org>
I submitted a bug to GCC: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470 |
With the time there is multiple test errors appeared (for 0.7.1 on x86_64):
Full build log and test details http://git.altlinux.org/beehive/logs/Sisyphus/x86_64/archive/2022/0702/error/liboqs-0.7.1-alt2
For example, excerpt for one test:
I can say this output is quite obscure and hard to understand what exactly is failed.
The text was updated successfully, but these errors were encountered: