pqclean_hqc: Fix build on GCC-12

[vt-alt]

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.

As discussed at #1244

Fixes #1244

• Does this PR change the input/output behaviour of a cryptographic algorithm (i.e., does it change known answer test values)? (If so, a version bump will be required from x.y.z to x.(y+1).0.)
• Does this PR change the the list of algorithms available -- either adding, removing, or renaming? (If so, PRs in OQS-OpenSSL, OQS-BoringSSL, and OQS-OpenSSH will also be required by the time this is merged.)

No such changes.

Tested locally on 0.7.1.

[baentsch]

Two issues with this PR:

1. CI is failing because it is using a very old CI config -> Please update your repo to main.
2. These patches would be overwritten by a next HQC import. To make them survive such next import, you could create 3 suitable patch files in scripts/copy_from_upstream/patches/ (and then run scripts/copy_from_upstream/copy_from_upstream.py)?

Of course we could consider this PR a "hotfix" that shall not survive a next HQC upstream code import -- in which case item 2 is not relevant. Asking @dstebila for an opinion: Can we assume HQC will get a fix for this problem upstream (i.e., is this a hotfix) or is maintenance for HQC upstream unlikely (which would call for this to become a permanent patch allowing liboqs to keep supporting HQC regardless of upstream support)?

[dstebila]

My preference is to minimize the amount of local patching we do, so instead to push these changes up to PQClean and then re-import them. But it would be fine to merge here as long without adding them to the patchset as long as we also submit a PR at PQClean with the same patches.

[baentsch]

as we also submit a PR at PQClean with the same patches

@vt-alt : Would you be willing to do a PR at https://github.com/PQClean ?

[vt-alt]

as we also submit a PR at PQClean with the same patches

@vt-alt : Would you be willing to do a PR at https://github.com/PQClean ?

Not really, because, 1) I believe upstream should do a better fix than this workaround, and 2) to PR to PQClean I would need to compile and use somewhat and test and play with PQClean which I don't want to.

[baentsch]

and test and play with PQClean which I don't want to

Completely understandable. I'd then tend to approve this PR (edit/add: when CI passes) as-is and let this code "die automatically" as and when upstream improves things. If not, well deal with that then.

[vt-alt]

I rebased PR over the main. All checks have passed.

[vt-alt]

JFYI. I just noticed that you are (perhaps unintentionally) modify contributed commits. See, while I was PR'ing the commit with

Author:     Vitaly Chikunov <[email protected]>
AuthorDate: Tue Jul 12 18:00:39 2022 +0300

It's merged as:

Author:     Vitalio <[email protected]>
AuthorDate: Mon Jul 25 23:22:02 2022 +0300
Commit:     GitHub <[email protected]>
CommitDate: Mon Jul 25 16:22:02 2022 -0400

So it's not merged as is but mangled by GitHub. Perhaps wrong button (like squash or edit) is pressed. I don't know exactly the cause there.

Also, if user have enabled Keep my email addresses private in Settings -> Emails (when merging with such GitHub mangling) GitHub will also remove author's email (even if it's shown in commit) and replace it with their stub email, like this:

Author:     Douglas Stebila <[email protected]>
AuthorDate: Wed Aug 3 10:19:28 2022 -0400

Some contributors may not expect such change of credits.

[vt-alt]

Based on discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106470 I'm slightly worried that this workaround, while is working as intended, is not completely correct fix but still [ab]using undefined behavior, and maybe we should follow GCC author's suggestion and do, while still temporary, a correct fix?

I also opened in PQClean issue 453 about this bug.

[dstebila]

JFYI. I just noticed that you are (perhaps unintentionally) modify contributed commits.

We use "Squash and merge" for our commits to have a clean commit history. Would that be what's causing the apparent modification?

[vt-alt]

JFYI. I just noticed that you are (perhaps unintentionally) modify contributed commits.

We use "Squash and merge" for our commits to have a clean commit history. Would that be what's causing the apparent modification?

Yes seems possible. Perhaps rebase and merge is better.

[dstebila]

Yes seems possible. Perhaps rebase and merge is better.

Rebasing before merging ensures that conflicts are handled in the branch before the merge takes place, but would still mean all the commits on the branch are copied one-by-one into main. We prefer to have just a single commit on main for each pull request, regardless of how many commits were done during the development of the PR on its own branch -- that's what squashing gives us. I realize that squashing the commit history loses who authored which individual commits. Github tries to mitigate this by including some additional information on the comments, but I don't know of a better way of dealing with this at the moment.

[vt-alt]

Yes, if you want squashing perhaps there is no fix.