Merge bitcoin-core#566: Enable context creation in preallocated memory

0522caa Explain caller's obligations for preallocated memory (Tim Ruffing)
238305f Move _preallocated functions to separate header (Tim Ruffing)
695feb6 Export _preallocated functions (Tim Ruffing)
814cc78 Add tests for contexts in preallocated memory (Tim Ruffing)
ba12dd0 Check arguments of _preallocated functions (Tim Ruffing)
5feadde Support cloning a context into preallocated memory (Tim Ruffing)
c4fd5da Switch to a single malloc call (Tim Ruffing)
ef020de Add size constants for preallocated memory (Tim Ruffing)
1bf7c05 Prepare for manual memory management in preallocated memory (Tim Ruffing)

Pull request description:

  @apoelstra

  This builds on bitcoin-core#557.

  Manually managing memory is always a pain in the ass in some way. I tried to keep the pain manageable. I'm open to suggestions to make this less ugly or error-prone.

  to do:
   * tests
   * export functions

ACKs for commit 0522ca:

Tree-SHA512: 8ddb5b70219b6f095e780a9812d2387ab2a7f399803ce4101e27da504b479a61ebe08b6380568c7ba6f1e73d7d0b1f58a3c0a66fa0fdec7a64cd0740e156ce38

Author: gmaxwell

Makefile.am

@@ -8,6 +8,7 @@ else
 JNI_LIB =
 endif
 include_HEADERS = include/secp256k1.h
+include_HEADERS += include/secp256k1_preallocated.h
 noinst_HEADERS =
 noinst_HEADERS += src/scalar.h
 noinst_HEADERS += src/scalar_4x64.h

include/secp256k1.h

@@ -33,9 +33,10 @@ extern "C" {
  *  verification).
  *
  *  A constructed context can safely be used from multiple threads
- *  simultaneously, but API call that take a non-const pointer to a context
+ *  simultaneously, but API calls that take a non-const pointer to a context
  *  need exclusive access to it. In particular this is the case for
- *  secp256k1_context_destroy and secp256k1_context_randomize.
+ *  secp256k1_context_destroy, secp256k1_context_preallocated_destroy,
+ *  and secp256k1_context_randomize.
  *
  *  Regarding randomization, either do it once at creation time (in which case
  *  you do not need any locking for the other calls), or use a read-write lock.
@@ -163,7 +164,8 @@ typedef int (*secp256k1_nonce_function)(
 #define SECP256K1_FLAGS_BIT_CONTEXT_SIGN (1 << 9)
 #define SECP256K1_FLAGS_BIT_COMPRESSION (1 << 8)
 
-/** Flags to pass to secp256k1_context_create. */
+/** Flags to pass to secp256k1_context_create, secp256k1_context_preallocated_size, and
+ *  secp256k1_context_preallocated_create. */
 #define SECP256K1_CONTEXT_VERIFY (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_VERIFY)
 #define SECP256K1_CONTEXT_SIGN (SECP256K1_FLAGS_TYPE_CONTEXT | SECP256K1_FLAGS_BIT_CONTEXT_SIGN)
 #define SECP256K1_CONTEXT_NONE (SECP256K1_FLAGS_TYPE_CONTEXT)
@@ -186,7 +188,11 @@ typedef int (*secp256k1_nonce_function)(
  */
 SECP256K1_API extern const secp256k1_context *secp256k1_context_no_precomp;
 
-/** Create a secp256k1 context object.
+/** Create a secp256k1 context object (in dynamically allocated memory).
+ *
+ *  This function uses malloc to allocate memory. It is guaranteed that malloc is
+ *  called at most once for every call of this function. If you need to avoid dynamic
+ *  memory allocation entirely, see the functions in secp256k1_preallocated.h.
  *
  *  Returns: a newly created context object.
  *  In:      flags: which parts of the context to initialize.
@@ -197,7 +203,11 @@ SECP256K1_API secp256k1_context* secp256k1_context_create(
     unsigned int flags
 ) SECP256K1_WARN_UNUSED_RESULT;
 
-/** Copies a secp256k1 context object.
+/** Copy a secp256k1 context object (into dynamically allocated memory).
+ *
+ *  This function uses malloc to allocate memory. It is guaranteed that malloc is
+ *  called at most once for every call of this function. If you need to avoid dynamic
+ *  memory allocation entirely, see the functions in secp256k1_preallocated.h.
  *
  *  Returns: a newly created context object.
  *  Args:    ctx: an existing context to copy (cannot be NULL)
@@ -206,10 +216,18 @@ SECP256K1_API secp256k1_context* secp256k1_context_clone(
     const secp256k1_context* ctx
 ) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT;
 
-/** Destroy a secp256k1 context object.
+/** Destroy a secp256k1 context object (created in dynamically allocated memory).
  *
  *  The context pointer may not be used afterwards.
- *  Args:   ctx: an existing context to destroy (cannot be NULL)
+ *
+ *  The context to destroy must have been created using secp256k1_context_create
+ *  or secp256k1_context_clone. If the context has instead been created using
+ *  secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone, the
+ *  behaviour is undefined. In that case, secp256k1_context_preallocated_destroy must
+ *  be used instead.
+ *
+ *  Args:   ctx: an existing context to destroy, constructed using
+ *               secp256k1_context_create or secp256k1_context_clone
  */
 SECP256K1_API void secp256k1_context_destroy(
     secp256k1_context* ctx
@@ -636,7 +654,8 @@ SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul(
  * contexts not initialized for signing; then it will have no effect and return 1.
  *
  * You should call this after secp256k1_context_create or
- * secp256k1_context_clone, and may call this repeatedly afterwards.
+ * secp256k1_context_clone (and secp256k1_context_preallocated_create or
+ * secp256k1_context_clone, resp.), and you may call this repeatedly afterwards.
  */
 SECP256K1_API SECP256K1_WARN_UNUSED_RESULT int secp256k1_context_randomize(
     secp256k1_context* ctx,

include/secp256k1_preallocated.h

@@ -0,0 +1,128 @@
+#ifndef SECP256K1_PREALLOCATED_H
+#define SECP256K1_PREALLOCATED_H
+
+#include "secp256k1.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* The module provided by this header file is intended for settings in which it
+ * is not possible or desirable to rely on dynamic memory allocation. It provides
+ * functions for creating, cloning, and destroying secp256k1 context objects in a
+ * contiguous fixed-size block of memory provided by the caller.
+ *
+ * Context objects created by functions in this module can be used like contexts
+ * objects created by functions in secp256k1.h, i.e., they can be passed to any
+ * API function that excepts a context object (see secp256k1.h for details). The
+ * only exception is that context objects created by functions in this module
+ * must be destroyed using secp256k1_context_preallocated_destroy (in this
+ * module) instead of secp256k1_context_destroy (in secp256k1.h).
+ *
+ * It is guaranteed that functions in by this module will not call malloc or its
+ * friends realloc, calloc, and free.
+ */
+
+/** Determine the memory size of a secp256k1 context object to be created in
+ *  caller-provided memory.
+ *
+ *  The purpose of this function is to determine how much memory must be provided
+ *  to secp256k1_context_preallocated_create.
+ *
+ *  Returns: the required size of the caller-provided memory block
+ *  In:      flags:    which parts of the context to initialize.
+ */
+SECP256K1_API size_t secp256k1_context_preallocated_size(
+    unsigned int flags
+) SECP256K1_WARN_UNUSED_RESULT;
+
+/** Create a secp256k1 context object in caller-provided memory.
+ *
+ *  The caller must provide a pointer to a rewritable contiguous block of memory
+ *  of size at least secp256k1_context_preallocated_size(flags) bytes, suitably
+ *  aligned to hold an object of any type.
+ *
+ *  The block of memory is exclusively owned by the created context object during
+ *  the lifetime of this context object, which begins with the call to this
+ *  function and ends when a call to secp256k1_context_preallocated_destroy
+ *  (which destroys the context object again) returns. During the lifetime of the
+ *  context object, the caller is obligated not to access this block of memory,
+ *  i.e., the caller may not read or write the memory, e.g., by copying the memory
+ *  contents to a different location or trying to create a second context object
+ *  in the memory. In simpler words, the prealloc pointer (or any pointer derived
+ *  from it) should not be used during the lifetime of the context object.
+ *
+ *  Returns: a newly created context object.
+ *  In:      prealloc: a pointer to a rewritable contiguous block of memory of
+ *                     size at least secp256k1_context_preallocated_size(flags)
+ *                     bytes, as detailed above (cannot be NULL)
+ *           flags:    which parts of the context to initialize.
+ *
+ *  See also secp256k1_context_randomize (in secp256k1.h)
+ *  and secp256k1_context_preallocated_destroy.
+ */
+SECP256K1_API secp256k1_context* secp256k1_context_preallocated_create(
+    void* prealloc,
+    unsigned int flags
+) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT;
+
+/** Determine the memory size of a secp256k1 context object to be copied into
+ *  caller-provided memory.
+ *
+ *  Returns: the required size of the caller-provided memory block.
+ *  In:      ctx: an existing context to copy (cannot be NULL)
+ */
+SECP256K1_API size_t secp256k1_context_preallocated_clone_size(
+    const secp256k1_context* ctx
+) SECP256K1_ARG_NONNULL(1) SECP256K1_WARN_UNUSED_RESULT;
+
+/** Copy a secp256k1 context object into caller-provided memory.
+ *
+ *  The caller must provide a pointer to a rewritable contiguous block of memory
+ *  of size at least secp256k1_context_preallocated_size(flags) bytes, suitably
+ *  aligned to hold an object of any type.
+ *
+ *  The block of memory is exclusively owned by the created context object during
+ *  the lifetime of this context object, see the description of
+ *  secp256k1_context_preallocated_create for details.
+ *
+ *  Returns: a newly created context object.
+ *  Args:    ctx:      an existing context to copy (cannot be NULL)
+ *  In:      prealloc: a pointer to a rewritable contiguous block of memory of
+ *                     size at least secp256k1_context_preallocated_size(flags)
+ *                     bytes, as detailed above (cannot be NULL)
+ */
+SECP256K1_API secp256k1_context* secp256k1_context_preallocated_clone(
+    const secp256k1_context* ctx,
+    void* prealloc
+) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_WARN_UNUSED_RESULT;
+
+/** Destroy a secp256k1 context object that has been created in
+ *  caller-provided memory.
+ *
+ *  The context pointer may not be used afterwards.
+ *
+ *  The context to destroy must have been created using
+ *  secp256k1_context_preallocated_create or secp256k1_context_preallocated_clone.
+ *  If the context has instead been created using secp256k1_context_create or
+ *  secp256k1_context_clone, the behaviour is undefined. In that case,
+ *  secp256k1_context_destroy must be used instead.
+ *
+ *  If required, it is the responsibility of the caller to deallocate the block
+ *  of memory properly after this function returns, e.g., by calling free on the
+ *  preallocated pointer given to secp256k1_context_preallocated_create or
+ *  secp256k1_context_preallocated_clone.
+ *
+ *  Args:   ctx: an existing context to destroy, constructed using
+ *               secp256k1_context_preallocated_create or
+ *               secp256k1_context_preallocated_clone (cannot be NULL)
+ */
+SECP256K1_API void secp256k1_context_preallocated_destroy(
+    secp256k1_context* ctx
+);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* SECP256K1_PREALLOCATED_H */

src/ecmult.h

@@ -20,10 +20,10 @@ typedef struct {
 #endif
 } secp256k1_ecmult_context;
 
+static const size_t SECP256K1_ECMULT_CONTEXT_PREALLOCATED_SIZE;
 static void secp256k1_ecmult_context_init(secp256k1_ecmult_context *ctx);
-static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, const secp256k1_callback *cb);
-static void secp256k1_ecmult_context_clone(secp256k1_ecmult_context *dst,
-                                           const secp256k1_ecmult_context *src, const secp256k1_callback *cb);
+static void secp256k1_ecmult_context_build(secp256k1_ecmult_context *ctx, void **prealloc);
+static void secp256k1_ecmult_context_finalize_memcpy(secp256k1_ecmult_context *dst, const secp256k1_ecmult_context *src);
 static void secp256k1_ecmult_context_clear(secp256k1_ecmult_context *ctx);
 static int secp256k1_ecmult_context_is_built(const secp256k1_ecmult_context *ctx);
 

src/ecmult_gen.h

@@ -28,10 +28,10 @@ typedef struct {
     secp256k1_gej initial;
 } secp256k1_ecmult_gen_context;
 
+static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE;
 static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context* ctx);
-static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx, const secp256k1_callback* cb);
-static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst,
-                                               const secp256k1_ecmult_gen_context* src, const secp256k1_callback* cb);
+static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context* ctx, void **prealloc);
+static void secp256k1_ecmult_gen_context_finalize_memcpy(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context* src);
 static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context* ctx);
 static int secp256k1_ecmult_gen_context_is_built(const secp256k1_ecmult_gen_context* ctx);
 

src/ecmult_gen_impl.h

@@ -7,30 +7,40 @@
 #ifndef SECP256K1_ECMULT_GEN_IMPL_H
 #define SECP256K1_ECMULT_GEN_IMPL_H
 
+#include "util.h"
 #include "scalar.h"
 #include "group.h"
 #include "ecmult_gen.h"
 #include "hash_impl.h"
 #ifdef USE_ECMULT_STATIC_PRECOMPUTATION
 #include "ecmult_static_context.h"
 #endif
+
+#ifndef USE_ECMULT_STATIC_PRECOMPUTATION
+    static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE = ROUND_TO_ALIGN(sizeof(*((secp256k1_ecmult_gen_context*) NULL)->prec));
+#else
+    static const size_t SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE = 0;
+#endif
+
 static void secp256k1_ecmult_gen_context_init(secp256k1_ecmult_gen_context *ctx) {
     ctx->prec = NULL;
 }
 
-static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx, const secp256k1_callback* cb) {
+static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx, void **prealloc) {
 #ifndef USE_ECMULT_STATIC_PRECOMPUTATION
     secp256k1_ge prec[1024];
     secp256k1_gej gj;
     secp256k1_gej nums_gej;
     int i, j;
+    size_t const prealloc_size = SECP256K1_ECMULT_GEN_CONTEXT_PREALLOCATED_SIZE;
+    void* const base = *prealloc;
 #endif
 
     if (ctx->prec != NULL) {
         return;
     }
 #ifndef USE_ECMULT_STATIC_PRECOMPUTATION
-    ctx->prec = (secp256k1_ge_storage (*)[64][16])checked_malloc(cb, sizeof(*ctx->prec));
+    ctx->prec = (secp256k1_ge_storage (*)[64][16])manual_alloc(prealloc, prealloc_size, base, prealloc_size);
 
     /* get the generator */
     secp256k1_gej_set_ge(&gj, &secp256k1_ge_const_g);
@@ -85,7 +95,7 @@ static void secp256k1_ecmult_gen_context_build(secp256k1_ecmult_gen_context *ctx
         }
     }
 #else
-    (void)cb;
+    (void)prealloc;
     ctx->prec = (secp256k1_ge_storage (*)[64][16])secp256k1_ecmult_static_context;
 #endif
     secp256k1_ecmult_gen_blind(ctx, NULL);
@@ -95,27 +105,18 @@ static int secp256k1_ecmult_gen_context_is_built(const secp256k1_ecmult_gen_cont
     return ctx->prec != NULL;
 }
 
-static void secp256k1_ecmult_gen_context_clone(secp256k1_ecmult_gen_context *dst,
-                                               const secp256k1_ecmult_gen_context *src, const secp256k1_callback* cb) {
-    if (src->prec == NULL) {
-        dst->prec = NULL;
-    } else {
+static void secp256k1_ecmult_gen_context_finalize_memcpy(secp256k1_ecmult_gen_context *dst, const secp256k1_ecmult_gen_context *src) {
 #ifndef USE_ECMULT_STATIC_PRECOMPUTATION
-        dst->prec = (secp256k1_ge_storage (*)[64][16])checked_malloc(cb, sizeof(*dst->prec));
-        memcpy(dst->prec, src->prec, sizeof(*dst->prec));
+    if (src->prec != NULL) {
+        /* We cast to void* first to suppress a -Wcast-align warning. */
+        dst->prec = (secp256k1_ge_storage (*)[64][16])(void*)((unsigned char*)dst + ((unsigned char*)src->prec - (unsigned char*)src));
+    }
 #else
-        (void)cb;
-        dst->prec = src->prec;
+    (void)dst, (void)src;
 #endif
-        dst->initial = src->initial;
-        dst->blind = src->blind;
-    }
 }
 
 static void secp256k1_ecmult_gen_context_clear(secp256k1_ecmult_gen_context *ctx) {
-#ifndef USE_ECMULT_STATIC_PRECOMPUTATION
-    free(ctx->prec);
-#endif
     secp256k1_scalar_clear(&ctx->blind);
     secp256k1_gej_clear(&ctx->initial);
     ctx->prec = NULL;