Replace locking by atomic operations

`s_warray_init()` and `s_warray_free()` are not safe and MUST NOT be called
from multiple threads.

This also removes `MP_WARRAY_NUM`, since automatic initialization will not
be safe for more than one thread.

Signed-off-by: Steffen Jaeckel <[email protected]>

Author: sjaeckel

.github/workflows/main.yml

@@ -72,9 +72,7 @@ jobs:
 
           # Build with small stack-size
           - { BUILDOPTIONS: '--with-cc=gcc --with-m32 --with-m64 --cflags=-DMP_SMALL_STACK_SIZE',                                SANITIZER: '',  COMPILE_DEBUG: '0', COMPILE_LTO: '0', CONV_WARNINGS: '',        OTHERDEPS: 'gcc-multilib' }
-          - { BUILDOPTIONS: '--with-cc=gcc --with-m32 --with-m64 --cflags=-DMP_SMALL_STACK_SIZE --cflags=-DMP_NO_LOCKING',       SANITIZER: '',  COMPILE_DEBUG: '0', COMPILE_LTO: '0', CONV_WARNINGS: '',        OTHERDEPS: 'gcc-multilib' }
           - { BUILDOPTIONS: '--with-cc=clang-10 --with-m32 --with-m64 --cflags=-DMP_SMALL_STACK_SIZE',                           SANITIZER: '1', COMPILE_DEBUG: '0', COMPILE_LTO: '0', CONV_WARNINGS: '',        OTHERDEPS: 'clang-10 llvm-10 gcc-multilib' }
-          - { BUILDOPTIONS: '--with-cc=clang-10 --with-m32 --with-m64 --cflags=-DMP_SMALL_STACK_SIZE --cflags=-DMP_TEST_LOCKING', SANITIZER: '1', COMPILE_DEBUG: '0', COMPILE_LTO: '0', CONV_WARNINGS: '',       OTHERDEPS: 'clang-10 llvm-10 gcc-multilib' }
 
           # Test "autotuning", the automatic evaluation and setting of the Toom-Cook cut-offs.
           #- env: SANITIZER=1 BUILDOPTIONS='--with-cc=gcc-5 --cflags=-DMP_16BIT --limit-valgrind --make-option=tune'

demo/test.c

@@ -2451,21 +2451,6 @@ static int test_mp_pack_unpack(void)
    return EXIT_FAILURE;
 }
 
-
-#ifdef MP_TEST_LOCKING
-#ifdef MP_NO_LOCKING
-#error "Can't test locking when locking is disabled"
-#endif
-static mp_lock lock_ctx;
-static int noop_lock_unlock(void *ctx)
-{
-   EXPECT(ctx == &lock_ctx);
-   return 0;
-LBL_ERR:
-   return -1;
-}
-#endif
-
 #ifndef LTM_TEST_DYNAMIC
 #define ONLY_PUBLIC_API_C
 #endif
@@ -2540,14 +2525,6 @@ static int unit_tests(int argc, char **argv)
    unsigned long i, ok, fail, nop;
    uint64_t t;
    int j;
-#ifdef MP_TEST_LOCKING
-   lock_ctx.lock = noop_lock_unlock;
-   lock_ctx.unlock = noop_lock_unlock;
-   lock_ctx.ctx = &lock_ctx;
-
-   if (mp_warray_init(MP_WARRAY_NUM, true, &lock_ctx) != MP_OKAY)
-      return EXIT_FAILURE;
-#endif
    ok = fail = nop = 0;
 
    t = (uint64_t)time(NULL);

doc/bn.tex

@@ -357,14 +357,10 @@ \subsection{Small-Stack option}
 The library can be compiled with the symbol \texttt{MP\_SMALL\_STACK\_SIZE} defined, which results in
 the temporary \texttt{MP\_WARRAY}-sized stack buffers being put on the heap.
 This comes with one problem, namely: formerly promised thread-safety isn't given anymore.
-Therefore if the Small-Stack option is enabled while doing multi threading, the provided locking
-mechanism shall be used.
-For some use cases it can be desired to use the Small-Stack option, but there are no threads and
-therefore we provide the possibility to disable locking by defining the symbol \texttt{MP\_NO\_LOCKING}.
+Therefore if the Small-Stack option is enabled while doing multi threading, one shall always initialize
+the library by calling \texttt{mp\_warray\_init()} once with the correct number of threads.
 
-In case one already knows how many threads must be supported, the symbol \texttt{MP\_WARRAY\_NUM} can
-be useful. It can be pre-defined at compile time to the number of heap buffers created on automatic
-initialisation. C.f. \ref{ch:SMALL_STACK_API} for the dynamic API and further details.
+C.f. \ref{ch:SMALL_STACK_API} for the API description and further details.
 
 \section{Purpose of LibTomMath}
 Unlike	GNU MP (GMP) Library, LIP, OpenSSL or various other commercial kits (Miracl), LibTomMath
@@ -443,8 +439,10 @@ \section{Building Programs}
 In order to use LibTomMath you must include ``tommath.h'' and link against the appropriate library
 file (typically
 libtommath.a).	There is no library initialization required and the entire library is thread safe
-if it is used in its default configuration. Locking is recommended if the small-stack option
-is enabled and multiple threads are used, c.f. \ref{ch:SMALL_STACK_INTRO} resp. \ref{ch:SMALL_STACK_API}
+if it is used in its default configuration. The small-stack option makes use of atomic operations
+to maintain its internal state and therefore does not require locking, but it MUST be initialized
+if used from multiple threads. For further information see \ref{ch:SMALL_STACK_INTRO} resp.
+\ref{ch:SMALL_STACK_API}.
 
 \section{Return Codes}
 There are five possible return codes a function may return.
@@ -839,27 +837,12 @@ \section{Small-Stack option}
 
 \index{mp\_warray\_init}
 \begin{alltt}
-mp_err mp_warray_init(size_t n_alloc, bool preallocate, mp_lock *lock);
+mp_err mp_warray_init(size_t n_alloc, bool preallocate);
 \end{alltt}
 
 The flag \texttt{preallocate} controls whether the internal buffers --
 \texttt{n\_alloc} buffers of size \texttt{MP\_WARRAY} -- will be allocated when
 \texttt{mp\_warray\_init()} is called, or whether they will be allocated when required.
-The \texttt{mp\_lock} struct looks as follows and shall be used to protect the
-internal structure when using the library in a multi-threaded application.
-
-\index{mp\_lock}
-\begin{alltt}
-typedef struct {
-   int (*lock)(void *ctx);
-   int (*unlock)(void *ctx);
-   void *ctx;
-} mp_lock;
-\end{alltt}
-
-The \texttt{mp\_lock.lock} resp. \texttt{mp\_lock.unlock} functions will be called before resp.
-after modifying the internal struct.
-The \texttt{mp\_lock.ctx} element will be passed to those functions.
 
 To free the internally allocated memory the following function shall be called.
 

mp_warray_free.c

@@ -10,7 +10,6 @@ static int s_warray_free(void)
 {
    int ret = 0;
    size_t n;
-   S_MP_WARRAY_LOCK();
    for (n = 0; n < s_mp_warray.allocated; ++n) {
       if (s_mp_warray.l_used[n].warray) {
          ret = -2;
@@ -23,7 +22,6 @@ static int s_warray_free(void)
    }
    s_mp_warray_free(s_mp_warray.usable);
 ERR_OUT:
-   S_MP_WARRAY_UNLOCK();
    return ret;
 }
 

mp_warray_init.c

@@ -3,22 +3,13 @@
 /* LibTomMath, multiple-precision integer library -- Tom St Denis */
 /* SPDX-License-Identifier: Unlicense */
 
-static mp_err s_warray_init(size_t n_alloc, bool preallocate, mp_lock *lock)
+static mp_err s_warray_init(size_t n_alloc, bool preallocate)
 {
    size_t n;
    if (s_mp_warray.l_free != NULL || s_mp_warray.l_used != NULL) {
       return MP_VAL;
    }
 
-   if (MP_HAS(MP_USE_LOCKING) && (lock != NULL)) {
-      if (lock->lock == NULL || lock->unlock == NULL)
-         return MP_VAL;
-      s_mp_warray.lock = *lock;
-      s_mp_warray.locking_enabled = true;
-   } else {
-      s_mp_zero_buf(&s_mp_warray.lock, sizeof(s_mp_warray.lock));
-   }
-
    s_mp_warray.l_free = MP_CALLOC(n_alloc, sizeof(*(s_mp_warray.l_free)));
    s_mp_warray.l_used = MP_CALLOC(n_alloc, sizeof(*(s_mp_warray.l_used)));
    if (s_mp_warray.l_free == NULL || s_mp_warray.l_used == NULL) {
@@ -46,9 +37,9 @@ static mp_err s_warray_init(size_t n_alloc, bool preallocate, mp_lock *lock)
    return MP_OKAY;
 }
 
-mp_err mp_warray_init(size_t n_alloc, bool preallocate, mp_lock *lock)
+mp_err mp_warray_init(size_t n_alloc, bool preallocate)
 {
-   if (MP_HAS(MP_SMALL_STACK_SIZE)) return s_warray_init(n_alloc, preallocate, lock);
+   if (MP_HAS(MP_SMALL_STACK_SIZE)) return s_warray_init(n_alloc, preallocate);
    return MP_ERR;
 }
 

s_mp_warray_get.c

@@ -7,26 +7,27 @@ void *s_mp_warray_get(void)
 {
    void *ret = NULL;
    size_t n;
-   S_MP_WARRAY_LOCK();
    if (s_mp_warray.usable == 0) {
-      if (mp_warray_init(MP_WARRAY_NUM, false, NULL) != MP_OKAY)
+      if (mp_warray_init(1, false) != MP_OKAY)
          return NULL;
    }
    for (n = 0; n < s_mp_warray.allocated; ++n) {
-      if (s_mp_warray.l_free[n].warray) {
-         s_mp_warray.l_used[n] = s_mp_warray.l_free[n];
-         s_mp_warray.l_free[n].warray = NULL;
-         ret = s_mp_warray.l_used[n].warray;
+      if (s_mp_warray.l_free[n].warray == NULL)
+         continue;
+      ret = s_mp_warray.l_free[n].warray;
+      if (MP_CMPEXCH(&s_mp_warray.l_free[n].warray, &ret, NULL)) {
+         s_mp_warray.l_used[n].warray = ret;
          goto LBL_OUT;
       }
    }
+   ret = NULL;
    if (s_mp_warray.allocated + 1 > s_mp_warray.usable)
       goto LBL_OUT;
    ret = MP_CALLOC(MP_WARRAY, sizeof(mp_word));
-   s_mp_warray.l_used[s_mp_warray.allocated++].warray = ret;
+   if (ret != NULL)
+      s_mp_warray.l_used[s_mp_warray.allocated++].warray = ret;
 
 LBL_OUT:
-   S_MP_WARRAY_UNLOCK();
    return ret;
 }
 

s_mp_warray_put.c

@@ -5,16 +5,14 @@
 
 void s_mp_warray_put(void *w)
 {
-   size_t n;
-   S_MP_WARRAY_LOCK();
-   for (n = 0; n < s_mp_warray.allocated; ++n) {
+   size_t n, allocated = s_mp_warray.allocated;
+   for (n = 0; n < allocated; ++n) {
       if (s_mp_warray.l_used[n].warray == w) {
-         s_mp_warray.l_free[n] = s_mp_warray.l_used[n];
          s_mp_warray.l_used[n].warray = NULL;
+         s_mp_warray.l_free[n].warray = w;
          break;
       }
    }
-   S_MP_WARRAY_UNLOCK();
 }
 
 #endif

tommath.h

@@ -78,25 +78,6 @@ typedef uint32_t             mp_digit;
 #define MP_MASK          ((((mp_digit)1)<<((mp_digit)MP_DIGIT_BIT))-((mp_digit)1))
 #define MP_DIGIT_MAX     MP_MASK
 
-/* In case the stack size has to be limited, use a WARRAY from the heap */
-#ifdef MP_SMALL_STACK_SIZE
-/* Per default we enable the locking mechanism.
- * Please disable by defining `MP_NO_LOCKING` if you really know what you do.
- */
-#ifndef MP_NO_LOCKING
-#define MP_USE_LOCKING
-#endif
-#endif /* MP_SMALL_STACK_SIZE */
-
-/* The user can define how many WARRAY instances are allocated,
- * usually this should equal the number of parallel threads that
- * use LTM functionality.
- * This has no effect if `MP_SMALL_STACK_SIZE` is not defined.
- */
-#ifndef MP_WARRAY_NUM
-#define MP_WARRAY_NUM 1
-#endif
-
 /* Primality generation flags */
 #define MP_PRIME_BBS      0x0001 /* BBS style prime */
 #define MP_PRIME_SAFE     0x0002 /* Safe prime (p-1)/2 == prime */
@@ -607,13 +588,7 @@ mp_err mp_fread(mp_int *a, int radix, FILE *stream) MP_WUR;
 mp_err mp_fwrite(const mp_int *a, int radix, FILE *stream) MP_WUR;
 #endif
 
-typedef struct {
-   int (*lock)(void *ctx);
-   int (*unlock)(void *ctx);
-   void *ctx;
-} mp_lock;
-
-mp_err mp_warray_init(size_t n_alloc, bool preallocate, mp_lock *lock);
+mp_err mp_warray_init(size_t n_alloc, bool preallocate);
 int mp_warray_free(void);
 
 #define mp_to_binary(M, S, N)  mp_to_radix((M), (S), (N), NULL, 2)

tommath_class.h

@@ -969,7 +969,6 @@
 
 #if defined(MP_WARRAY_INIT_C)
 #   define S_MP_WARRAY_FREE_C
-#   define S_MP_ZERO_BUF_C
 #endif
 
 #if defined(MP_XOR_C)

tommath_private.h

@@ -104,6 +104,10 @@ extern void *MP_CALLOC(size_t nmemb, size_t size);
 extern void MP_FREE(void *mem, size_t size);
 #endif
 
+#ifndef MP_CMPEXCH
+#define MP_CMPEXCH(ptr, expected, desired) __atomic_compare_exchange_n(ptr, expected, desired, true, __ATOMIC_ACQ_REL, __ATOMIC_ACQUIRE)
+#endif
+
 /* feature detection macro */
 #ifdef _MSC_VER
 /* Prevent false positive: not enough arguments for function-like macro invocation */
@@ -245,23 +249,12 @@ MP_PRIVATE mp_err s_mp_fp_log_d(const mp_int *a, mp_word *c) MP_WUR;
 #define MP_CHECK_WARRAY(name)
 #endif
 
-#ifdef MP_USE_LOCKING
-#define MP_USE_LOCKING_C
-#define S_MP_WARRAY_LOCK() do { if (s_mp_warray.locking_enabled) { s_mp_warray.lock.lock(s_mp_warray.lock.ctx); } } while(0)
-#define S_MP_WARRAY_UNLOCK() do { if (s_mp_warray.locking_enabled) { s_mp_warray.lock.unlock(s_mp_warray.lock.ctx); } } while(0)
-#else
-#define S_MP_WARRAY_LOCK()
-#define S_MP_WARRAY_UNLOCK()
-#endif
-
 struct warray {
    void *warray;
 };
 typedef struct {
    struct warray *l_free, *l_used;
    size_t allocated, usable;
-   bool locking_enabled;
-   mp_lock lock;
 } st_warray;
 
 extern MP_PRIVATE st_warray s_mp_warray;