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.

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

Incorrect SHA1 hash generated on Raspberry PI #45

Closed
Shielsy opened this issue May 18, 2018 · 13 comments
Closed

Incorrect SHA1 hash generated on Raspberry PI #45

Shielsy opened this issue May 18, 2018 · 13 comments

Comments

@Shielsy
Copy link

Shielsy commented May 18, 2018

Device: Raspberry PI 3 Model B+
O/S: Raspbian 9 (stretch)

I realise that the Raspberry PI isn't a supported platform but I've noticed an issue and thought I would raise it anyway. The following doesn't generate a correct SHA1 hash on a Raspberry PI.

Utility::Sha1::digest("test").hexString()

I've tested the results against echo -n test | shasum -a 1 | awk '{print $1}'.

Thanks,
Andy

@Shielsy Shielsy changed the title Incorrect SHA1 generated on Raspberry PI Incorrect SHA1 hash generated on Raspberry PI May 18, 2018
@mosra
Copy link
Owner

mosra commented May 19, 2018

Hi,

RPI is embedded Linux, so it is a supported platform :) I successfully used Corrade on a BeagleBoard, some Tegra chip and other embedded devices.

Can you build with tests enabled (BUILD_TESTS CMake option) and run ./build/src/Corrade/Utility/Test/UtilitySha1Test? Curious what that prints out.

@Shielsy
Copy link
Author

Shielsy commented May 22, 2018

Thanks for the quick reply. I've ran all tests and the results are below.

There are 2 failures:

  9 - UtilityDebugTest (Failed)
 14 - UtilitySha1Test (Failed)
Test project /home/pi/tmp/corrade/as_build/src/Corrade/Utility/Test
      Start  1: UtilityArgumentsTest
 1/20 Test  #1: UtilityArgumentsTest .............   Passed    0.02 sec
      Start  2: UtilityAssertTest
 2/20 Test  #2: UtilityAssertTest ................   Passed    0.01 sec
      Start  3: UtilityAssertDisabledTest
 3/20 Test  #3: UtilityAssertDisabledTest ........   Passed    0.01 sec
      Start  4: UtilityAssertGracefulTest
 4/20 Test  #4: UtilityAssertGracefulTest ........   Passed    0.01 sec
      Start  5: UtilityEndianTest
 5/20 Test  #5: UtilityEndianTest ................   Passed    0.01 sec
      Start  6: UtilityMurmurHash2Test
 6/20 Test  #6: UtilityMurmurHash2Test ...........   Passed    0.02 sec
      Start  7: UtilityConfigurationTest
 7/20 Test  #7: UtilityConfigurationTest .........   Passed    0.05 sec
      Start  8: UtilityConfigurationValueTest
 8/20 Test  #8: UtilityConfigurationValueTest ....   Passed    0.01 sec
      Start  9: UtilityDebugTest
 9/20 Test  #9: UtilityDebugTest .................***Failed    0.01 sec
Starting Corrade::Utility::Test::DebugTest with 33 test cases...
Debug output is a TTY?   no
Warning output is a TTY? no
Error output is a TTY?   no
    OK [01] isTty()
    OK [02] boolean()
    OK [03] floats<float>()
    OK [04] floats<double>()
  FAIL [05] floats<long double>() at /home/pi/tmp/corrade/src/Corrade/Utility/Test/DebugTest.cpp on line 202
        Values o.str() and FloatsData<T>::expected() are not the same, actual is
        3.14159265358979312 -12345.6789012345671 1.23456789012345688e-12 3.14159

        but expected
        3.14159265358979324 -12345.6789012345679 1.23456789012345679e-12 3.14159

    OK [06] chars()
    OK [07] pointer()
    OK [08] unicode()
    OK [09] custom()
    OK [10] nospace()
    OK [11] newline()
    OK [12] noNewlineAtTheEnd()
Black bold
    OK [13] colors(Black)
Red bold
    OK [14] colors(Red)
Green bold
    OK [15] colors(Green)
Yellow bold
    OK [16] colors(Yellow)
Blue bold
    OK [17] colors(Blue)
Magenta bold
    OK [18] colors(Magenta)
Cyan bold
    OK [19] colors(Cyan)
White bold
    OK [20] colors(White)
Default bold
    OK [21] colors(Default)
Default Green
    OK [22] colorsAutoReset()
Red Default
    OK [23] colorsExplicitReset()
Default Default
    OK [24] colorsDisabled()
Hello
Hello
    OK [25] colorsNospace()
This shouldn't be red.
  SKIP [26] colorsNoOutput()
        Only possible to test visually.
This should have default color.
This should be cyan.
This also and this blue.
This should be cyan again.
Disabling colors shouldn't affect outer scope, so also cyan.
And this resets back to default color.
    OK [27] colorsScoped()
    OK [28] iterable()
    OK [29] tuple()
    OK [30] ostreamFallback()
    OK [31] ostreamFallbackPriority()
    OK [32] scopedOutput()
    OK [33] debugColor()
Finished Corrade::Utility::Test::DebugTest with 1 errors out of 61 checks.

      Start 10: UtilityFatalTest
10/20 Test #10: UtilityFatalTest .................   Passed    0.01 sec
      Start 11: UtilityDirectoryTest
11/20 Test #11: UtilityDirectoryTest .............   Passed    0.06 sec
      Start 12: UtilityHashDigestTest
12/20 Test #12: UtilityHashDigestTest ............   Passed    0.01 sec
      Start 13: UtilityMacrosTest
13/20 Test #13: UtilityMacrosTest ................   Passed    0.01 sec
      Start 14: UtilitySha1Test
14/20 Test #14: UtilitySha1Test ..................***Failed    0.01 sec
Starting Corrade::Utility::Test::Sha1Test with 6 test cases...
  FAIL [1] emptyString() at /home/pi/tmp/corrade/src/Corrade/Utility/Test/Sha1Test.cpp on line 62
        Values Sha1::digest("") and Sha1::Digest::fromHexString("da39a3ee5e6b4b0d3255bfef95601890afd80709") are not the same, actual is
        7073c2761c38837eb837837ef037837eafd80709
        but expected
        da39a3ee5e6b4b0d3255bfef95601890afd80709
  FAIL [2] exact64bytes() at /home/pi/tmp/corrade/src/Corrade/Utility/Test/Sha1Test.cpp on line 67
        Values Sha1::digest("123456789a123456789b123456789c123456789d123456789e123456789f1234") and Sha1::Digest::fromHexString("d9aa447706df8797b4f5fe94caa9f6ea723a87c8") are not the same, actual is
        7073c276046bf4762038837ea437837e723a87c8
        but expected
        d9aa447706df8797b4f5fe94caa9f6ea723a87c8
  FAIL [3] exactOneBlockPadding() at /home/pi/tmp/corrade/src/Corrade/Utility/Test/Sha1Test.cpp on line 72
        Values Sha1::digest("123456789a123456789b123456789c123456789d123456789e12345") and Sha1::Digest::fromHexString("4cc8d5cfacbb575ddeeed504dd4f7cc09a9d49a3") are not the same, actual is
        7073c276c04d837e0038837ea437837e9a9d49a3
        but expected
        4cc8d5cfacbb575ddeeed504dd4f7cc09a9d49a3
  FAIL [4] twoBlockPadding() at /home/pi/tmp/corrade/src/Corrade/Utility/Test/Sha1Test.cpp on line 77
        Values Sha1::digest("123456789a123456789b123456789c123456789d123456789e123456") and Sha1::Digest::fromHexString("40e94c62ada5dc762f3e9c472001ca64a67d2cbb") are not the same, actual is
        7073c27600000000a437837ef037837ea67d2cbb
        but expected
        40e94c62ada5dc762f3e9c472001ca64a67d2cbb
  FAIL [5] iterative()@1 at /home/pi/tmp/corrade/src/Corrade/Utility/Test/Sha1Test.cpp on line 100
        Values hasher.digest() and Sha1::Digest::fromHexString("cd36b370758a259b34845084a6cc38473cb95e27") are not the same, actual is
        640000000000000001000000bc0100003cb95e27
        but expected
        cd36b370758a259b34845084a6cc38473cb95e27
  FAIL [6] reuse() at /home/pi/tmp/corrade/src/Corrade/Utility/Test/Sha1Test.cpp on line 106
        Values hasher.digest() and Sha1::Digest::fromHexString("cd36b370758a259b34845084a6cc38473cb95e27") are not the same, actual is
        6a000000f037837e6037837e000000003cb95e27
        but expected
        cd36b370758a259b34845084a6cc38473cb95e27
Finished Corrade::Utility::Test::Sha1Test with 6 errors out of 6 checks.

      Start 15: UtilityStringTest
15/20 Test #15: UtilityStringTest ................   Passed    0.01 sec
      Start 16: UtilitySystemTest
16/20 Test #16: UtilitySystemTest ................   Passed    0.02 sec
      Start 17: UtilityTypeTraitsTest
17/20 Test #17: UtilityTypeTraitsTest ............   Passed    0.01 sec
      Start 18: UtilityUnicodeTest
18/20 Test #18: UtilityUnicodeTest ...............   Passed    0.01 sec
      Start 19: UtilityResourceTest
19/20 Test #19: UtilityResourceTest ..............   Passed    0.01 sec
      Start 20: UtilityResourceStaticTest
20/20 Test #20: UtilityResourceStaticTest ........   Passed    0.01 sec

90% tests passed, 2 tests failed out of 20

Total Test time (real) =   0.35 sec

The following tests FAILED:
	  9 - UtilityDebugTest (Failed)
	 14 - UtilitySha1Test (Failed)
Errors while running CTest

@mosra
Copy link
Owner

mosra commented May 22, 2018

Hi, thanks a lot!

The long double failure is kinda expected. As far as I could see, long double is the same as double on RPI. I'll fix the test to skip testing for long double if it's the same as double.

However, the SHA-1 is ... weird. It produces correct value only for the last 8 characters (4 bytes) and the rest is totally wrong, even for the empty case. Not sure how to debug it here, as the closest I could get to a RPI is a 64bit Android phone and there it works.

Anything special with the RPI? char being unsigned by default? booting into big-endian mode? or something like that? Does the compiler print any warnings (strict aliasing etc.) when compiling the code?

@mosra mosra added this to TODO in Platforms via automation May 22, 2018
@Shielsy
Copy link
Author

Shielsy commented May 22, 2018

The RPI is straight out of the box and I haven't done anything special with it.

I was building a Release build. I've just rebuilt a Debug build and now the Sha1 tests all pass :)

There are few warnings when building the Release build (pasted below):

[ 97%] Building CXX object doc/snippets/CMakeFiles/testsuite-benchmark.dir/testsuite-benchmark.cpp.o
/home/pi/tmp/corrade/doc/snippets/testsuite-benchmark.cpp: In function ‘float {anonymous}::fastinvsqrt(float)’:
/home/pi/tmp/corrade/doc/snippets/testsuite-benchmark.cpp:41:35: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
     i = *reinterpret_cast<int*>(&y);
                                   ^
/home/pi/tmp/corrade/doc/snippets/testsuite-benchmark.cpp:43:37: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
     y = *reinterpret_cast<float*>(&i);
                                     ^
[ 98%] Linking CXX executable testsuite-benchmark
[ 98%] Built target testsuite-benchmark

Scanning dependencies of target ContainersEnumSetTest
[ 14%] Building CXX object src/Corrade/Containers/Test/CMakeFiles/ContainersEnumSetTest.dir/EnumSetTest.cpp.o
/home/pi/tmp/corrade/src/Corrade/Containers/Test/EnumSetTest.cpp: In member function ‘void Corrade::Containers::Test::EnumSetTest::constructNoInit()’:
/home/pi/tmp/corrade/src/Corrade/Containers/Test/EnumSetTest.cpp:118:25: warning: ‘features.Corrade::Containers::EnumSet<Corrade::Containers::Test::{anonymous}::Feature, 15>::value’ may be used uninitialized in this function [-Wmaybe-uninitialized]
         CORRADE_COMPARE(int(features), 4);
                         ^~~

@mosra
Copy link
Owner

mosra commented May 22, 2018

I was building a Release build. I've just rebuilt a Debug build and now the Sha1 tests all pass :)

Oh well, the worst kind of bugs :/ What's the GCC version?

The warnings above are harmless.

@Shielsy
Copy link
Author

Shielsy commented May 22, 2018

Yeah - not a nice bug :(

GCC version is: 6.3.0 20170516 - 6.3.0-18+rpi1+deb9ui

@mosra
Copy link
Owner

mosra commented May 24, 2018

Just out of curiosity: are you able to get Clang there and try with it, if it produces the same error?

Unless you want to tackle it yourself, I'm afraid that I don't have any chance of fixing this until I get a hold of some RPI board...

@Shielsy
Copy link
Author

Shielsy commented May 25, 2018

I've rebuilt with Clang and the test passes in both Release/Debug.

@mosra
Copy link
Owner

mosra commented May 25, 2018

Okay, that's one more indication that it's really some GCC optimization bug. Some more ideas (apologies in advance if I'm explaining something you already know):

  • CMAKE_CXX_FLAGS_RELEASE (if you edit the cache file or look through ccmake) contains some flags and some of them might trigger this. Can you try dialing them down to -O2 or -O1 and see if that helps?
  • Each -O level is grouping various optimization flags. So for example, if it worked on -O2 and not on -O3 then you could try enabling the -O3 flags in addition to -O2 one by one until one of them breaks it.
  • Then maybe, depending on what flag triggered it, it could be possible to figure out some change to the code that would work around the misoptimization.
  • This could be also worked around by putting for example #pragma GCC optimize("O2") around the affected code if compiling under GCC 6 on ARM, but that's a nuclear option :)
  • Maybe this is a GCC 6-specific bug and it was fixed in GCC 7(.0, .1, .2, .3)? I have no idea if you are able to get GCC 7 there, though. If newer GCC works, you could try going through the changelogs and search for arm, neon or aarch64 to pinpoint a specific bug and with some luck there could be a workaround mentioned.

Or, well, just use Clang, if you can. As I said, I don't have access to ARM64 RPI, so the suggestions above are all I can do :)

@Shielsy
Copy link
Author

Shielsy commented May 25, 2018

Thanks for the detailed response.

I was experimenting with the optimisation flags. Tests pass with -O2 but fail with -O3.

I installed GCC8 following these instructions and the Sha1 tests pass on all builds. I haven't tested with GCC7 so I'm not sure when the issue was fixed. I'm happy to stick with GCC8 for now so you can close this issue unless you wish to investigate further. Thanks for your help.

@mosra
Copy link
Owner

mosra commented May 25, 2018

Great, thanks for trying this out. I came up with the following patch that forces -O2 on GCC < 8 on ARM for the whole code. It's still the "nuclear option", but since it's already fixed in GCC 8, it doesn't affect all users and will get obsoleted over time, so I think it's okay.

diff --git a/src/Corrade/Utility/Sha1.cpp b/src/Corrade/Utility/Sha1.cpp
index b6e84ea1..f3a9b5a6 100644
--- a/src/Corrade/Utility/Sha1.cpp
+++ b/src/Corrade/Utility/Sha1.cpp
@@ -52,6 +52,18 @@ unsigned int leftrotate(unsigned int data, unsigned int shift) {
 
 Sha1::Sha1(): _dataSize(0), _digest{InitialDigest[0], InitialDigest[1], InitialDigest[2], InitialDigest[3], InitialDigest[4]} {}
 
+#if defined(__GNUC__) && !defined(__clang__) && defined(CORRADE_TARGET_ARM) && __GNUC__ < 8
+#pragma GCC push_options
+#pragma GCC optimize ("O2")
+#endif
+
 Sha1& Sha1::operator<<(const std::string& data) {
     const std::size_t dataOffset = _buffer.empty() ? 0 : 64 - _buffer.size();
 
@@ -154,4 +166,9 @@ void Sha1::processChunk(const char* data) {
         _digest[i] += d[i];
 }
 
+#if defined(__GNUC__) && !defined(__clang__) && defined(CORRADE_TARGET_ARM) && __GNUC__ < 8
+#pragma GCC pop_options
+#endif
+
 }}

If you got some time, could you try applying it and compiling with GCC 6 again? The best would be if you could try to reduce scope of the pragma to maybe just a single function, but I abused you as a remote debugger enough already ;)

Thanks a lot!

@Shielsy
Copy link
Author

Shielsy commented Jun 8, 2018

Sorry about the long delay - I've managed to narrow down the issue to the line below in the digest() function. I haven't had time to investigate any further but thought I'd update .

Digest d = Digest::fromByteArray(reinterpret_cast<const char*>(digest));

@mosra mosra added this to the 2018.0c milestone Jun 8, 2018
@mosra
Copy link
Owner

mosra commented Jun 8, 2018

Thanks a lot for getting back to this and for narrowing it down to a single line 👍

I pushed the change as 052a560, will appear in master shortly. Since this affects only GCC 6, is fixed again in newer versions and can be worked around like this, I don't think it's worth spending more time on it (unless it reappears again later).

@mosra mosra closed this as completed Jun 8, 2018
Platforms automation moved this from TODO to Done Jun 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Platforms
  
Done
Development

No branches or pull requests

2 participants