search

sharplispers / ironclad

A cryptographic toolkit written in Common Lisp
BSD 3-Clause "New" or "Revised" License
166 stars 28 forks source link

Some tests are failed on ECL #63

Closed catap closed 3 months ago

catap commented 11 months ago

I'd like to report that some tests are failing on ECL on macOS:

:info:test 61 out of 570 total tests failed: :KECCAK, :KECCAK/224, :KECCAK/256, :KECCAK/384, :SHA3, :SHA3/224, 
:info:test    :SHA3/256, :SHA3/384, :SHAKE128, :SHAKE256, 
:info:test    IRONCLAD-TESTS::KECCAK/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::KECCAK/224/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::KECCAK/256/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::KECCAK/384/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::SHA3/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::SHA3/224/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::SHA3/256/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::SHA3/384/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::SHAKE128/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::SHAKE256/INCREMENTAL, 
:info:test    IRONCLAD-TESTS::KECCAK/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::KECCAK/224/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::KECCAK/256/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::KECCAK/384/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::SHA3/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::SHA3/224/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::SHA3/256/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::SHA3/384/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::SHAKE128/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::SHAKE256/BLOCK-BUFFERING, 
:info:test    IRONCLAD-TESTS::KECCAK/STREAM, IRONCLAD-TESTS::KECCAK/224/STREAM, 
:info:test    IRONCLAD-TESTS::KECCAK/256/STREAM, IRONCLAD-TESTS::KECCAK/384/STREAM, 
:info:test    IRONCLAD-TESTS::SHA3/STREAM, IRONCLAD-TESTS::SHA3/224/STREAM, 
:info:test    IRONCLAD-TESTS::SHA3/256/STREAM, IRONCLAD-TESTS::SHA3/384/STREAM, 
:info:test    IRONCLAD-TESTS::SHAKE128/STREAM, IRONCLAD-TESTS::SHAKE256/STREAM, 
:info:test    IRONCLAD-TESTS::KECCAK/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::KECCAK/224/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::KECCAK/256/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::KECCAK/384/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::SHA3/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::SHA3/224/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::SHA3/256/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::SHA3/384/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::SHAKE128/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::SHAKE256/REINITIALIZE-INSTANCE, 
:info:test    IRONCLAD-TESTS::KECCAK/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::KECCAK/224/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::KECCAK/256/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::KECCAK/384/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::SHA3/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::SHA3/224/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::SHA3/256/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::SHA3/384/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::SHAKE128/FILL-POINTER, 
:info:test    IRONCLAD-TESTS::SHAKE256/FILL-POINTER, :ED448-SIGNATURE.An error occurred during initialization:

All of them failed like:

:info:test Test IRONCLAD-TESTS::SHA3/INCREMENTAL failed
:info:test Form: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE ':SHA3
:info:test                                             IRONCLAD-TESTS::*DIGEST-INCREMENTAL-TESTS*)
:info:test Expected value: T
:info:test Actual value: #<a SIMPLE-ERROR 0x108263980>.
:info:test Test IRONCLAD-TESTS::SHA3/224/INCREMENTAL failed
:info:test Form: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE ':SHA3/224
:info:test                                             IRONCLAD-TESTS::*DIGEST-INCREMENTAL-TESTS*)
:info:test Expected value: T
:info:test Actual value: #<a SIMPLE-ERROR 0x108272bc0>.
:info:test Test IRONCLAD-TESTS::SHA3/256/INCREMENTAL failed
:info:test Form: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE ':SHA3/256
:info:test                                             IRONCLAD-TESTS::*DIGEST-INCREMENTAL-TESTS*)
:info:test Expected value: T
:info:test Actual value: #<a SIMPLE-ERROR 0x108272740>.
:info:test Test IRONCLAD-TESTS::SHA3/384/INCREMENTAL failed
:info:test Form: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE ':SHA3/384
:info:test                                             IRONCLAD-TESTS::*DIGEST-INCREMENTAL-TESTS*)
:info:test Expected value: T
:info:test Actual value: #<a SIMPLE-ERROR 0x1082bba00>.
:info:test  IRONCLAD-TESTS::SHA384/INCREMENTAL IRONCLAD-TESTS::SHA512/INCREMENTAL
:info:test Test IRONCLAD-TESTS::SHAKE128/INCREMENTAL failed
:info:test Form: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE ':SHAKE128
:info:test                                             IRONCLAD-TESTS::*DIGEST-INCREMENTAL-TESTS*)
:info:test Expected value: T
:info:test Actual value: #<a SIMPLE-ERROR 0x10bc81800>.
:info:test Test IRONCLAD-TESTS::SHAKE256/INCREMENTAL failed
:info:test Form: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE ':SHAKE256
:info:test                                             IRONCLAD-TESTS::*DIGEST-INCREMENTAL-TESTS*)
:info:test Expected value: T
:info:test Actual value: #<a SIMPLE-ERROR 0x10bc81340>.
:info:test  IRONCLAD-TESTS::SKEIN1024/INCREMENTAL
glv2 commented 11 months ago

I can't reproduce on GNU/Linux, and I don't have anything running on macOS, so could you test the following code? It might give a better description of the error.

(asdf:load-system "ironclad")
(defun test-hash ()
  (let ((data (crypto:random-data 100)))
    (crypto:digest-sequence :sha3 data)))
(test-hash)
catap commented 11 months ago 
>> (defun test-hash ()
  (let ((data (crypto:random-data 100)))
    (crypto:digest-sequence :sha3 data)))

TEST-HASH
>> (test-hash)

#(138 53 202 1 225 64 18 132 221 254 85 113 27 167 109 59 ...)
>>

so, this code works well

glv2 commented 11 months ago

Then, could you try:

(asdf:load-system "ironclad/tests")
(ironclad-tests::run-test-vector-file :sha3 ironclad-tests::*digest-tests*)
catap commented 11 months ago 
T
> (ironclad-tests::run-test-vector-file :sha3 ironclad-tests::*digest-tests*)

Condition of type: SIMPLE-ERROR
one-shot SHA3 digest of #() failed

Available restarts:

1. (RESTART-TOPLEVEL) Go back to Top-Level REPL.

Broken at SI:BYTECODES. [Evaluation of: (IRONCLAD-TESTS::RUN-TEST-VECTOR-FILE :SHA3 IRONCLAD-TESTS::*DIGEST-TESTS*)] In: #<process TOP-LEVEL 0x106b2cf80>.
>>
glv2 commented 11 months ago

Do you get the same error with:

(let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))
catap commented 11 months ago

@glv2 it works:

>> (require :asdf)

;;; Loading #P"/opt/local/lib/ecl-21.2.1/asdf.fas"
("ASDF" "asdf" "UIOP" "uiop" "CMP")
>> (asdf:load-system "ironclad/tests")

T
>> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(226 14 60 199 75 1 43 1 165 135 61 139 149 42 196 112 ...)
>>
glv2 commented 11 months ago

It looks like it's not returning the right result. It should be:

#(166 159 115 204 162 58 154 197 200 181 103 220 24 90 117 110 151 201 130 22
  79 226 88 89 224 209 220 193 71 92 128 166 21 178 18 58 241 245 249 76 17 227
  233 64 44 58 197 88 245 0 25 157 149 182 211 227 1 117 133 134 40 29 205 38)

Maybe ECL has a bug on MacOS... Do you have other Common Lisp implementations (like SBCL or CCL) to check what result they return?

catap commented 11 months ago

@glv2 yes, SBCl, CLisp, CCL and ABCL works fine. The issue only with ECL.

glv2 commented 11 months ago

Have you tried using the latest ECL code from https://gitlab.com/embeddable-common-lisp/ecl/ (in case some bug was already fixed upstream)?

catap commented 11 months ago

@glv2 no, I'm using 21.2.1. But I may try and come back.

catap commented 11 months ago

On https://gitlab.com/embeddable-common-lisp/ecl/-/commit/0e05a7f360c6ae112ff4109aaaefb2d3bcd017ab

> (require :asdf)

;;; Loading #P"/opt/local/lib/ecl-21.2.1/asdf.fas"
("ASDF" "asdf" "UIOP" "uiop" "CMP")
> (asdf:load-system "ironclad/tests")

T
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(149 173 174 175 137 8 80 203 104 16 90 11 84 246 165 36 69 136 184 132 203
  152 144 111 187 188 12 90 168 187 27 31 200 247 70 251 199 73 192 52 62 179
  99 9 29 115 42 22 189 163 25 222 236 199 51 26 77 172 174 2 39 108 142 251)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(113 163 251 151 129 18 8 134 132 147 225 237 61 141 247 120 12 46 21 182 209
  114 42 15 42 177 141 149 156 158 91 46 49 209 2 234 221 231 117 89 214 161
  231 126 40 134 41 63 64 81 183 146 243 176 194 221 247 187 20 193 182 56 241
  198)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(120 13 77 231 133 4 68 128 110 17 31 20 58 115 185 235 157 193 225 36 165 54
  181 67 20 188 10 170 134 123 62 112 185 23 87 194 135 65 246 89 219 254 242
  56 83 175 106 181 148 215 79 142 105 194 255 100 119 23 72 182 237 115 240
  227)
>
catap commented 11 months ago

Let me run the whole tests

catap commented 11 months ago

Test fails on the same way on https://gitlab.com/embeddable-common-lisp/ecl/-/commit/0e05a7f360c6ae112ff4109aaaefb2d3bcd017ab

glv2 commented 11 months ago 
> (require :asdf)

;;; Loading #P"/opt/local/lib/ecl-21.2.1/asdf.fas"
("ASDF" "asdf" "UIOP" "uiop" "CMP")
> (asdf:load-system "ironclad/tests")

T
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(149 173 174 175 137 8 80 203 104 16 90 11 84 246 165 36 69 136 184 132 203
  152 144 111 187 188 12 90 168 187 27 31 200 247 70 251 199 73 192 52 62 179
  99 9 29 115 42 22 189 163 25 222 236 199 51 26 77 172 174 2 39 108 142 251)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(113 163 251 151 129 18 8 134 132 147 225 237 61 141 247 120 12 46 21 182 209
  114 42 15 42 177 141 149 156 158 91 46 49 209 2 234 221 231 117 89 214 161
  231 126 40 134 41 63 64 81 183 146 243 176 194 221 247 187 20 193 182 56 241
  198)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(120 13 77 231 133 4 68 128 110 17 31 20 58 115 185 235 157 193 225 36 165 54
  181 67 20 188 10 170 134 123 62 112 185 23 87 194 135 65 246 89 219 254 242
  56 83 175 106 181 148 215 79 142 105 194 255 100 119 23 72 182 237 115 240
  227)
>

So it's returning a different result every time... this is very strange. As if it's reading memory in a random location...

One thing you could try is changing the optimize declarations in "src/digests/sha3.lisp" and increasing the safety and debug parameters. For example something like (optimize (speed 3) (space 0) (safety 3) (debug 3)). Maybe it will detect an out of bound memory access, or something like that. Removing all the optimize declarations could be another thing to try....

catap commented 11 months ago

@glv2 it doesn't help :(

catap commented 11 months ago

@easye maybe you have some idea?

hraban commented 9 months ago 
> (require :asdf)

;;; Loading #P"/opt/local/lib/ecl-21.2.1/asdf.fas"
("ASDF" "asdf" "UIOP" "uiop" "CMP")
> (asdf:load-system "ironclad/tests")

T
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(149 173 174 175 137 8 80 203 104 16 90 11 84 246 165 36 69 136 184 132 203
  152 144 111 187 188 12 90 168 187 27 31 200 247 70 251 199 73 192 52 62 179
  99 9 29 115 42 22 189 163 25 222 236 199 51 26 77 172 174 2 39 108 142 251)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(113 163 251 151 129 18 8 134 132 147 225 237 61 141 247 120 12 46 21 182 209
  114 42 15 42 177 141 149 156 158 91 46 49 209 2 234 221 231 117 89 214 161
  231 126 40 134 41 63 64 81 183 146 243 176 194 221 247 187 20 193 182 56 241
  198)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(120 13 77 231 133 4 68 128 110 17 31 20 58 115 185 235 157 193 225 36 165 54
  181 67 20 188 10 170 134 123 62 112 185 23 87 194 135 65 246 89 219 254 242
  56 83 175 106 181 148 215 79 142 105 194 255 100 119 23 72 182 237 115 240
  227)
>

So it's returning a different result every time... this is very strange. As if it's reading memory in a random location...

One thing you could try is changing the optimize declarations in "src/digests/sha3.lisp" and increasing the safety and debug parameters. For example something like (optimize (speed 3) (space 0) (safety 3) (debug 3)). Maybe it will detect an out of bound memory access, or something like that. Removing all the optimize declarations could be another thing to try....

I did what you suggested and it solved the problem!


#(213 126 127 255 62 174 252 126 217 100 63 58 42 162 172 124 31 5 209 147 224
  240 27 113 1 224 182 30 94 188 78 203 100 96 115 47 53 163 212 231 168 87 247
  219 65 216 191 220 216 130 171 95 218 170 118 105 249 248 10 3 235 232 20 211)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(213 126 127 255 62 174 252 126 217 100 63 58 42 162 172 124 31 5 209 147 224
  240 27 113 1 224 182 30 94 188 78 203 100 96 115 47 53 163 212 231 168 87 247
  219 65 216 191 220 216 130 171 95 218 170 118 105 249 248 10 3 235 232 20 211)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(213 126 127 255 62 174 252 126 217 100 63 58 42 162 172 124 31 5 209 147 224
  240 27 113 1 224 182 30 94 188 78 203 100 96 115 47 53 163 212 231 168 87 247
  219 65 216 191 220 216 130 171 95 218 170 118 105 249 248 10 3 235 232 20 211)

Here's the diff:

diff --git c/src/digests/sha3.lisp i/src/digests/sha3.lisp
index 4f72865..ffd5dcb 100644
--- c/src/digests/sha3.lisp
+++ i/src/digests/sha3.lisp
@@ -69,7 +69,7 @@
          (ftype (function ((integer 0 23)) keccak-lane) get-keccak-round-constant))
 (defun get-keccak-round-constant (i)
   (declare (type (integer 0 23) i)
-           (optimize (speed 3) (space 0) (safety 0) (debug 0)))
+           (optimize (speed 0) (space 0) (safety 3) (debug 3)))
   (let ((constants (load-time-value +keccak-round-constants+ t)))
     (declare (type (simple-array keccak-lane (24)) constants))
     (aref constants i)))
@@ -77,7 +77,7 @@
 (declaim (inline make-keccak-state)
          (ftype (function () keccak-state) make-keccak-state))
 (defun make-keccak-state ()
-  (declare (optimize (speed 3) (space 0) (safety 0) (debug 0)))
+  (declare (optimize (speed 0) (space 0) (safety 3) (debug 3)))
   (make-array #.+keccak-state-lanes+ :element-type 'keccak-lane :initial-element 0))

@@ -90,7 +90,7 @@
            (type (integer 0 1600) bit-rate)
            (type (simple-array (unsigned-byte 8) (*)) input)
            (type fixnum start)
-           (optimize (speed 3) (space 0) (safety 0) (debug 0)))
+           (optimize (speed 0) (space 0) (safety 3) (debug 3)))
   (let ((rate-bytes (truncate bit-rate 8)))
     (declare (type (integer 0 200) rate-bytes))
     (dotimes (y +keccak-state-rows+)
@@ -207,7 +207,7 @@ the content on normal form exit."
 (declaim (ftype (function (keccak-state)) keccak-rounds))
 (defun keccak-rounds (state)
   (declare (type keccak-state state)
-           (optimize (speed 3) (space 0) (safety 0) (debug 0)))
+           (optimize (speed 0) (space 0) (safety 3) (debug 3)))
   (with-keccak-state-accessors (state)
     (with-temp-keccak-state (b)
       (with-temp-keccak-rows (c d)
@@ -388,7 +388,7 @@ the content on normal form exit."
   (declare (type sha3 state)
            (type (simple-array (unsigned-byte 8) (*)) vector)
            (type fixnum start end)
-           (optimize (speed 3) (safety 0) (space 0) (debug 0)))
+           (optimize (speed 0) (safety 3) (space 0) (debug 3)))
   (let* ((keccak-state (sha3-state state))
          (buffer (sha3-buffer state))
          (buffer-index (sha3-buffer-index state))
@@ -433,7 +433,7 @@ the content on normal form exit."
   (declare (type sha3 state)
            (type (simple-array (unsigned-byte 8) (*)) digest)
            (type integer digest-start)
-           (optimize (speed 3) (safety 1) (space 0) (debug 0)))
+           (optimize (speed 0) (safety 3) (space 0) (debug 3)))
   (let ((padding-type (typecase state
                         (shake128 :xof)
                         (shake256 :xof)

Without this change, I can repro @catap's bug.

glv2 commented 9 months ago

@hraban

I did what you suggested and it solved the problem!


#(213 126 127 255 62 174 252 126 217 100 63 58 42 162 172 124 31 5 209 147 224
  240 27 113 1 224 182 30 94 188 78 203 100 96 115 47 53 163 212 231 168 87 247
  219 65 216 191 220 216 130 171 95 218 170 118 105 249 248 10 3 235 232 20 211)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(213 126 127 255 62 174 252 126 217 100 63 58 42 162 172 124 31 5 209 147 224
  240 27 113 1 224 182 30 94 188 78 203 100 96 115 47 53 163 212 231 168 87 247
  219 65 216 191 220 216 130 171 95 218 170 118 105 249 248 10 3 235 232 20 211)
> (let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(213 126 127 255 62 174 252 126 217 100 63 58 42 162 172 124 31 5 209 147 224
  240 27 113 1 224 182 30 94 188 78 203 100 96 115 47 53 163 212 231 168 87 247
  219 65 216 191 220 216 130 171 95 218 170 118 105 249 248 10 3 235 232 20 211)

Here's the diff:


diff --git c/src/digests/sha3.lisp i/src/digests/sha3.lisp
index 4f72865..ffd5dcb 100644
--- c/src/digests/sha3.lisp
+++ i/src/digests/sha3.lisp
@@ -69,7 +69,7 @@
          (ftype (function ((integer 0 23)) keccak-lane) get-keccak-round-constant))
 (defun get-keccak-round-constant (i)
   (declare (type (integer 0 23) i)
-           (optimize (speed 3) (space 0) (safety 0) (debug 0)))
+           (optimize (speed 0) (space 0) (safety 3) (debug 3)))
  [...]

It now always returns the same thing, but it it still a wrong result. It should be:

(let ((data (make-array 0 :element-type '(unsigned-byte 8))))
  (crypto:digest-sequence :sha3 data))

#(166 159 115 204 162 58 154 197 200 181 103 220 24 90 117 110 151 201 130 22
  79 226 88 89 224 209 220 193 71 92 128 166 21 178 18 58 241 245 249 76 17 227
  233 64 44 58 197 88 245 0 25 157 149 182 211 227 1 117 133 134 40 29 205 38)

Are you also on MacOS? What compiler is used to compile the C code generated by ECL?

hraban commented 9 months ago

I am on aarch64-darwin (M2 mac), and apparently I'm using clang 11.1.0.

hraban commented 9 months ago

Using GCC fixes it, regardless of optimization level.

catap commented 7 months ago

Using GCC fixes it, regardless of optimization level.

See: https://github.com/ivmai/bdwgc/issues/569#issuecomment-1699827841 and https://gitlab.com/embeddable-common-lisp/ecl/-/issues/705

Here I'm tracking a wired issue in ECL which prevents it to be build on macOS i386 with clang; and gcc works fine.

catap commented 7 months ago

Shall it be reported to ECL upstream? It seems like a ECL bug.

hraban commented 7 months ago

Yeah maybe it's emitting UB somehow?

catap commented 7 months ago

I don't know. Can I ask your help to fill a bug into ECL? Seems that you have more context than me here.

hraban commented 7 months ago

We can open the bug with what little information we have and see what happens... If you send me a link to a ticket I'll try to create a reproducible example to add to it

catap commented 7 months ago

The main ticket: https://gitlab.com/embeddable-common-lisp/ecl/-/issues/705

feel free to ask help to test it.

hraban commented 7 months ago

(Un)fortunately I don't have problems building ECL 21.2.1 itself so I can't help with that part--I only have problems using it to compile ironclad. Are you sure my input is relevant on that ticket? Best I can do is provide a link back to here.

dkochmanski commented 6 months ago

I'm able to reproduce this issue on the machine provided by @catap . I've somewhat minimized the example to:

(defun keccak-rounds* ()
  (let* ((a 0)
         (b (rol64 a (get-keccak-rotate-offset 0 0))))
    b))
(compile 'keccak-rounds*)

(keccak-rounds*)

it seems, that clang miscompiles something - b is initialized to some random value. when I substitute a with 0 in the call to rol64 results are correct.

dkochmanski commented 6 months ago

/Even better:

(declaim (inline rol64*))

(defun rol64* (a s)
  (declare (type (unsigned-byte 64) a)
           (type (unsigned-byte 6) s))
  (ffi:c-inline (a s)
                (:uint64-t :uint8-t)
                :uint64-t
                "(#0 << #1) | (#0 >> (64 - #1))"
                :one-liner t
                :side-effects nil)
  #+ (or)
  (logior (ldb (byte 64 0) (ash a s)) (ash a (- s 64))))

(defun keccak-rounds* ()
  (rol64* 0 0))

(compile 'rol64*)
(compile 'keccak-rounds*)

(keccak-rounds*)
;; -> should be 0
;; -> osx gives 259 (clang)
dkochmanski commented 6 months ago

Minimized test case:

(declaim (inline rol64*))

(defun rol64* (a s)
  (declare (type (unsigned-byte 64) a)
           (type (unsigned-byte 6) s))
  (ffi:c-inline (a s)
                (:uint64-t :uint8-t)
                :uint64-t
                "(#0 << #1) | (#0 >> (64 - #1))"
                :one-liner t
                :side-effects nil))

#+(or)
(defun keccak-rounds* ()
  (rol64* 0 0))

;; -> should be 0
;; -> osx gives 259

After compilation this code returns invalid value on OSX.

Darwin [Kirills-mini.sa31-gst.catap.net]redacted] 22.4.0 Darwin Kernel Version 22.4.0: Mon Mar  6 21:00:17 PST 2023; root:xnu-8796.101.5~3/RELEASE_X86_64 x86_64

After decompilation the relevant part of code is equivalent (roughly) to:

#include <stdio.h>
#include <stdint.h>
#include <inttypes.h>

int main () {
  unsigned long v1a;
  int8_t v2s;
  uint64_t result;
  v1a = (unsigned long)(0);
  v2s = 0;
  result = (((uint64_t)(v1a) << (uint8_t)(v2s)) |
            ((uint64_t)(v1a) >> (64 - (uint8_t)(v2s))));
  printf("%" PRIu64 "\n", result);
  return 0;
}

On x86-64 linux I get 0, while on osx this gives random garbage (both gcc and clang, unless -O0 is specified).. It does not matter whether we change v1a type to uint64_t, because on OSX these types are equivalent.

Note that this is a shift count overflow, becuase 64-0 = 64. The issue is not spotted by the compiler and warning is not issued, because we use a variable (and clang/gcc are not "clever enough" :).

All in all this c-inline hits an undefined behavior and apparently it is enough to put compilers on OSX out of the groove. rol64 (and preasumbly rot64 an other places) should not attempt to shift by the width of the variable - special-casing 0 is one possible solution.

catap commented 6 months ago

@dkochmanski as far as I recall v1a and v2s shouldn't be initialized by zero by compiler. This requirement only for static variables, and when you're on the stack it might or might not be zeroed.

dkochmanski commented 6 months ago

both v1a and v2s are initialized below declarations.

catap commented 6 months ago

both v1a and v2s are initialized below declarations.

I definitely need more coffee!

hraban commented 5 months ago

Thanks for your debugging @dkochmanski . Do you know if upstream ECL is aware of this issue and whether they are tracking it somewhere?

dkochmanski commented 5 months ago

@hraban as ECL dev I'm aware of it :) that said- this is a bug in ironclad's backend for ECL, so there's nothing to track on ECL side.

glv2 commented 5 months ago

I added a possible fix (6a80a6ae9aeb0c4cac1489909960e1468457c3c3). Could someone with an OSX machine test if it works?

hraban commented 5 months ago

Works on 64-bit x86 linux and mac! https://github.com/hraban/cl-nix-lite/actions/runs/8051305500