Merge bitcoin-core/secp256k1#1881: tests: Fix GCC 17 snapshot warning

9d75769dec tests: Fix GCC 17 snapshot warning (Tim Ruffing)

Pull request description:

  Passing a non-malloc pointer to free() would be UB. In this case, the
  free() line is never actually reached (and GCC 17 fails to prove this)
  in a correct implementation of secp256k1_scratch_space_destroy(), but
  the test shouldn't rely on the correctness of the tested function.

  Alternative to #1880. I think this is cleaner
   - it doesn't recreate the scratch space in the middle of some other tests
   - it has an obvious matching (malloc, free) pair
   - it additionally checks that only `magic` is accessed

ACKs for top commit:
  hebasto:
    ACK 9d75769dec, I have reviewed the code and it looks OK.
  theStack:
    re-ACK 9d75769dec

Tree-SHA512: eb1a73c04e1996aeecabaa804e6a4fb1f5e6dfba4b9a775b6dd33d147be17185115a5aa9b7d9bf50260c20849e159d1b57f9ee41901d3d691dec25f2643b7cd2
This commit is contained in:
merge-script
2026-06-25 16:44:49 +02:00

View File

@@ -372,7 +372,6 @@ static void run_scratch_tests(void) {
size_t checkpoint;
size_t checkpoint_2;
secp256k1_scratch_space *scratch;
secp256k1_scratch_space local_scratch;
/* Test public API */
scratch = secp256k1_scratch_space_create(CTX, 1000);
@@ -412,16 +411,7 @@ static void run_scratch_tests(void) {
CHECK_ERROR_VOID(CTX, secp256k1_scratch_apply_checkpoint(&CTX->error_callback, scratch, checkpoint_2)); /* checkpoint_2 is after checkpoint */
CHECK_ERROR_VOID(CTX, secp256k1_scratch_apply_checkpoint(&CTX->error_callback, scratch, (size_t) -1)); /* this is just wildly invalid */
/* try to use badly initialized scratch space */
secp256k1_scratch_space_destroy(CTX, scratch);
memset(&local_scratch, 0, sizeof(local_scratch));
scratch = &local_scratch;
CHECK_ERROR(CTX, secp256k1_scratch_max_allocation(&CTX->error_callback, scratch, 0));
CHECK_ERROR(CTX, secp256k1_scratch_alloc(&CTX->error_callback, scratch, 500));
CHECK_ERROR_VOID(CTX, secp256k1_scratch_space_destroy(CTX, scratch));
/* Test that large integers do not wrap around in a bad way */
scratch = secp256k1_scratch_space_create(CTX, 1000);
/* Try max allocation with a large number of objects. Only makes sense if
* ALIGNMENT is greater than 1 because otherwise the objects take no extra
* space. */
@@ -436,6 +426,21 @@ static void run_scratch_tests(void) {
secp256k1_scratch_space_destroy(CTX, NULL); /* no-op */
}
/* try to use badly initialized scratch space */
static void run_invalid_scratch_space_tests(void) {
secp256k1_scratch_space* scratch = checked_malloc(&CTX->error_callback, sizeof(*scratch));
size_t magic_size = sizeof(scratch->magic);
memset(scratch, 0, sizeof(*scratch));
/* catch accesses beyond the magic */
SECP256K1_CHECKMEM_UNDEFINE((unsigned char*)scratch + magic_size, sizeof(*scratch) - magic_size);
CHECK_ERROR(CTX, secp256k1_scratch_max_allocation(&CTX->error_callback, scratch, 0));
CHECK_ERROR(CTX, secp256k1_scratch_alloc(&CTX->error_callback, scratch, 500));
CHECK_ERROR_VOID(CTX, secp256k1_scratch_space_destroy(CTX, scratch));
free(scratch);
}
/* A compression function that does nothing */
static void invalid_sha256_compression(uint32_t *s, const unsigned char *msg, size_t rounds) {
(void)s; (void)msg; (void)rounds;
@@ -7935,6 +7940,7 @@ static const struct tf_test_entry tests_general[] = {
CASE(all_static_context_tests),
CASE(deprecated_context_flags_test),
CASE(scratch_tests),
CASE(invalid_scratch_space_tests),
CASE(plug_sha256_compression_tests),
CASE(sha256_multi_block_compression_tests),
};