Skip to content

Commit

Permalink
Merge bitcoin#798: Check assumptions on integer implementation at com…
Browse files Browse the repository at this point in the history
…pile time

7c06899 Compile-time check assumptions on integer types (Pieter Wuille)
02b6c87 Add support for (signed) __int128 (Pieter Wuille)

Pull request description:

  A compile-time check is implemented in a new `src/assumptions.h` which verifies several aspects that are implementation-defined in C:
  * size of bytes
  * conversion between unsigned and (negative) signed types
  * right-shifts of negative signed types.

ACKs for top commit:
  gmaxwell:
    ACK 7c06899
  real-or-random:
    ACK 7c06899 code review and tested

Tree-SHA512: 3903251973681c88d64d4af0f6cb40fde11eb436804c5b6202c3715b78b1a48bcb287f601b394fd0b503437e3832ba011885e992fe65098b33edc430d9b1f67d
  • Loading branch information
real-or-random committed Aug 16, 2020
2 parents 979961c + 7c06899 commit 670cdd3
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ noinst_HEADERS += src/field_5x52.h
noinst_HEADERS += src/field_5x52_impl.h
noinst_HEADERS += src/field_5x52_int128_impl.h
noinst_HEADERS += src/field_5x52_asm_impl.h
noinst_HEADERS += src/assumptions.h
noinst_HEADERS += src/util.h
noinst_HEADERS += src/scratch.h
noinst_HEADERS += src/scratch_impl.h
Expand Down
74 changes: 74 additions & 0 deletions src/assumptions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/**********************************************************************
* Copyright (c) 2020 Pieter Wuille *
* Distributed under the MIT software license, see the accompanying *
* file COPYING or http://www.opensource.org/licenses/mit-license.php.*
**********************************************************************/

#ifndef SECP256K1_ASSUMPTIONS_H
#define SECP256K1_ASSUMPTIONS_H

#include "util.h"

/* This library, like most software, relies on a number of compiler implementation defined (but not undefined)
behaviours. Although the behaviours we require are essentially universal we test them specifically here to
reduce the odds of experiencing an unwelcome surprise.
*/

struct secp256k1_assumption_checker {
/* This uses a trick to implement a static assertion in C89: a type with an array of negative size is not
allowed. */
int dummy_array[(
/* Bytes are 8 bits. */
CHAR_BIT == 8 &&

/* Conversions from unsigned to signed outside of the bounds of the signed type are
implementation-defined. Verify that they function as reinterpreting the lower
bits of the input in two's complement notation. Do this for conversions:
- from uint(N)_t to int(N)_t with negative result
- from uint(2N)_t to int(N)_t with negative result
- from int(2N)_t to int(N)_t with negative result
- from int(2N)_t to int(N)_t with positive result */

/* To int8_t. */
((int8_t)(uint8_t)0xAB == (int8_t)-(int8_t)0x55) &&
((int8_t)(uint16_t)0xABCD == (int8_t)-(int8_t)0x33) &&
((int8_t)(int16_t)(uint16_t)0xCDEF == (int8_t)(uint8_t)0xEF) &&
((int8_t)(int16_t)(uint16_t)0x9234 == (int8_t)(uint8_t)0x34) &&

/* To int16_t. */
((int16_t)(uint16_t)0xBCDE == (int16_t)-(int16_t)0x4322) &&
((int16_t)(uint32_t)0xA1B2C3D4 == (int16_t)-(int16_t)0x3C2C) &&
((int16_t)(int32_t)(uint32_t)0xC1D2E3F4 == (int16_t)(uint16_t)0xE3F4) &&
((int16_t)(int32_t)(uint32_t)0x92345678 == (int16_t)(uint16_t)0x5678) &&

/* To int32_t. */
((int32_t)(uint32_t)0xB2C3D4E5 == (int32_t)-(int32_t)0x4D3C2B1B) &&
((int32_t)(uint64_t)0xA123B456C789D012ULL == (int32_t)-(int32_t)0x38762FEE) &&
((int32_t)(int64_t)(uint64_t)0xC1D2E3F4A5B6C7D8ULL == (int32_t)(uint32_t)0xA5B6C7D8) &&
((int32_t)(int64_t)(uint64_t)0xABCDEF0123456789ULL == (int32_t)(uint32_t)0x23456789) &&

/* To int64_t. */
((int64_t)(uint64_t)0xB123C456D789E012ULL == (int64_t)-(int64_t)0x4EDC3BA928761FEEULL) &&
#if defined(SECP256K1_WIDEMUL_INT128)
((int64_t)(((uint128_t)0xA1234567B8901234ULL << 64) + 0xC5678901D2345678ULL) == (int64_t)-(int64_t)0x3A9876FE2DCBA988ULL) &&
(((int64_t)(int128_t)(((uint128_t)0xB1C2D3E4F5A6B7C8ULL << 64) + 0xD9E0F1A2B3C4D5E6ULL)) == (int64_t)(uint64_t)0xD9E0F1A2B3C4D5E6ULL) &&
(((int64_t)(int128_t)(((uint128_t)0xABCDEF0123456789ULL << 64) + 0x0123456789ABCDEFULL)) == (int64_t)(uint64_t)0x0123456789ABCDEFULL) &&

/* To int128_t. */
((int128_t)(((uint128_t)0xB1234567C8901234ULL << 64) + 0xD5678901E2345678ULL) == (int128_t)(-(int128_t)0x8E1648B3F50E80DCULL * 0x8E1648B3F50E80DDULL + 0x5EA688D5482F9464ULL)) &&
#endif

/* Right shift on negative signed values is implementation defined. Verify that it
acts as a right shift in two's complement with sign extension (i.e duplicating
the top bit into newly added bits). */
((((int8_t)0xE8) >> 2) == (int8_t)(uint8_t)0xFA) &&
((((int16_t)0xE9AC) >> 4) == (int16_t)(uint16_t)0xFE9A) &&
((((int32_t)0x937C918A) >> 9) == (int32_t)(uint32_t)0xFFC9BE48) &&
((((int64_t)0xA8B72231DF9CF4B9ULL) >> 19) == (int64_t)(uint64_t)0xFFFFF516E4463BF3ULL) &&
#if defined(SECP256K1_WIDEMUL_INT128)
((((int128_t)(((uint128_t)0xCD833A65684A0DBCULL << 64) + 0xB349312F71EA7637ULL)) >> 39) == (int128_t)(((uint128_t)0xFFFFFFFFFF9B0674ULL << 64) + 0xCAD0941B79669262ULL)) &&
#endif
1) * 2 - 1];
};

#endif /* SECP256K1_ASSUMPTIONS_H */
1 change: 1 addition & 0 deletions src/bench_internal.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "include/secp256k1.h"

#include "assumptions.h"
#include "util.h"
#include "hash_impl.h"
#include "num_impl.h"
Expand Down
1 change: 1 addition & 0 deletions src/gen_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "basic-config.h"

#include "include/secp256k1.h"
#include "assumptions.h"
#include "util.h"
#include "field_impl.h"
#include "scalar_impl.h"
Expand Down
1 change: 1 addition & 0 deletions src/secp256k1.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "include/secp256k1.h"
#include "include/secp256k1_preallocated.h"

#include "assumptions.h"
#include "util.h"
#include "num_impl.h"
#include "field_impl.h"
Expand Down
1 change: 1 addition & 0 deletions src/tests_exhaustive.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#endif

#include "include/secp256k1.h"
#include "assumptions.h"
#include "group.h"
#include "secp256k1.c"
#include "testrand_impl.h"
Expand Down
1 change: 1 addition & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ static SECP256K1_INLINE void secp256k1_int_cmov(int *r, const int *a, int flag)
#endif
#if defined(SECP256K1_WIDEMUL_INT128)
SECP256K1_GNUC_EXT typedef unsigned __int128 uint128_t;
SECP256K1_GNUC_EXT typedef __int128 int128_t;
#endif

#endif /* SECP256K1_UTIL_H */
1 change: 1 addition & 0 deletions src/valgrind_ctime_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <valgrind/memcheck.h>
#include "include/secp256k1.h"
#include "assumptions.h"
#include "util.h"

#if ENABLE_MODULE_ECDH
Expand Down

0 comments on commit 670cdd3

Please sign in to comment.