monero-project / monero

Monero: the secure, private, untraceable cryptocurrency
https://getmonero.org
Other
9k stars 3.12k forks source link

Fail to build on PPC64 #3826

Closed hegjon closed 2 years ago

hegjon commented 6 years ago

Seems like this is the cause:

cc1: error: unrecognized command line option '-maes'
cc1: error: unrecognized command line option '-march=native'

Full log: https://kojipkgs.fedoraproject.org//work/tasks/3387/27023387/build.log

nioroso-x3 commented 6 years ago

Are you on big endian or little endian? Some months ago I managed to modify the CMakeLists.txt enough to get it to compile, but in big endian ppc64 there are just too many endiannes problems. The daemon is unable to connect to anything and the cli wallet creates invalid keys.

hegjon commented 6 years ago

Big Endian.

CPU info:

Architecture:        ppc64
CPU op-mode(s):      32-bit, 64-bit
Byte Order:          Big Endian
CPU(s):              4
On-line CPU(s) list: 0-3
Thread(s) per core:  1
Core(s) per socket:  1
Socket(s):           4
NUMA node(s):        1
Model:               2.1 (pvr 004b 0201)
Model name:          POWER8 (architected), altivec supported
Hypervisor vendor:   KVM
Virtualization type: para
L1d cache:           64K
L1i cache:           32K
NUMA node0 CPU(s):   0-3
hegjon commented 6 years ago

Some months ago I managed to modify the CMakeLists.txt enough to get it to compile, but in big endian ppc64 there are just too many endiannes problems.

Would it be possible to white list those platforms that support -maes instead of having a list of the known platforms that does not support the flag?

hyc commented 6 years ago

Yeah I think there's far too much dependency on little-endian math scattered throughout the code. Feel free to track all occurrences down and submit patches to fix them.

nioroso-x3 commented 6 years ago

I created a pull request for the modified CMakeLists. It compiles fine in gentoo ppc64, and almost in macos leopard. Any pointers on where little endian math is most used? Most of the crypto source files seem to handle endianness fine.

hegjon commented 6 years ago

I created a pull request for the modified CMakeLists.

Thanks.

Why is it needed to set the -mcpu flags?

nioroso-x3 commented 6 years ago

Why is it needed to set the -mcpu flags?

-mcpu=native for some reason fails with G5 or ppc970 gcc, so I just set it manually to the lowest common denominator, a G4 for 32 bit, G5 for 64 bit and Power8 for 64 bit little endian.

jtgrassie commented 6 years ago

@nioroso-x3

Any pointers on where little endian math is most used? Most of the crypto source files seem to handle endianness fine.

See: https://github.com/monero-project/monero/blob/master/src/crypto/crypto-ops.c

A lot of the math in there is LE dependent.

jtgrassie commented 6 years ago

Also, the crypto implementations in the src/crypto directory are the x86 implementations, for example, chacha has a different implementation for PowerPC: https://cr.yp.to/chacha.html.

moneromooo-monero commented 6 years ago

Is it any better nowadays ? That particular thing got fixed a while ago.

nioroso-x3 commented 6 years ago

Is it any better nowadays ? That particular thing got fixed a while ago.

Monero is endian agnostic now? I haven't booted my power mac in a while, so I havent tested anything.

moneromooo-monero commented 6 years ago

Well, we won't know for sure until someone reports another problem :)

nioroso-x3 commented 6 years ago

Just compiled latest git, monerod fails all handshakes and monero-wallet-cli creates invalid keys and is unable to open valid wallets.

moneromooo-monero commented 6 years ago

Can you start with unit_tests please ?

moneromooo-monero commented 6 years ago

And crypto tests. "make release-test" should run them all.

nioroso-x3 commented 6 years ago

This is the output: Running tests...
Test project /home/jribeiro/Development/monero/build/Linux/master/release Start 1: hash-target
1/15 Test #1: hash-target ...................... Passed 0.51 sec Start 2: core_tests
2/15 Test #2: core_tests .......................Exception: Other7354.57 sec Start 3: cncrypto
3/15 Test #3: cncrypto .........................
Failed 52.76 sec Start 4: unit_tests
4/15 Test #4: unit_tests .......................Exception: Other335.84 sec Start 5: difficulty
5/15 Test #5: difficulty ....................... Passed 0.23 sec
Start 6: hash-fast
6/15 Test #6: hash-fast ........................ Passed 0.08 sec Start 7: hash-slow
7/15 Test #7: hash-slow ........................
Failed 1.36 sec Start 8: hash-slow-1
8/15 Test #8: hash-slow-1 ......................Failed 1.61 sec
Start 9: hash-slow-2
9/15 Test #9: hash-slow-2 ......................
Failed 4.72 sec Start 10: hash-tree
10/15 Test #10: hash-tree ........................ Passed 0.19 sec Start 11: hash-extra-blake
11/15 Test #11: hash-extra-blake ................. Passed 0.04 sec Start 12: hash-extra-groestl
12/15 Test #12: hash-extra-groestl ...............Failed 0.75 sec Start 13: hash-extra-jh
13/15 Test #13: hash-extra-jh .................... Passed 0.03 sec Start 14: hash-extra-skein
14/15 Test #14: hash-extra-skein ................. Passed 0.04 sec
Start 15: hash-variant2-int-sqrt
15/15 Test #15: hash-variant2-int-sqrt ...........
Exception: Other166.52 sec

47% tests passed, 8 tests failed out of 15

Total Test time (real) = 7919.46 sec

The following tests FAILED: 2 - core_tests (OTHER_FAULT) 3 - cncrypto (Failed) 4 - unit_tests (OTHER_FAULT) 7 - hash-slow (Failed) 8 - hash-slow-1 (Failed) 9 - hash-slow-2 (Failed) 12 - hash-extra-groestl (Failed) 15 - hash-variant2-int-sqrt (OTHER_FAULT) Errors while running CTest

The exceptions are because I killed the tests that were taking too long. Attached is the core_tests log.

core_tests.log.gz

moneromooo-monero commented 6 years ago

Can you please run and attach the output of these two commands:

./build/Linux/master/release/tests/crypto/cncrypto-tests tests/crypto/tests.txt ./build/Linux/master/release/tests/unit_tests/unit_tests

nioroso-x3 commented 6 years ago

Ok, done. unit_tests.log.gz cncrypt-tests.log.gz

moneromooo-monero commented 6 years ago

Are you running with commit hash aa1d321e5f207a69161ffb919453dfe7311b9e61 ?

nioroso-x3 commented 6 years ago

The short HEAD outputs 5c85da5, I cloned the repo yesterday.

moneromooo-monero commented 6 years ago

The main problem seems to be Keccak being wrong. Which is a shame since the last change to Keccak was to allegedly make it work on big endian architectures :) That is used for the PRNG, which in turn makes most of the crypto tests fail.

moneromooo-monero commented 6 years ago

As for the IP errors, this should fix it:

diff --git a/contrib/epee/include/net/local_ip.h b/contrib/epee/include/net/local_ip.h
index 52c5855b9..90e6a07b0 100644
--- a/contrib/epee/include/net/local_ip.h
+++ b/contrib/epee/include/net/local_ip.h
@@ -27,6 +27,15 @@

 #pragma once

+namespace
+{
+  static inline uint32_t leip(uint32_t x)
+  {
+    x = ((x & 0x00ff00ff) << 8) | ((x & 0xff00ff00) >> 8);
+    return (x << 16) | (x >> 16);
+  }
+}
+
 namespace epee
 {
   namespace net_utils
@@ -34,6 +43,7 @@ namespace epee
     inline
     bool is_ip_local(uint32_t ip)
     {
+      ip = leip(ip);
       /*
       local ip area
       10.0.0.0 <97> 10.255.255.255 
@@ -57,6 +67,7 @@ namespace epee
     inline
     bool is_ip_loopback(uint32_t ip)
     {
+      ip = leip(ip);
       if( (ip | 0xffffff00) == 0xffffff7f)
         return true;
       //MAKE_IP
moneromooo-monero commented 6 years ago

This might fix the keccak part (edited, needs more parentheses):

diff --git a/src/crypto/keccak.c b/src/crypto/keccak.c
index b5946036e..ee20adb2d 100644
--- a/src/crypto/keccak.c
+++ b/src/crypto/keccak.c
@@ -145,7 +145,7 @@ void keccak1600(const uint8_t *in, size_t inlen, uint8_t *md)
 #define IS_ALIGNED_64(p) (0 == (7 & ((const char*)(p) - (const char*)0)))
 #define KECCAK_PROCESS_BLOCK(st, block) { \
     for (int i_ = 0; i_ < KECCAK_WORDS; i_++){ \
-        ((st))[i_] ^= ((block))[i_]; \
+        ((st))[i_] ^= swap64le(((block))[i_]); \
     }; \
     keccakf(st, KECCAK_ROUNDS); }

moneromooo-monero commented 6 years ago

The IP stuff is wrong, the IPs should be in network byte order. If they're not, then somhting else is probably wrong.

moneromooo-monero commented 6 years ago

Or maybe not. That will need thinking.

nioroso-x3 commented 6 years ago

The patches didnt change much.

Test project /home/jribeiro/Development/monero/build/Linux/master/release
Start 1: hash-target
1/15 Test #1: hash-target ...................... Passed 0.42 sec
Start 2: core_tests
2/15 Test #2: core_tests .......................Exception: Other5903.01 sec
Start 3: cncrypto
3/15 Test #3: cncrypto .........................
Failed 67.77 sec
Start 4: unit_tests
4/15 Test #4: unit_tests .......................Failed 849.29 sec
Start 5: difficulty 5/15 Test #5: difficulty ....................... Passed 0.24 sec Start 6: hash-fast 6/15 Test #6: hash-fast ........................ Passed 0.08 sec Start 7: hash-slow 7/15 Test #7: hash-slow ........................
Failed 1.37 sec Start 8: hash-slow-1 8/15 Test #8: hash-slow-1 ......................Failed 1.79 sec Start 9: hash-slow-2 9/15 Test #9: hash-slow-2 ......................Failed 4.75 sec Start 10: hash-tree 10/15 Test #10: hash-tree ........................ Passed 0.03 sec Start 11: hash-extra-blake 11/15 Test #11: hash-extra-blake ................. Passed 0.04 sec Start 12: hash-extra-groestl 12/15 Test #12: hash-extra-groestl ...............***Failed 0.68 sec Start 13: hash-extra-jh 13/15 Test #13: hash-extra-jh .................... Passed 0.03 sec Start 14: hash-extra-skein 14/15 Test #14: hash-extra-skein ................. Passed 0.04 sec Start 15: hash-variant2-int-sqrt 15/15 Test #15: hash-variant2-int-sqrt ........... Passed 1178.90 sec

cncrypto-tests.log.gz unit_tests.log.gz

moneromooo-monero commented 6 years ago

Well, you're going to have to debug it I'm afraid, or wait for someone else with big endian hardware to have a look. Since it was recently "fixed" for big endian, it should be mostly there already.

nioroso-x3 commented 6 years ago

I have zero knowledge of how monero works, what should I look into first? Get the functions of keccak.c to have the same output as little endian machines?

moneromooo-monero commented 6 years ago

Since the (primary) culprit seems to be Keccak, you don't need to know how monero works, just how to build it. The Keccak output for a given input should indeed be identical on big and little endian archs. I suspect once Keccak's fixed, a lot of stuff will start working. We can then see what's next in line. Thanks

xiphon commented 6 years ago

@nioroso-x3 Please apply the patch from attached PR, run the tests and provide the logs.

nioroso-x3 commented 6 years ago

From the unit_test logs seems like keccak is fixed now. cncrypto-tests.log.gz unit_tests.log.gz core_tests.log.gz

Test project /home/jribeiro/Development/monero/build/Linux/master/release
Start 1: hash-target
1/15 Test #1: hash-target ...................... Passed 0.31 sec
Start 2: core_tests
2/15 Test #2: core_tests .......................Exception: Other3939.69 sec
Start 3: cncrypto
3/15 Test #3: cncrypto .........................
Failed 57.74 sec
Start 4: unit_tests
4/15 Test #4: unit_tests .......................Failed 811.43 sec
Start 5: difficulty
5/15 Test #5: difficulty ....................... Passed 0.24 sec
Start 6: hash-fast
6/15 Test #6: hash-fast ........................ Passed 0.06 sec
Start 7: hash-slow
7/15 Test #7: hash-slow ........................
Failed 1.36 sec
Start 8: hash-slow-1
8/15 Test #8: hash-slow-1 ......................Failed 1.65 sec
Start 9: hash-slow-2
9/15 Test #9: hash-slow-2 ......................
Failed 4.86 sec
Start 10: hash-tree
10/15 Test #10: hash-tree ........................ Passed 0.19 sec
Start 11: hash-extra-blake
11/15 Test #11: hash-extra-blake ................. Passed 0.04 sec
Start 12: hash-extra-groestl
12/15 Test #12: hash-extra-groestl ...............***Failed 0.71 sec
Start 13: hash-extra-jh
13/15 Test #13: hash-extra-jh .................... Passed 0.03 sec
Start 14: hash-extra-skein
14/15 Test #14: hash-extra-skein ................. Passed 0.03 sec
Start 15: hash-variant2-int-sqrt
15/15 Test #15: hash-variant2-int-sqrt ........... Passed 1314.93 sec

53% tests passed, 7 tests failed out of 15

Total Test time (real) = 6133.48 sec

The following tests FAILED: 2 - core_tests (OTHER_FAULT) 3 - cncrypto (Failed) 4 - unit_tests (Failed) 7 - hash-slow (Failed) 8 - hash-slow-1 (Failed) 9 - hash-slow-2 (Failed) 12 - hash-extra-groestl (Failed)

xiphon commented 6 years ago

Thanks, will ping you to test further big-endian patches on hardware if you don't mind

xiphon commented 6 years ago

@nioroso-x3 updated #4689 with fixes for groestl. Please test the new version,

moneromooo-monero commented 6 years ago

That one might fix the mnemonics unit test:

diff --git a/src/mnemonics/electrum-words.cpp b/src/mnemonics/electrum-words.cpp
index 3d6338856..ba2c8e120 100644
--- a/src/mnemonics/electrum-words.cpp
+++ b/src/mnemonics/electrum-words.cpp
@@ -335,7 +335,8 @@ namespace crypto
           return false;
         }

-        dst.append((const char*)&w[0], 4);  // copy 4 bytes to position
+        uint32_t w0 = SWAP32LE(*(const uint32_t*)&w[0]);
+        dst.append((const char*)&w0, 4);  // copy 4 bytes to position
         memwipe(w, sizeof(w));
       }

nioroso-x3 commented 6 years ago

Groestl and the mnemonics tests now pass.

Test project /home/jribeiro/Development/monero/build/Linux/master/release
Start 1: hash-target
1/15 Test #1: hash-target ...................... Passed 0.52 sec
Start 2: core_tests
2/15 Test #2: core_tests .......................Exception: Other641.52 sec
Start 3: cncrypto
3/15 Test #3: cncrypto .........................
Failed 53.65 sec
Start 4: unit_tests 4/15 Test #4: unit_tests .......................Failed 829.74 sec Start 5: difficulty 5/15 Test #5: difficulty ....................... Passed 0.06 sec Start 6: hash-fast 6/15 Test #6: hash-fast ........................ Passed 0.07 sec Start 7: hash-slow 7/15 Test #7: hash-slow ........................Failed 1.39 sec Start 8: hash-slow-1 8/15 Test #8: hash-slow-1 ......................Failed 1.66 sec Start 9: hash-slow-2 9/15 Test #9: hash-slow-2 ......................Failed 4.89 sec Start 10: hash-tree 10/15 Test #10: hash-tree ........................ Passed 0.19 sec Start 11: hash-extra-blake 11/15 Test #11: hash-extra-blake ................. Passed 0.04 sec Start 12: hash-extra-groestl 12/15 Test #12: hash-extra-groestl ............... Passed 0.06 sec Start 13: hash-extra-jh 13/15 Test #13: hash-extra-jh .................... Passed 0.04 sec Start 14: hash-extra-skein 14/15 Test #14: hash-extra-skein ................. Passed 0.04 sec Start 15: hash-variant2-int-sqrt 15/15 Test #15: hash-variant2-int-sqrt ........... Passed 1351.16 sec

unit_tests.log.gz cncrypto-tests.log.gz

moneromooo-monero commented 6 years ago

That might fix the remainder of the RNG:

diff --git a/src/crypto/hash.c b/src/crypto/hash.c
index 42f272e34..d8c549f19 100644
--- a/src/crypto/hash.c
+++ b/src/crypto/hash.c
@@ -36,7 +36,14 @@
 #include "keccak.h"

 void hash_permutation(union hash_state *state) {
+#if BYTE_ORDER == LITTLE_ENDIAN
   keccakf((uint64_t*)state, 24);
+#else
+  uint64_t le_state[24];
+  memcpy_swap64le(le_state, state, 24);
+  keccakf(le_state, 24);
+  memcpy_swap64le(state, le_state, 24);
+#endif
 }
 void hash_process(union hash_state *state, const uint8_t *buf, size_t count) {            
nioroso-x3 commented 6 years ago

Your patch to hash.c causes this error: stack smashing detected : terminated Aborted

moneromooo-monero commented 6 years ago

Oooh, the 24 is the number of rounds, my mistake. Here's a fixed version:

diff --git a/src/crypto/hash.c b/src/crypto/hash.c
index 42f272e34..43ce32957 100644
--- a/src/crypto/hash.c
+++ b/src/crypto/hash.c
@@ -36,7 +36,14 @@
 #include "keccak.h"

 void hash_permutation(union hash_state *state) {
+#if BYTE_ORDER == LITTLE_ENDIAN
   keccakf((uint64_t*)state, 24);
+#else
+  uint64_t le_state[25];
+  memcpy_swap64le(le_state, state, 25);
+  keccakf(le_state, 24);
+  memcpy_swap64le(state, le_state, 25);
+#endif
 }

 void hash_process(union hash_state *state, const uint8_t *buf, size_t count) {
nioroso-x3 commented 6 years ago

cncrypto tests now pass

  Start  1: hash-target

1/15 Test #1: hash-target ...................... Passed 0.50 sec Start 2: core_tests 2/15 Test #2: core_tests .......................Exception: Other2284.95 sec Start 3: cncrypto 3/15 Test #3: cncrypto ......................... Passed 40.11 sec Start 4: unit_tests 4/15 Test #4: unit_tests .......................Failed 815.90 sec Start 5: difficulty 5/15 Test #5: difficulty ....................... Passed 0.07 sec Start 6: hash-fast 6/15 Test #6: hash-fast ........................ Passed 0.07 sec Start 7: hash-slow 7/15 Test #7: hash-slow ........................Failed 1.23 sec Start 8: hash-slow-1 8/15 Test #8: hash-slow-1 ......................Failed 1.76 sec Start 9: hash-slow-2 9/15 Test #9: hash-slow-2 ......................***Failed 3.37 sec Start 10: hash-tree 10/15 Test #10: hash-tree ........................ Passed 0.04 sec Start 11: hash-extra-blake 11/15 Test #11: hash-extra-blake ................. Passed 0.04 sec Start 12: hash-extra-groestl 12/15 Test #12: hash-extra-groestl ............... Passed 0.06 sec Start 13: hash-extra-jh 13/15 Test #13: hash-extra-jh .................... Passed 0.04 sec Start 14: hash-extra-skein 14/15 Test #14: hash-extra-skein ................. Passed 0.04 sec Start 15: hash-variant2-int-sqrt 15/15 Test #15: hash-variant2-int-sqrt ........... Passed 1240.98 sec

unit_tests.log.gz core_tests.log.gz cncrypto-tests.log.gz

xiphon commented 6 years ago

4755

nioroso-x3 commented 6 years ago

nvm, looks like its a WIP.

moneromooo-monero commented 6 years ago

For the slow hash tests:

diff --git a/src/crypto/slow-hash.c b/src/crypto/slow-hash.c
index 40cfb0461..ce716b652 100644
--- a/src/crypto/slow-hash.c
+++ b/src/crypto/slow-hash.c
@@ -109,8 +109,8 @@ extern int aesb_pseudo_round(const uint8_t *in, uint8_t *out, const uint8_t *exp
     memcpy(b + AES_BLOCK_SIZE, state.hs.b + 64, AES_BLOCK_SIZE); \
     xor64(b + AES_BLOCK_SIZE, state.hs.b + 80); \
     xor64(b + AES_BLOCK_SIZE + 8, state.hs.b + 88); \
-    division_result = state.hs.w[12]; \
-    sqrt_result = state.hs.w[13]; \
+    division_result = SWAP64LE(state.hs.w[12]); \
+    sqrt_result = SWAP64LE(state.hs.w[13]); \
   } while (0)

 #define VARIANT2_SHUFFLE_ADD_SSE2(base_ptr, offset) \
@@ -145,30 +145,31 @@ extern int aesb_pseudo_round(const uint8_t *in, uint8_t *out, const uint8_t *exp
     const uint64_t chunk1_old[2] = { chunk1[0], chunk1[1] }; \
     \
     uint64_t b1[2]; \
-    memcpy(b1, b + 16, 16); \
-    chunk1[0] = chunk3[0] + b1[0]; \
-    chunk1[1] = chunk3[1] + b1[1]; \
+    memcpy_swap64le(b1, b + 16, 2); \
+    chunk1[0] = SWAP64LE(chunk3[0] + b1[0]); \
+    chunk1[1] = SWAP64LE(chunk3[1] + b1[1]); \
     \
     uint64_t a0[2]; \
-    memcpy(a0, a, 16); \
-    chunk3[0] = chunk2[0] + a0[0]; \
-    chunk3[1] = chunk2[1] + a0[1]; \
+    memcpy_swap64le(a0, a, 2); \
+    chunk3[0] = SWAP64LE(chunk2[0] + a0[0]); \
+    chunk3[1] = SWAP64LE(chunk2[1] + a0[1]); \
     \
     uint64_t b0[2]; \
-    memcpy(b0, b, 16); \
-    chunk2[0] = chunk1_old[0] + b0[0]; \
-    chunk2[1] = chunk1_old[1] + b0[1]; \
+    memcpy_swap64le(b0, b, 2); \
+    chunk2[0] = SWAP64LE(chunk1_old[0] + b0[0]); \
+    chunk2[1] = SWAP64LE(chunk1_old[1] + b0[1]); \
   } while (0)

 #define VARIANT2_INTEGER_MATH_DIVISION_STEP(b, ptr) \
-  ((uint64_t*)(b))[0] ^= division_result ^ (sqrt_result << 32); \
+  uint64_t tmpx = division_result ^ (sqrt_result << 32); \
+  ((uint64_t*)(b))[0] ^= SWAP64LE(tmpx); \
   { \
-    const uint64_t dividend = ((uint64_t*)(ptr))[1]; \
-    const uint32_t divisor = (((uint64_t*)(ptr))[0] + (uint32_t)(sqrt_result << 1)) | 0x80000001UL; \
+    const uint64_t dividend = SWAP64LE(((uint64_t*)(ptr))[1]); \
+    const uint32_t divisor = (SWAP64LE(((uint64_t*)(ptr))[0]) + (uint32_t)(sqrt_result << 1)) | 0x80000001UL; \
     division_result = ((uint32_t)(dividend / divisor)) + \
                      (((uint64_t)(dividend % divisor)) << 32); \
   } \
-  const uint64_t sqrt_input = ((uint64_t*)(ptr))[0] + division_result
+  const uint64_t sqrt_input = SWAP64LE(((uint64_t*)(ptr))[0]) + division_result

 #define VARIANT2_INTEGER_MATH_SSE2(b, ptr) \
   do if (variant >= 2) \
@@ -207,14 +208,14 @@ extern int aesb_pseudo_round(const uint8_t *in, uint8_t *out, const uint8_t *exp
 #define VARIANT2_2() \
   do if (variant >= 2) \
   { \
-    *U64(hp_state + (j ^ 0x10)) ^= hi; \
-    *(U64(hp_state + (j ^ 0x10)) + 1) ^= lo; \
-    hi ^= *U64(hp_state + (j ^ 0x20)); \
-    lo ^= *(U64(hp_state + (j ^ 0x20)) + 1); \
+    *U64(hp_state + (j ^ 0x10)) ^= SWAP64LE(hi); \
+    *(U64(hp_state + (j ^ 0x10)) + 1) ^= SWAP64LE(lo); \
+    hi ^= SWAP64LE(*U64(hp_state + (j ^ 0x20))); \
+    lo ^= SWAP64LE(*(U64(hp_state + (j ^ 0x20)) + 1)); \
   } while (0)

-#if !defined NO_AES && (defined(__x86_64__) || (defined(_MSC_VER) && defined(_WIN64)))
+#if 0 && !defined NO_AES && (defined(__x86_64__) || (defined(_MSC_VER) && defined(_WIN64)))
 // Optimised code below, uses x86-specific intrinsics, SSE2, AES-NI
 // Fall back to more portable code is down at the bottom

@@ -1411,7 +1412,7 @@ static void (*const extra_hashes[4])(const void *, size_t, char *) = {
 extern int aesb_single_round(const uint8_t *in, uint8_t*out, const uint8_t *expandedKey);
 extern int aesb_pseudo_round(const uint8_t *in, uint8_t *out, const uint8_t *expandedKey);

-static size_t e2i(const uint8_t* a, size_t count) { return (*((uint64_t*)a) / AES_BLOCK_SIZE) & (count - 1); }
+static size_t e2i(const uint8_t* a, size_t count) { return (SWAP64LE(*((uint64_t*)a)) / AES_BLOCK_SIZE) & (count - 1); }

 static void mul(const uint8_t* a, const uint8_t* b, uint8_t* res) {
   uint64_t a0, b0;
jtgrassie commented 6 years ago

@moneromooo-monero hey moo, are all your PPC patches in a branch somewhere? I plan on doing some tests with them (and the the PRs).

moneromooo-monero commented 6 years ago

Not a single one. The ones that were reported to work are PRed (4757, 4726). xiphon has 4689.

xiphon commented 6 years ago

4755 And another one patch. Fixed software AES encryption on big endian.

@nioroso-x3 please do the following:

  1. Apply https://github.com/monero-project/monero/issues/3826#issuecomment-434428054
  2. Run tests
  3. Apply #4755
  4. Run tests
jtgrassie commented 6 years ago

Thanks both. I'll get some time this w/e hopefully so will get these in, test then move onto some of the other bits not covered by tests.

nioroso-x3 commented 6 years ago

4755 And another one patch. Fixed software AES encryption on big endian.

@nioroso-x3 please do the following:

  1. Apply #3826 (comment)
  2. Run tests
  3. Apply #4755
  4. Run tests

Patch #3826 (comment) had no visible effects. With patch #4755 hash-slow and hash-slow-1 pass, but hash-slow-2 fails. There were no changes for cncrypto and unit_tests with both patches, Serialization.portability still fails.

moneromooo-monero commented 6 years ago

Curious that 3826 has no visible effect since e2i is very unlikely to only pick data where the swap64le is equal to the input. if hash-slow works without it, adding it should break it. If you put a #warning in that function, does it get triggered ? Just to check you're using the version I thought you'd be using.

xiphon commented 6 years ago

@nioroso-x3 Please apply this one on top of the current working tree and check the tests

diff --git a/src/crypto/slow-hash.c b/src/crypto/slow-hash.c
index cc3f79c7..3c56e9d7 100644
--- a/src/crypto/slow-hash.c
+++ b/src/crypto/slow-hash.c
@@ -154,18 +154,18 @@ extern int aesb_pseudo_round(const uint8_t *in, uint8_t *out, const uint8_t *exp
     \
     uint64_t b1[2]; \
     memcpy_swap64le(b1, b + 16, 2); \
-    chunk1[0] = SWAP64LE(chunk3[0] + b1[0]); \
-    chunk1[1] = SWAP64LE(chunk3[1] + b1[1]); \
+    chunk1[0] = SWAP64LE(SWAP64LE(chunk3[0]) + b1[0]); \
+    chunk1[1] = SWAP64LE(SWAP64LE(chunk3[1]) + b1[1]); \
     \
     uint64_t a0[2]; \
     memcpy_swap64le(a0, a, 2); \
-    chunk3[0] = SWAP64LE(chunk2[0] + a0[0]); \
-    chunk3[1] = SWAP64LE(chunk2[1] + a0[1]); \
+    chunk3[0] = SWAP64LE(SWAP64LE(chunk2[0]) + a0[0]); \
+    chunk3[1] = SWAP64LE(SWAP64LE(chunk2[1]) + a0[1]); \
     \
     uint64_t b0[2]; \
     memcpy_swap64le(b0, b, 2); \
-    chunk2[0] = SWAP64LE(chunk1_old[0] + b0[0]); \
-    chunk2[1] = SWAP64LE(chunk1_old[1] + b0[1]); \
+    chunk2[0] = SWAP64LE(SWAP64LE(chunk1_old[0]) + b0[0]); \
+    chunk2[1] = SWAP64LE(SWAP64LE(chunk1_old[1]) + b0[1]); \
   } while (0)

 #define VARIANT2_INTEGER_MATH_DIVISION_STEP(b, ptr) \
moneromooo-monero commented 6 years ago

Oh nice catch :)