scratch: replace frames with "checkpoint" system

src/ecmult_impl.h

@@ -653,15 +653,13 @@ static int secp256k1_ecmult_strauss_batch(const secp256k1_callback* error_callba
     secp256k1_scalar* scalars;
     struct secp256k1_strauss_state state;
     size_t i;
+    const size_t scratch_checkpoint = secp256k1_scratch_checkpoint(error_callback, scratch);
 
     secp256k1_gej_set_infinity(r);
     if (inp_g_sc == NULL && n_points == 0) {
         return 1;
     }
 
-    if (!secp256k1_scratch_allocate_frame(error_callback, scratch, secp256k1_strauss_scratch_size(n_points), STRAUSS_SCRATCH_OBJECTS)) {
-        return 0;
-    }
     points = (secp256k1_gej*)secp256k1_scratch_alloc(error_callback, scratch, n_points * sizeof(secp256k1_gej));
     scalars = (secp256k1_scalar*)secp256k1_scratch_alloc(error_callback, scratch, n_points * sizeof(secp256k1_scalar));
     state.prej = (secp256k1_gej*)secp256k1_scratch_alloc(error_callback, scratch, n_points * ECMULT_TABLE_SIZE(WINDOW_A) * sizeof(secp256k1_gej));
@@ -674,16 +672,21 @@ static int secp256k1_ecmult_strauss_batch(const secp256k1_callback* error_callba
 #endif
     state.ps = (struct secp256k1_strauss_point_state*)secp256k1_scratch_alloc(error_callback, scratch, n_points * sizeof(struct secp256k1_strauss_point_state));
 
+    if (points == NULL || scalars == NULL || state.prej == NULL || state.zr == NULL || state.pre_a == NULL) {
+        secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
+        return 0;
+    }
+
     for (i = 0; i < n_points; i++) {
         secp256k1_ge point;
         if (!cb(&scalars[i], &point, i+cb_offset, cbdata)) {
-            secp256k1_scratch_deallocate_frame(error_callback, scratch);
+            secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
             return 0;
         }
         secp256k1_gej_set_ge(&points[i], &point);
     }
     secp256k1_ecmult_strauss_wnaf(ctx, &state, r, n_points, points, scalars, inp_g_sc);
-    secp256k1_scratch_deallocate_frame(error_callback, scratch);
+    secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
     return 1;
 }
 
@@ -986,6 +989,7 @@ static size_t secp256k1_pippenger_scratch_size(size_t n_points, int bucket_windo
 }
 
 static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_callback, const secp256k1_ecmult_context *ctx, secp256k1_scratch *scratch, secp256k1_gej *r, const secp256k1_scalar *inp_g_sc, secp256k1_ecmult_multi_callback cb, void *cbdata, size_t n_points, size_t cb_offset) {
+    const size_t scratch_checkpoint = secp256k1_scratch_checkpoint(error_callback, scratch);
     /* Use 2(n+1) with the endomorphism, n+1 without, when calculating batch
      * sizes. The reason for +1 is that we add the G scalar to the list of
      * other scalars. */
@@ -1010,15 +1014,21 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call
     }
 
     bucket_window = secp256k1_pippenger_bucket_window(n_points);
-    if (!secp256k1_scratch_allocate_frame(error_callback, scratch, secp256k1_pippenger_scratch_size(n_points, bucket_window), PIPPENGER_SCRATCH_OBJECTS)) {
-        return 0;
-    }
     points = (secp256k1_ge *) secp256k1_scratch_alloc(error_callback, scratch, entries * sizeof(*points));
     scalars = (secp256k1_scalar *) secp256k1_scratch_alloc(error_callback, scratch, entries * sizeof(*scalars));
     state_space = (struct secp256k1_pippenger_state *) secp256k1_scratch_alloc(error_callback, scratch, sizeof(*state_space));
+    if (points == NULL || scalars == NULL || state_space == NULL) {
+        secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
+        return 0;
+    }
+
     state_space->ps = (struct secp256k1_pippenger_point_state *) secp256k1_scratch_alloc(error_callback, scratch, entries * sizeof(*state_space->ps));
     state_space->wnaf_na = (int *) secp256k1_scratch_alloc(error_callback, scratch, entries*(WNAF_SIZE(bucket_window+1)) * sizeof(int));
     buckets = (secp256k1_gej *) secp256k1_scratch_alloc(error_callback, scratch, (1<<bucket_window) * sizeof(*buckets));
+    if (state_space->ps == NULL || state_space->wnaf_na == NULL || buckets == NULL) {
+        secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
+        return 0;
+    }
 
     if (inp_g_sc != NULL) {
         scalars[0] = *inp_g_sc;
@@ -1032,7 +1042,7 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call
 
     while (point_idx < n_points) {
         if (!cb(&scalars[idx], &points[idx], point_idx + cb_offset, cbdata)) {
-            secp256k1_scratch_deallocate_frame(error_callback, scratch);
+            secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
             return 0;
         }
         idx++;
@@ -1056,7 +1066,7 @@ static int secp256k1_ecmult_pippenger_batch(const secp256k1_callback* error_call
     for(i = 0; i < 1<<bucket_window; i++) {
         secp256k1_gej_clear(&buckets[i]);
     }
-    secp256k1_scratch_deallocate_frame(error_callback, scratch);
+    secp256k1_scratch_apply_checkpoint(error_callback, scratch, scratch_checkpoint);
     return 1;
 }
 

src/scratch.h

@@ -7,17 +7,17 @@
 #ifndef _SECP256K1_SCRATCH_
 #define _SECP256K1_SCRATCH_
 
-#define SECP256K1_SCRATCH_MAX_FRAMES	5
-
 /* The typedef is used internally; the struct name is used in the public API
  * (where it is exposed as a different typedef) */
 typedef struct secp256k1_scratch_space_struct {
+    /** guard against interpreting this object as other types */
     unsigned char magic[8];
+    /** actual allocated data */
     void *data;
-    void *current_frame;
+    /** amount that has been allocated (i.e. `data + offset` is the next
-    size_t offset[SECP256K1_SCRATCH_MAX_FRAMES];
+     *  available pointer)  */
-    size_t frame_size[SECP256K1_SCRATCH_MAX_FRAMES];
+    size_t alloc_size;
-    size_t frame;
+    /** maximum size available to allocate */
     size_t max_size;
 } secp256k1_scratch;
 
@@ -25,11 +25,13 @@ static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* err
 
 static void secp256k1_scratch_destroy(const secp256k1_callback* error_callback, secp256k1_scratch* scratch);
 
-/** Attempts to allocate a new stack frame with `n` available bytes. Returns 1 on success, 0 on failure */
+/** Returns an opaque object used to "checkpoint" a scratch space. Used
-static int secp256k1_scratch_allocate_frame(const secp256k1_callback* error_callback, secp256k1_scratch* scratch, size_t n, size_t objects);
+ *  with `secp256k1_scratch_apply_checkpoint` to undo allocations. */
+static size_t secp256k1_scratch_checkpoint(const secp256k1_callback* error_callback, const secp256k1_scratch* scratch);
 
-/** Deallocates a stack frame */
+/** Applies a check point received from `secp256k1_scratch_checkpoint`,
-static void secp256k1_scratch_deallocate_frame(const secp256k1_callback* error_callback, secp256k1_scratch* scratch);
+ *  undoing all allocations since that point. */
+static void secp256k1_scratch_apply_checkpoint(const secp256k1_callback* error_callback, secp256k1_scratch* scratch, size_t checkpoint);
 
 /** Returns the maximum allocation the scratch space will allow */
 static size_t secp256k1_scratch_max_allocation(const secp256k1_callback* error_callback, const secp256k1_scratch* scratch, size_t n_objects);

src/scratch_impl.h

@@ -25,7 +25,7 @@ static secp256k1_scratch* secp256k1_scratch_create(const secp256k1_callback* err
 
 static void secp256k1_scratch_destroy(const secp256k1_callback* error_callback, secp256k1_scratch* scratch) {
     if (scratch != NULL) {
-        VERIFY_CHECK(scratch->frame == 0);
+        VERIFY_CHECK(scratch->alloc_size == 0); /* all checkpoints should be applied */
         if (memcmp(scratch->magic, "scratch", 8) != 0) {
             secp256k1_callback_call(error_callback, "invalid scratch space");
             return;
@@ -35,58 +35,39 @@ static void secp256k1_scratch_destroy(const secp256k1_callback* error_callback,
     }
 }
 
-static size_t secp256k1_scratch_max_allocation(const secp256k1_callback* error_callback, const secp256k1_scratch* scratch, size_t objects) {
+static size_t secp256k1_scratch_checkpoint(const secp256k1_callback* error_callback, const secp256k1_scratch* scratch) {
-    size_t i = 0;
-    size_t allocated = 0;
     if (memcmp(scratch->magic, "scratch", 8) != 0) {
         secp256k1_callback_call(error_callback, "invalid scratch space");
         return 0;
     }
-    for (i = 0; i < scratch->frame; i++) {
+    return scratch->alloc_size;
-        allocated += scratch->frame_size[i];
-    }
-    if (scratch->max_size - allocated <= objects * (ALIGNMENT - 1)) {
-        return 0;
-    }
-    return scratch->max_size - allocated - objects *  (ALIGNMENT - 1);
 }
 
-static int secp256k1_scratch_allocate_frame(const secp256k1_callback* error_callback, secp256k1_scratch* scratch, size_t n, size_t objects) {
+static void secp256k1_scratch_apply_checkpoint(const secp256k1_callback* error_callback, secp256k1_scratch* scratch, size_t checkpoint) {
-    VERIFY_CHECK(scratch->frame < SECP256K1_SCRATCH_MAX_FRAMES);
-
-    if (memcmp(scratch->magic, "scratch", 8) != 0) {
-        secp256k1_callback_call(error_callback, "invalid scratch space");
-        return 0;
-    }
-
-    if (n <= secp256k1_scratch_max_allocation(error_callback, scratch, objects)) {
-        n += objects * (ALIGNMENT - 1);
-        scratch->current_frame = scratch->data;
-        scratch->data = (void *) ((char *) scratch->data + n);
-        scratch->frame_size[scratch->frame] = n;
-        scratch->offset[scratch->frame] = 0;
-        scratch->frame++;
-        return 1;
-    } else {
-        return 0;
-    }
-}
-
-static void secp256k1_scratch_deallocate_frame(const secp256k1_callback* error_callback, secp256k1_scratch* scratch) {
     if (memcmp(scratch->magic, "scratch", 8) != 0) {
         secp256k1_callback_call(error_callback, "invalid scratch space");
         return;
     }
+    if (checkpoint > scratch->alloc_size) {
+        secp256k1_callback_call(error_callback, "invalid checkpoint");
+        return;
+    }
+    scratch->alloc_size = checkpoint;
+}
 
-    VERIFY_CHECK(scratch->frame > 0);
+static size_t secp256k1_scratch_max_allocation(const secp256k1_callback* error_callback, const secp256k1_scratch* scratch, size_t objects) {
-
+    if (memcmp(scratch->magic, "scratch", 8) != 0) {
-    scratch->frame--;
+        secp256k1_callback_call(error_callback, "invalid scratch space");
-    scratch->data = (void *) ((char *) scratch->data - scratch->frame_size[scratch->frame]);
+        return 0;
+    }
+    if (scratch->max_size - scratch->alloc_size <= objects * (ALIGNMENT - 1)) {
+        return 0;
+    }
+    return scratch->max_size - scratch->alloc_size - objects * (ALIGNMENT - 1);
 }
 
 static void *secp256k1_scratch_alloc(const secp256k1_callback* error_callback, secp256k1_scratch* scratch, size_t size) {
     void *ret;
-    size_t frame = scratch->frame - 1;
     size = ROUND_TO_ALIGN(size);
 
     if (memcmp(scratch->magic, "scratch", 8) != 0) {
@@ -94,12 +75,12 @@ static void *secp256k1_scratch_alloc(const secp256k1_callback* error_callback, s
         return NULL;
     }
 
-    if (scratch->frame == 0 || size + scratch->offset[frame] > scratch->frame_size[frame]) {
+    if (size > scratch->max_size - scratch->alloc_size) {
         return NULL;
     }
-    ret = (void *) ((char *) scratch->current_frame + scratch->offset[frame]);
+    ret = (void *) ((char *) scratch->data + scratch->alloc_size);
     memset(ret, 0, size);
-    scratch->offset[frame] += size;
+    scratch->alloc_size += size;
 
     return ret;
 }

src/tests.c

@@ -333,7 +333,11 @@ void run_context_tests(int use_prealloc) {
 }
 
 void run_scratch_tests(void) {
+    const size_t adj_alloc = ((500 + ALIGNMENT - 1) / ALIGNMENT) * ALIGNMENT;
+
     int32_t ecount = 0;
+    size_t checkpoint;
+    size_t checkpoint_2;
     secp256k1_context *none = secp256k1_context_create(SECP256K1_CONTEXT_NONE);
     secp256k1_scratch_space *scratch;
     secp256k1_scratch_space local_scratch;
@@ -348,43 +352,54 @@ void run_scratch_tests(void) {
 
     /* Test internal API */
     CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0) == 1000);
-    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 1) < 1000);
+    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 1) == 1000 - (ALIGNMENT - 1));
+    CHECK(scratch->alloc_size == 0);
+    CHECK(scratch->alloc_size % ALIGNMENT == 0);
 
-    /* Allocating 500 bytes with no frame fails */
+    /* Allocating 500 bytes succeeds */
-    CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) == NULL);
+    checkpoint = secp256k1_scratch_checkpoint(&none->error_callback, scratch);
-    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0) == 1000);
-
-    /* ...but pushing a new stack frame does affect the max allocation */
-    CHECK(secp256k1_scratch_allocate_frame(&none->error_callback, scratch, 500, 1) == 1);
-    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 1) < 500); /* 500 - (ALIGNMENT - 1) */
     CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) != NULL);
+    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0) == 1000 - adj_alloc);
+    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 1) == 1000 - adj_alloc - (ALIGNMENT - 1));
+    CHECK(scratch->alloc_size != 0);
+    CHECK(scratch->alloc_size % ALIGNMENT == 0);
+
+    /* Allocating another 500 bytes fails */
     CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) == NULL);
+    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0) == 1000 - adj_alloc);
+    CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 1) == 1000 - adj_alloc - (ALIGNMENT - 1));
+    CHECK(scratch->alloc_size != 0);
+    CHECK(scratch->alloc_size % ALIGNMENT == 0);
 
-    CHECK(secp256k1_scratch_allocate_frame(&none->error_callback, scratch, 500, 1) == 0);
+    /* ...but it succeeds once we apply the checkpoint to undo it */
-
+    secp256k1_scratch_apply_checkpoint(&none->error_callback, scratch, checkpoint);
-    /* ...and this effect is undone by popping the frame */
+    CHECK(scratch->alloc_size == 0);
-    secp256k1_scratch_deallocate_frame(&none->error_callback, scratch);
     CHECK(secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0) == 1000);
-    CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) == NULL);
+    CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) != NULL);
+    CHECK(scratch->alloc_size != 0);
 
-    /* cleanup */
+    /* try to apply a bad checkpoint */
-    secp256k1_scratch_space_destroy(none, scratch);
+    checkpoint_2 = secp256k1_scratch_checkpoint(&none->error_callback, scratch);
+    secp256k1_scratch_apply_checkpoint(&none->error_callback, scratch, checkpoint);
     CHECK(ecount == 0);
+    secp256k1_scratch_apply_checkpoint(&none->error_callback, scratch, checkpoint_2); /* checkpoint_2 is after checkpoint */
+    CHECK(ecount == 1);
+    secp256k1_scratch_apply_checkpoint(&none->error_callback, scratch, (size_t) -1); /* this is just wildly invalid */
+    CHECK(ecount == 2);
 
     /* try to use badly initialized scratch space */
+    secp256k1_scratch_space_destroy(none, scratch);
     memset(&local_scratch, 0, sizeof(local_scratch));
     scratch = &local_scratch;
     CHECK(!secp256k1_scratch_max_allocation(&none->error_callback, scratch, 0));
-    CHECK(ecount == 1);
-    CHECK(secp256k1_scratch_allocate_frame(&none->error_callback, scratch, 500, 1) == 0);
-    CHECK(ecount == 2);
-    CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) == NULL);
     CHECK(ecount == 3);
-    secp256k1_scratch_deallocate_frame(&none->error_callback, scratch);
+    CHECK(secp256k1_scratch_alloc(&none->error_callback, scratch, 500) == NULL);
     CHECK(ecount == 4);
     secp256k1_scratch_space_destroy(none, scratch);
     CHECK(ecount == 5);
 
+    /* cleanup */
+    secp256k1_scratch_space_destroy(none, NULL); /* no-op */
     secp256k1_context_destroy(none);
 }
 
@@ -2946,16 +2961,26 @@ void test_ecmult_multi_pippenger_max_points(void) {
     int bucket_window = 0;
 
     for(; scratch_size < max_size; scratch_size+=256) {
+        size_t i;
+        size_t total_alloc;
+        size_t checkpoint;
         scratch = secp256k1_scratch_create(&ctx->error_callback, scratch_size);
         CHECK(scratch != NULL);
+        checkpoint = secp256k1_scratch_checkpoint(&ctx->error_callback, scratch);
         n_points_supported = secp256k1_pippenger_max_points(&ctx->error_callback, scratch);
         if (n_points_supported == 0) {
             secp256k1_scratch_destroy(&ctx->error_callback, scratch);
             continue;
         }
         bucket_window = secp256k1_pippenger_bucket_window(n_points_supported);
-        CHECK(secp256k1_scratch_allocate_frame(&ctx->error_callback, scratch, secp256k1_pippenger_scratch_size(n_points_supported, bucket_window), PIPPENGER_SCRATCH_OBJECTS));
+        /* allocate `total_alloc` bytes over `PIPPENGER_SCRATCH_OBJECTS` many allocations */
-        secp256k1_scratch_deallocate_frame(&ctx->error_callback, scratch);
+        total_alloc = secp256k1_pippenger_scratch_size(n_points_supported, bucket_window);
+        for (i = 0; i < PIPPENGER_SCRATCH_OBJECTS - 1; i++) {
+            CHECK(secp256k1_scratch_alloc(&ctx->error_callback, scratch, 1));
+            total_alloc--;
+        }
+        CHECK(secp256k1_scratch_alloc(&ctx->error_callback, scratch, total_alloc));
+        secp256k1_scratch_apply_checkpoint(&ctx->error_callback, scratch, checkpoint);
         secp256k1_scratch_destroy(&ctx->error_callback, scratch);
     }
     CHECK(bucket_window == PIPPENGER_MAX_BUCKET_WINDOW);