diff options
author | iphydf <iphydf@users.noreply.github.com> | 2018-02-03 16:13:22 +0000 |
---|---|---|
committer | iphydf <iphydf@users.noreply.github.com> | 2018-02-06 20:21:27 +0000 |
commit | 835b9fbdc933878a744cd6ba1c77942610f4fe06 (patch) | |
tree | da7286c8a8f00106eb92bf53701c8711b20612ff | |
parent | feaefbcbb3ab5bd3fe7dc963d6f963669673fadc (diff) |
Improve stability of crypto_memcmp test.
Also reduce number of people in conference to 5, because on Circle CI the
test times out trying to connect more than 6 or 7 people. The persistent
conferences PR will improve this so we can set it much higher then.
-rw-r--r-- | .travis.yml | 1 | ||||
-rw-r--r-- | CMakeLists.txt | 44 | ||||
-rw-r--r-- | auto_tests/conference_test.c | 2 | ||||
-rw-r--r-- | auto_tests/crypto_test.c | 78 | ||||
-rw-r--r-- | circle.yml | 2 | ||||
-rw-r--r-- | other/travis/env.sh | 1 | ||||
-rwxr-xr-x | other/travis/toxcore-script | 1 | ||||
-rw-r--r-- | toxcore/BUILD.bazel | 9 | ||||
-rw-r--r-- | toxcore/crypto_core_test.cpp | 96 |
9 files changed, 153 insertions, 81 deletions
diff --git a/.travis.yml b/.travis.yml index 48f5ce3a..1b1a1606 100644 --- a/.travis.yml +++ b/.travis.yml | |||
@@ -24,6 +24,7 @@ matrix: | |||
24 | - libopencv-contrib-dev # For av_test. | 24 | - libopencv-contrib-dev # For av_test. |
25 | - libopus-dev # For toxav. | 25 | - libopus-dev # For toxav. |
26 | - libsndfile1-dev # For av_test. | 26 | - libsndfile1-dev # For av_test. |
27 | - libgtest-dev # For unit tests. | ||
27 | - libvpx-dev # For toxav. | 28 | - libvpx-dev # For toxav. |
28 | - opam # For apidsl and Frama-C. | 29 | - opam # For apidsl and Frama-C. |
29 | - portaudio19-dev # For av_test. | 30 | - portaudio19-dev # For av_test. |
diff --git a/CMakeLists.txt b/CMakeLists.txt index 542f151d..8cd5bae6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt | |||
@@ -429,7 +429,49 @@ install_module(toxcore DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/tox) | |||
429 | 429 | ||
430 | ################################################################################ | 430 | ################################################################################ |
431 | # | 431 | # |
432 | # :: Automated regression tests | 432 | # :: Unit tests: no networking, just pure function calls. |
433 | # | ||
434 | ################################################################################ | ||
435 | |||
436 | # Compile the GTest library. | ||
437 | # | ||
438 | if(EXISTS "/usr/src/gtest/src/gtest-all.cc") | ||
439 | add_library(gtest | ||
440 | /usr/src/gtest/src/gtest-all.cc | ||
441 | /usr/src/gtest/src/gtest_main.cc) | ||
442 | target_include_directories(gtest PRIVATE /usr/src/gtest) | ||
443 | check_cxx_compiler_flag("-w" HAVE_CXX_W QUIET) | ||
444 | check_cxx_compiler_flag("-Wno-global-constructors" HAVE_CXX_W_NO_GLOBAL_CONSTRUCTORS QUIET) | ||
445 | check_cxx_compiler_flag("-Wno-zero-as-null-pointer-constant" HAVE_CXX_W_NO_ZERO_AS_NULL_POINTER_CONSTANT QUIET) | ||
446 | if(HAVE_CXX_W) | ||
447 | set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-w") | ||
448 | endif() | ||
449 | set(HAVE_GTEST TRUE) | ||
450 | endif() | ||
451 | |||
452 | function(unit_test subdir target) | ||
453 | if(HAVE_GTEST) | ||
454 | add_executable(unit_${target}_test ${subdir}/${target}_test.cpp) | ||
455 | target_link_modules(unit_${target}_test ${toxcore_SUBLIBS} gtest) | ||
456 | set(gtest_CFLAGS "") | ||
457 | if(HAVE_CXX_W_NO_GLOBAL_CONSTRUCTORS) | ||
458 | set(gtest_CFLAGS "${gtest_CFLAGS} -Wno-global-constructors") | ||
459 | endif() | ||
460 | if(HAVE_CXX_W_NO_ZERO_AS_NULL_POINTER_CONSTANT) | ||
461 | set(gtest_CFLAGS "${gtest_CFLAGS} -Wno-zero-as-null-pointer-constant") | ||
462 | endif() | ||
463 | set_target_properties(unit_${target}_test PROPERTIES COMPILE_FLAGS "${gtest_CFLAGS}") | ||
464 | add_test(NAME ${target} COMMAND ${CROSSCOMPILING_EMULATOR} unit_${target}_test) | ||
465 | endif() | ||
466 | endfunction() | ||
467 | |||
468 | # The actual unit tests follow. | ||
469 | # | ||
470 | unit_test(toxcore crypto_core) | ||
471 | |||
472 | ################################################################################ | ||
473 | # | ||
474 | # :: Automated regression tests: create a tox network and run integration tests | ||
433 | # | 475 | # |
434 | ################################################################################ | 476 | ################################################################################ |
435 | 477 | ||
diff --git a/auto_tests/conference_test.c b/auto_tests/conference_test.c index 2fbb9c98..bd3ea8d0 100644 --- a/auto_tests/conference_test.c +++ b/auto_tests/conference_test.c | |||
@@ -20,7 +20,7 @@ | |||
20 | 20 | ||
21 | #include "helpers.h" | 21 | #include "helpers.h" |
22 | 22 | ||
23 | #define NUM_GROUP_TOX 8 | 23 | #define NUM_GROUP_TOX 5 |
24 | 24 | ||
25 | static void handle_self_connection_status(Tox *tox, TOX_CONNECTION connection_status, void *user_data) | 25 | static void handle_self_connection_status(Tox *tox, TOX_CONNECTION connection_status, void *user_data) |
26 | { | 26 | { |
diff --git a/auto_tests/crypto_test.c b/auto_tests/crypto_test.c index a1b7fd30..045cd7ab 100644 --- a/auto_tests/crypto_test.c +++ b/auto_tests/crypto_test.c | |||
@@ -339,83 +339,6 @@ START_TEST(test_memzero) | |||
339 | } | 339 | } |
340 | END_TEST | 340 | END_TEST |
341 | 341 | ||
342 | #define CRYPTO_TEST_MEMCMP_SIZE 1024*1024 // 1MiB | ||
343 | #define CRYPTO_TEST_MEMCMP_COUNT 2000 | ||
344 | #define CRYPTO_TEST_MEMCMP_EPS 10 | ||
345 | |||
346 | static int cmp(const void *a, const void *b) | ||
347 | { | ||
348 | const clock_t *first = (const clock_t *) a; | ||
349 | const clock_t *second = (const clock_t *) b; | ||
350 | |||
351 | if (*first < *second) { | ||
352 | return -1; | ||
353 | } | ||
354 | |||
355 | if (*first > *second) { | ||
356 | return 1; | ||
357 | } | ||
358 | |||
359 | return 0; | ||
360 | } | ||
361 | |||
362 | static clock_t memcmp_time(void *a, void *b, size_t len) | ||
363 | { | ||
364 | clock_t start = clock(); | ||
365 | crypto_memcmp(a, b, len); | ||
366 | return clock() - start; | ||
367 | } | ||
368 | |||
369 | static clock_t memcmp_median(void *a, void *b, size_t len) | ||
370 | { | ||
371 | size_t i; | ||
372 | clock_t results[CRYPTO_TEST_MEMCMP_COUNT]; | ||
373 | |||
374 | for (i = 0; i < CRYPTO_TEST_MEMCMP_COUNT; i++) { | ||
375 | results[i] = memcmp_time(a, b, len); | ||
376 | } | ||
377 | |||
378 | qsort(results, CRYPTO_TEST_MEMCMP_COUNT, sizeof(*results), cmp); | ||
379 | return results[CRYPTO_TEST_MEMCMP_COUNT / 2]; | ||
380 | } | ||
381 | |||
382 | START_TEST(test_memcmp) | ||
383 | { | ||
384 | uint8_t *src = (uint8_t *)malloc(CRYPTO_TEST_MEMCMP_SIZE); | ||
385 | rand_bytes(src, CRYPTO_TEST_MEMCMP_SIZE); | ||
386 | |||
387 | uint8_t *same = (uint8_t *)malloc(CRYPTO_TEST_MEMCMP_SIZE); | ||
388 | memcpy(same, src, CRYPTO_TEST_MEMCMP_SIZE); | ||
389 | |||
390 | uint8_t *not_same = (uint8_t *)malloc(CRYPTO_TEST_MEMCMP_SIZE); | ||
391 | rand_bytes(not_same, CRYPTO_TEST_MEMCMP_SIZE); | ||
392 | |||
393 | printf("timing memcmp (equal arrays)\n"); | ||
394 | clock_t same_median = memcmp_median(src, same, CRYPTO_TEST_MEMCMP_SIZE); | ||
395 | printf("timing memcmp (non-equal arrays)\n"); | ||
396 | clock_t not_same_median = memcmp_median(src, not_same, CRYPTO_TEST_MEMCMP_SIZE); | ||
397 | printf("comparing times\n"); | ||
398 | |||
399 | free(not_same); | ||
400 | free(same); | ||
401 | free(src); | ||
402 | |||
403 | clock_t delta; | ||
404 | |||
405 | if (same_median > not_same_median) { | ||
406 | delta = same_median - not_same_median; | ||
407 | } else { | ||
408 | delta = not_same_median - same_median; | ||
409 | } | ||
410 | |||
411 | ck_assert_msg(delta < CRYPTO_TEST_MEMCMP_EPS, | ||
412 | "Delta time is too long (%d >= %d)\n" | ||
413 | "Time of the same data comparation: %d\n" | ||
414 | "Time of the different data comparation: %d", | ||
415 | delta, CRYPTO_TEST_MEMCMP_EPS, same_median, not_same_median); | ||
416 | } | ||
417 | END_TEST | ||
418 | |||
419 | static Suite *crypto_suite(void) | 342 | static Suite *crypto_suite(void) |
420 | { | 343 | { |
421 | Suite *s = suite_create("Crypto"); | 344 | Suite *s = suite_create("Crypto"); |
@@ -427,7 +350,6 @@ static Suite *crypto_suite(void) | |||
427 | DEFTESTCASE(large_data_symmetric); | 350 | DEFTESTCASE(large_data_symmetric); |
428 | DEFTESTCASE_SLOW(increment_nonce, 20); | 351 | DEFTESTCASE_SLOW(increment_nonce, 20); |
429 | DEFTESTCASE(memzero); | 352 | DEFTESTCASE(memzero); |
430 | DEFTESTCASE_SLOW(memcmp, 30); | ||
431 | 353 | ||
432 | return s; | 354 | return s; |
433 | } | 355 | } |
@@ -137,4 +137,4 @@ compile: | |||
137 | 137 | ||
138 | test: | 138 | test: |
139 | override: | 139 | override: |
140 | - make VERBOSE=1 test || make VERBOSE=1 ARGS="--rerun-failed" test || make VERBOSE=1 ARGS="--rerun-failed" test || make VERBOSE=1 ARGS="--rerun-failed" test | 140 | - make test ARGS="--rerun-failed" CTEST_OUTPUT_ON_FAILURE=1 || make test ARGS="--rerun-failed" CTEST_OUTPUT_ON_FAILURE=1 || make test ARGS="--rerun-failed" CTEST_OUTPUT_ON_FAILURE=1 || make test ARGS="--rerun-failed" CTEST_OUTPUT_ON_FAILURE=1 || make test ARGS="--rerun-failed" CTEST_OUTPUT_ON_FAILURE=1 |
diff --git a/other/travis/env.sh b/other/travis/env.sh index f4ef8ef3..7d90d395 100644 --- a/other/travis/env.sh +++ b/other/travis/env.sh | |||
@@ -7,6 +7,7 @@ export LD_LIBRARY_PATH=$CACHE_DIR/lib | |||
7 | export PKG_CONFIG_PATH=$CACHE_DIR/lib/pkgconfig | 7 | export PKG_CONFIG_PATH=$CACHE_DIR/lib/pkgconfig |
8 | export ASTYLE=$CACHE_DIR/astyle/build/gcc/bin/astyle | 8 | export ASTYLE=$CACHE_DIR/astyle/build/gcc/bin/astyle |
9 | export CFLAGS="-O3 -DTRAVIS_ENV=1" | 9 | export CFLAGS="-O3 -DTRAVIS_ENV=1" |
10 | export CXXFLAGS="-O3 -DTRAVIS_ENV=1" | ||
10 | export CMAKE_EXTRA_FLAGS="-DERROR_ON_WARNING=ON" | 11 | export CMAKE_EXTRA_FLAGS="-DERROR_ON_WARNING=ON" |
11 | export MAKE=make | 12 | export MAKE=make |
12 | 13 | ||
diff --git a/other/travis/toxcore-script b/other/travis/toxcore-script index 763be2e4..82c39ce9 100755 --- a/other/travis/toxcore-script +++ b/other/travis/toxcore-script | |||
@@ -2,6 +2,7 @@ | |||
2 | 2 | ||
3 | # Enable test coverage recording. | 3 | # Enable test coverage recording. |
4 | export CFLAGS="$CFLAGS -fprofile-arcs -ftest-coverage" | 4 | export CFLAGS="$CFLAGS -fprofile-arcs -ftest-coverage" |
5 | export CXXFLAGS="$CXXFLAGS -fprofile-arcs -ftest-coverage" | ||
5 | 6 | ||
6 | set_opt() { | 7 | set_opt() { |
7 | opts="$opts -D$1="`expr ON \& \( $i % 2 \) \| OFF` | 8 | opts="$opts -D$1="`expr ON \& \( $i % 2 \) \| OFF` |
diff --git a/toxcore/BUILD.bazel b/toxcore/BUILD.bazel index e9ffdbd2..3b96dd6d 100644 --- a/toxcore/BUILD.bazel +++ b/toxcore/BUILD.bazel | |||
@@ -28,6 +28,15 @@ cc_library( | |||
28 | ], | 28 | ], |
29 | ) | 29 | ) |
30 | 30 | ||
31 | cc_test( | ||
32 | name = "crypto_core_test", | ||
33 | srcs = ["crypto_core_test.cpp"], | ||
34 | deps = [ | ||
35 | ":crypto_core", | ||
36 | "@gtest", | ||
37 | ], | ||
38 | ) | ||
39 | |||
31 | cc_library( | 40 | cc_library( |
32 | name = "list", | 41 | name = "list", |
33 | srcs = ["list.c"], | 42 | srcs = ["list.c"], |
diff --git a/toxcore/crypto_core_test.cpp b/toxcore/crypto_core_test.cpp new file mode 100644 index 00000000..8f91dce8 --- /dev/null +++ b/toxcore/crypto_core_test.cpp | |||
@@ -0,0 +1,96 @@ | |||
1 | #include "crypto_core.h" | ||
2 | |||
3 | #include <algorithm> | ||
4 | |||
5 | #include <gtest/gtest.h> | ||
6 | |||
7 | namespace | ||
8 | { | ||
9 | |||
10 | enum { | ||
11 | /** | ||
12 | * The size of the arrays to compare. This was chosen to take around 2000 | ||
13 | * CPU clocks on x86_64. | ||
14 | */ | ||
15 | CRYPTO_TEST_MEMCMP_SIZE = 1024 * 1024, // 1 MiB | ||
16 | /** | ||
17 | * The number of times we run memcmp in the test. | ||
18 | * | ||
19 | * We compute the median time taken to reduce error margins. | ||
20 | */ | ||
21 | CRYPTO_TEST_MEMCMP_ITERATIONS = 500, | ||
22 | /** | ||
23 | * The margin of error (in clocks) we allow for this test. | ||
24 | * | ||
25 | * Should be within 0.5% of ~2000 CPU clocks. In reality, the code is much | ||
26 | * more precise and is usually within 1 CPU clock. | ||
27 | */ | ||
28 | CRYPTO_TEST_MEMCMP_EPS = 10, | ||
29 | }; | ||
30 | |||
31 | clock_t memcmp_time(void *a, void *b, size_t len) | ||
32 | { | ||
33 | clock_t start = clock(); | ||
34 | crypto_memcmp(a, b, len); | ||
35 | return clock() - start; | ||
36 | } | ||
37 | |||
38 | /** | ||
39 | * This function performs the actual timing. It interleaves comparison of | ||
40 | * equal and non-equal arrays to reduce the influence of external effects | ||
41 | * such as the machine being a little more busy 1 second later. | ||
42 | */ | ||
43 | void memcmp_median(void *src, void *same, void *not_same, size_t len, | ||
44 | clock_t *same_median, clock_t *not_same_median) | ||
45 | { | ||
46 | clock_t same_results[CRYPTO_TEST_MEMCMP_ITERATIONS]; | ||
47 | clock_t not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS]; | ||
48 | |||
49 | for (size_t i = 0; i < CRYPTO_TEST_MEMCMP_ITERATIONS; i++) { | ||
50 | same_results[i] = memcmp_time(src, same, len); | ||
51 | not_same_results[i] = memcmp_time(src, not_same, len); | ||
52 | } | ||
53 | |||
54 | std::sort(same_results, same_results + CRYPTO_TEST_MEMCMP_ITERATIONS); | ||
55 | *same_median = same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2]; | ||
56 | std::sort(not_same_results, not_same_results + CRYPTO_TEST_MEMCMP_ITERATIONS); | ||
57 | *not_same_median = not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2]; | ||
58 | } | ||
59 | |||
60 | /** | ||
61 | * This test checks whether crypto_memcmp takes the same time for equal and | ||
62 | * non-equal chunks of memory. | ||
63 | */ | ||
64 | TEST(CryptoCore, MemcmpTimingIsDataIndependent) | ||
65 | { | ||
66 | // A random piece of memory. | ||
67 | uint8_t *src = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE]; | ||
68 | random_bytes(src, CRYPTO_TEST_MEMCMP_SIZE); | ||
69 | |||
70 | // A separate piece of memory containing the same data. | ||
71 | uint8_t *same = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE]; | ||
72 | memcpy(same, src, CRYPTO_TEST_MEMCMP_SIZE); | ||
73 | |||
74 | // Another piece of memory containing different data. | ||
75 | uint8_t *not_same = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE]; | ||
76 | random_bytes(not_same, CRYPTO_TEST_MEMCMP_SIZE); | ||
77 | |||
78 | clock_t same_median; | ||
79 | clock_t not_same_median; | ||
80 | memcmp_median(src, same, not_same, CRYPTO_TEST_MEMCMP_SIZE, &same_median, ¬_same_median); | ||
81 | |||
82 | delete[] not_same; | ||
83 | delete[] same; | ||
84 | delete[] src; | ||
85 | |||
86 | clock_t const delta = same_median > not_same_median | ||
87 | ? same_median - not_same_median | ||
88 | : not_same_median - same_median; | ||
89 | |||
90 | EXPECT_LT(delta, CRYPTO_TEST_MEMCMP_EPS) | ||
91 | << "Delta time is too long (" << delta << " >= " << CRYPTO_TEST_MEMCMP_EPS << ")\n" | ||
92 | << "Time of the same data comparation: " << same_median << " clocks\n" | ||
93 | << "Time of the different data comparation: " << not_same_median << " clocks"; | ||
94 | } | ||
95 | |||
96 | } // namespace | ||