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.

.travis.yml

@@ -24,6 +24,7 @@ matrix:
         - libopencv-contrib-dev # For av_test.
         - libopus-dev           # For toxav.
         - libsndfile1-dev       # For av_test.
+        - libgtest-dev          # For unit tests.
         - libvpx-dev            # For toxav.
         - opam                  # For apidsl and Frama-C.
         - portaudio19-dev       # For av_test.

CMakeLists.txt

@@ -429,7 +429,49 @@ install_module(toxcore DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/tox)
 
 ################################################################################
 #
-# :: Automated regression tests
+# :: Unit tests: no networking, just pure function calls.
+#
+################################################################################
+
+# Compile the GTest library.
+#
+if(EXISTS "/usr/src/gtest/src/gtest-all.cc")
+  add_library(gtest
+    /usr/src/gtest/src/gtest-all.cc
+    /usr/src/gtest/src/gtest_main.cc)
+  target_include_directories(gtest PRIVATE /usr/src/gtest)
+  check_cxx_compiler_flag("-w" HAVE_CXX_W QUIET)
+  check_cxx_compiler_flag("-Wno-global-constructors" HAVE_CXX_W_NO_GLOBAL_CONSTRUCTORS QUIET)
+  check_cxx_compiler_flag("-Wno-zero-as-null-pointer-constant" HAVE_CXX_W_NO_ZERO_AS_NULL_POINTER_CONSTANT QUIET)
+  if(HAVE_CXX_W)
+    set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-w")
+  endif()
+  set(HAVE_GTEST TRUE)
+endif()
+
+function(unit_test subdir target)
+  if(HAVE_GTEST)
+    add_executable(unit_${target}_test ${subdir}/${target}_test.cpp)
+    target_link_modules(unit_${target}_test ${toxcore_SUBLIBS} gtest)
+    set(gtest_CFLAGS "")
+    if(HAVE_CXX_W_NO_GLOBAL_CONSTRUCTORS)
+      set(gtest_CFLAGS "${gtest_CFLAGS} -Wno-global-constructors")
+    endif()
+    if(HAVE_CXX_W_NO_ZERO_AS_NULL_POINTER_CONSTANT)
+      set(gtest_CFLAGS "${gtest_CFLAGS} -Wno-zero-as-null-pointer-constant")
+    endif()
+    set_target_properties(unit_${target}_test PROPERTIES COMPILE_FLAGS "${gtest_CFLAGS}")
+    add_test(NAME ${target} COMMAND ${CROSSCOMPILING_EMULATOR} unit_${target}_test)
+  endif()
+endfunction()
+
+# The actual unit tests follow.
+#
+unit_test(toxcore crypto_core)
+
+################################################################################
+#
+# :: Automated regression tests: create a tox network and run integration tests
 #
 ################################################################################
 

auto_tests/conference_test.c

@@ -20,7 +20,7 @@
 
 #include "helpers.h"
 
-#define NUM_GROUP_TOX 8
+#define NUM_GROUP_TOX 5
 
 static void handle_self_connection_status(Tox *tox, TOX_CONNECTION connection_status, void *user_data)
 {

auto_tests/crypto_test.c

@@ -339,83 +339,6 @@ START_TEST(test_memzero)
 }
 END_TEST
 
-#define CRYPTO_TEST_MEMCMP_SIZE 1024*1024  // 1MiB
-#define CRYPTO_TEST_MEMCMP_COUNT 2000
-#define CRYPTO_TEST_MEMCMP_EPS 10
-
-static int cmp(const void *a, const void *b)
-{
-    const clock_t *first = (const clock_t *) a;
-    const clock_t *second = (const clock_t *) b;
-
-    if (*first < *second) {
-        return -1;
-    }
-
-    if (*first > *second) {
-        return 1;
-    }
-
-    return 0;
-}
-
-static clock_t memcmp_time(void *a, void *b, size_t len)
-{
-    clock_t start = clock();
-    crypto_memcmp(a, b, len);
-    return clock() - start;
-}
-
-static clock_t memcmp_median(void *a, void *b, size_t len)
-{
-    size_t i;
-    clock_t results[CRYPTO_TEST_MEMCMP_COUNT];
-
-    for (i = 0; i < CRYPTO_TEST_MEMCMP_COUNT; i++) {
-        results[i] = memcmp_time(a, b, len);
-    }
-
-    qsort(results, CRYPTO_TEST_MEMCMP_COUNT, sizeof(*results), cmp);
-    return results[CRYPTO_TEST_MEMCMP_COUNT / 2];
-}
-
-START_TEST(test_memcmp)
-{
-    uint8_t *src = (uint8_t *)malloc(CRYPTO_TEST_MEMCMP_SIZE);
-    rand_bytes(src, CRYPTO_TEST_MEMCMP_SIZE);
-
-    uint8_t *same = (uint8_t *)malloc(CRYPTO_TEST_MEMCMP_SIZE);
-    memcpy(same, src, CRYPTO_TEST_MEMCMP_SIZE);
-
-    uint8_t *not_same = (uint8_t *)malloc(CRYPTO_TEST_MEMCMP_SIZE);
-    rand_bytes(not_same, CRYPTO_TEST_MEMCMP_SIZE);
-
-    printf("timing memcmp (equal arrays)\n");
-    clock_t same_median = memcmp_median(src, same, CRYPTO_TEST_MEMCMP_SIZE);
-    printf("timing memcmp (non-equal arrays)\n");
-    clock_t not_same_median = memcmp_median(src, not_same, CRYPTO_TEST_MEMCMP_SIZE);
-    printf("comparing times\n");
-
-    free(not_same);
-    free(same);
-    free(src);
-
-    clock_t delta;
-
-    if (same_median > not_same_median) {
-        delta = same_median - not_same_median;
-    } else {
-        delta = not_same_median - same_median;
-    }
-
-    ck_assert_msg(delta < CRYPTO_TEST_MEMCMP_EPS,
-                  "Delta time is too long (%d >= %d)\n"
-                  "Time of the same data comparation: %d\n"
-                  "Time of the different data comparation: %d",
-                  delta, CRYPTO_TEST_MEMCMP_EPS, same_median, not_same_median);
-}
-END_TEST
-
 static Suite *crypto_suite(void)
 {
     Suite *s = suite_create("Crypto");
@@ -427,7 +350,6 @@ static Suite *crypto_suite(void)
     DEFTESTCASE(large_data_symmetric);
     DEFTESTCASE_SLOW(increment_nonce, 20);
     DEFTESTCASE(memzero);
-    DEFTESTCASE_SLOW(memcmp, 30);
 
     return s;
 }

circle.yml

@@ -137,4 +137,4 @@ compile:
 
 test:
   override:
-  - 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
+  - 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

other/travis/env.sh

@@ -7,6 +7,7 @@ export LD_LIBRARY_PATH=$CACHE_DIR/lib
 export PKG_CONFIG_PATH=$CACHE_DIR/lib/pkgconfig
 export ASTYLE=$CACHE_DIR/astyle/build/gcc/bin/astyle
 export CFLAGS="-O3 -DTRAVIS_ENV=1"
+export CXXFLAGS="-O3 -DTRAVIS_ENV=1"
 export CMAKE_EXTRA_FLAGS="-DERROR_ON_WARNING=ON"
 export MAKE=make
 

other/travis/toxcore-script

@@ -2,6 +2,7 @@
 
 # Enable test coverage recording.
 export CFLAGS="$CFLAGS -fprofile-arcs -ftest-coverage"
+export CXXFLAGS="$CXXFLAGS -fprofile-arcs -ftest-coverage"
 
 set_opt() {
   opts="$opts -D$1="`expr ON \& \( $i % 2 \) \| OFF`

toxcore/BUILD.bazel

@@ -28,6 +28,15 @@ cc_library(
     ],
 )
 
+cc_test(
+    name = "crypto_core_test",
+    srcs = ["crypto_core_test.cpp"],
+    deps = [
+        ":crypto_core",
+        "@gtest",
+    ],
+)
+
 cc_library(
     name = "list",
     srcs = ["list.c"],

toxcore/crypto_core_test.cpp

@@ -0,0 +1,96 @@
+#include "crypto_core.h"
+
+#include <algorithm>
+
+#include <gtest/gtest.h>
+
+namespace
+{
+
+enum {
+    /**
+     * The size of the arrays to compare. This was chosen to take around 2000
+     * CPU clocks on x86_64.
+     */
+    CRYPTO_TEST_MEMCMP_SIZE = 1024 * 1024, // 1 MiB
+    /**
+     * The number of times we run memcmp in the test.
+     *
+     * We compute the median time taken to reduce error margins.
+     */
+    CRYPTO_TEST_MEMCMP_ITERATIONS = 500,
+    /**
+     * The margin of error (in clocks) we allow for this test.
+     *
+     * Should be within 0.5% of ~2000 CPU clocks. In reality, the code is much
+     * more precise and is usually within 1 CPU clock.
+     */
+    CRYPTO_TEST_MEMCMP_EPS = 10,
+};
+
+clock_t memcmp_time(void *a, void *b, size_t len)
+{
+    clock_t start = clock();
+    crypto_memcmp(a, b, len);
+    return clock() - start;
+}
+
+/**
+ * This function performs the actual timing. It interleaves comparison of
+ * equal and non-equal arrays to reduce the influence of external effects
+ * such as the machine being a little more busy 1 second later.
+ */
+void memcmp_median(void *src, void *same, void *not_same, size_t len,
+                   clock_t *same_median, clock_t *not_same_median)
+{
+    clock_t same_results[CRYPTO_TEST_MEMCMP_ITERATIONS];
+    clock_t not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS];
+
+    for (size_t i = 0; i < CRYPTO_TEST_MEMCMP_ITERATIONS; i++) {
+        same_results[i] = memcmp_time(src, same, len);
+        not_same_results[i] = memcmp_time(src, not_same, len);
+    }
+
+    std::sort(same_results, same_results + CRYPTO_TEST_MEMCMP_ITERATIONS);
+    *same_median = same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2];
+    std::sort(not_same_results, not_same_results + CRYPTO_TEST_MEMCMP_ITERATIONS);
+    *not_same_median = not_same_results[CRYPTO_TEST_MEMCMP_ITERATIONS / 2];
+}
+
+/**
+ * This test checks whether crypto_memcmp takes the same time for equal and
+ * non-equal chunks of memory.
+ */
+TEST(CryptoCore, MemcmpTimingIsDataIndependent)
+{
+    // A random piece of memory.
+    uint8_t *src = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE];
+    random_bytes(src, CRYPTO_TEST_MEMCMP_SIZE);
+
+    // A separate piece of memory containing the same data.
+    uint8_t *same = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE];
+    memcpy(same, src, CRYPTO_TEST_MEMCMP_SIZE);
+
+    // Another piece of memory containing different data.
+    uint8_t *not_same = new uint8_t[CRYPTO_TEST_MEMCMP_SIZE];
+    random_bytes(not_same, CRYPTO_TEST_MEMCMP_SIZE);
+
+    clock_t same_median;
+    clock_t not_same_median;
+    memcmp_median(src, same, not_same, CRYPTO_TEST_MEMCMP_SIZE, &same_median, &not_same_median);
+
+    delete[] not_same;
+    delete[] same;
+    delete[] src;
+
+    clock_t const delta = same_median > not_same_median
+                          ? same_median - not_same_median
+                          : not_same_median - same_median;
+
+    EXPECT_LT(delta, CRYPTO_TEST_MEMCMP_EPS)
+            << "Delta time is too long (" << delta << " >= " << CRYPTO_TEST_MEMCMP_EPS << ")\n"
+            << "Time of the same data comparation: " << same_median << " clocks\n"
+            << "Time of the different data comparation: " << not_same_median << " clocks";
+}
+
+}  // namespace