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.

Sign up for GitHub

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

Jump to bottom

Incorrect parallel hash output when building with SSE2 instructions #19

Closed
damaki opened this issue Jun 4, 2022 · 0 comments · Fixed by #21
Closed

Incorrect parallel hash output when building with SSE2 instructions #19

damaki opened this issue Jun 4, 2022 · 0 comments · Fixed by #21
Assignees
@damaki
Labels
bug

Comments

@damaki
Copy link
Owner

damaki commented Jun 4, 2022

When building with LIBKECCAK_ARCH=x86_64 and LIBKECCAK_SIMD=SSE2 the parallel hashes (ParallelHash, KangarooTwelve, etc) produce incorrect output. The known answer tests fail with the following output:

Loading file: testvectors/ParallelHash/ParallelHash128_samples.txt
Running 3 tests ...
FAILURE:
   Expected MD: BA8DC1D1D979331D3F813603C67F72609AB5E44B94A0B8F9AF46514454A2B4F5
   Actual MD:   3E5AC8757659890646FC627DC8EC9DA8019E150CA203566AA42C1955564787BE
FAILURE:
   Expected MD: FC484DCB3F84DCEEDC353438151BEE58157D6EFED0445A81F165E495795B7206
   Actual MD:   B5236BC97DCC088B6A2A9D87BB0AD11F2A8ACE1143F73FBC8582987EFC5FC0B8
FAILURE:
   Expected MD: F7FD5312896C6685C828AF7E2ADB97E393E7F8D54E3C2EA4B95E5ACA3796E8FC
   Actual MD:   245C4A5FA512F7AAE3D33B6A3A8E2EB26A8026EC1909B7B3FE79EC3F09CD6635

Passed: 0
Failed: 3

The problem is only present when using SSE2; it is not present when building with LIBKECCAK_SIMD=AVX2 or LIBKECCAK_SIMD=none.

The problem is due to an incorrect definition of KeccakF_1600_P2 in src/x86_64/SSE2/keccak-parallel_keccak_1600.ads where it is defined as follows:

   package KeccakF_1600_P2 is new Keccak.Generic_Parallel_KeccakF
     (Lane_Size_Log => 6,
      Lane_Type     => Interfaces.Unsigned_64,
      VXXI_Index    => Arch.SSE2.V2DI_Vectors.V2DI_Index,
      VXXI          => Arch.SSE2.V2DI_Vectors.V2DI,
      VXXI_View     => Arch.SSE2.V2DI_Vectors.V2DI_View,
      Vector_Width  => 2,
      Load          => Arch.SSE2.V2DI_Vectors.Load,
      Store         => Arch.SSE2.V2DI_Vectors.Store,
      "xor"         => Arch.SSE2.V2DI_Vectors."xor",
      Rotate_Left   => Arch.SSE2.V2DI_Vectors.Shift_Left,
      And_Not       => Arch.SSE2.V2DI_Vectors.And_Not,
      Shift_Left    => Interfaces.Shift_Left,
      Shift_Right   => Interfaces.Shift_Right);

The Rotate_Left generic parameter is incorrectly set to Arch.SSE2.V2DI_Vectors.Shift_Left instead of Arch.SSE2.V2DI_Vectors.Rotate_Left.
@damaki damaki added the bug label Jun 4, 2022
@damaki damaki self-assigned this Jun 4, 2022
@damaki damaki mentioned this issue Jun 5, 2022
Merged
@damaki damaki closed this as completed in #21 Jun 5, 2022
damaki added a commit that referenced this issue Jun 5, 2022
@damaki
Setup GitHub CI (#21)
eb3c9f3 
This sets up CI using GitHub Actions to perform the following checks for each supported arch/SIMD combination:
* Build libkeccak.
* Build and run the benchmark program.
* Build and run the known answer tests (with contracts and runtime checks enabled)
* Build and run the unit tests (with contracts and runtime checks enabled)

Proofs are configured to only run manually and on release creation since they take a very long time to run
in CI (about 1h 20m). The intent is to allow the choice between running them manually in CI or pulling a
branch and running them locally (on a much more powerful machine) when reviewing pull requests.

This also fixes two issues found by CI to ensure a clean CI build:
* Fixed incorrect definition of `Keccak.Parallel_Keccak_1600.KeccakF_1600_P2` (fixes #19).
* Fixed some style and compile check issues in the test suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@damaki damaki

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Setup GitHub CI damaki/libkeccak
1 participant
@damaki