From 8563713a4fbcd7d3b3134fcf248ff985837cc3e1 Mon Sep 17 00:00:00 2001 From: Gregory Maxwell Date: Wed, 12 Nov 2014 12:05:42 -0800 Subject: [PATCH] Add non-null and unused-result warnings for the external API. GCC (and clang) supports extensions to annotate functions so that their results must be used and so that their arguments can't be statically provable to be null. If a caller violates these requirements they get a warning, so this helps them write correct code. I deployed this in libopus a couple years ago with good success, and the implementation here is basically copied straight from that. One consideration is that the non-null annotation teaches the optimizer and will actually compile out runtime non-nullness checks as dead-code. Since this is usually not whats wanted, the non-null annotations are disabled when compiling the library itself. The commit also removes some dead inclusions of assert.h and introduces compatibility macros for restrict and inline in preparation for some portability improvements. --- include/secp256k1.h | 165 ++++++++++++++++++++++++++++++++--------- src/ecmult_gen_impl.h | 1 - src/ecmult_impl.h | 1 - src/field_10x26_impl.h | 1 - src/field_5x52_impl.h | 1 - src/num_gmp_impl.h | 1 - src/secp256k1.c | 3 +- 7 files changed, 131 insertions(+), 42 deletions(-) diff --git a/include/secp256k1.h b/include/secp256k1.h index 4a468e00b2f..932bf0279fc 100644 --- a/include/secp256k1.h +++ b/include/secp256k1.h @@ -1,13 +1,61 @@ #ifndef _SECP256K1_ -#define _SECP256K1_ +# define _SECP256K1_ -#ifdef __cplusplus +# ifdef __cplusplus extern "C" { -#endif +# endif + +# if !defined(SECP256K1_GNUC_PREREQ) +# if defined(__GNUC__)&&defined(__GNUC_MINOR__) +# define SECP256K1_GNUC_PREREQ(_maj,_min) \ + ((__GNUC__<<16)+__GNUC_MINOR__>=((_maj)<<16)+(_min)) +# else +# define SECP256K1_GNUC_PREREQ(_maj,_min) 0 +# endif +# endif + +# if (!defined(__STDC_VERSION__) || (__STDC_VERSION__ < 199901L) ) +# if SECP256K1_GNUC_PREREQ(3,0) +# define SECP256K1_RESTRICT __restrict__ +# elif (defined(_MSC_VER) && _MSC_VER >= 1400) +# define SECP256K1_RESTRICT __restrict +# else +# define SECP256K1_RESTRICT +# endif +# else +# define SECP256K1_RESTRICT restrict +# endif + +# if (!defined(__STDC_VERSION__) || (__STDC_VERSION__ < 199901L) ) +# if SECP256K1_GNUC_PREREQ(2,7) +# define SECP256K1_INLINE __inline__ +# elif (defined(_MSC_VER)) +# define SECP256K1_INLINE __inline +# else +# define SECP256K1_INLINE +# endif +# else +# define SECP256K1_INLINE inline +# endif + +/**Warning attributes + * NONNULL is not used if SECP256K1_BUILD is set to avoid the compiler optimizing out + * some paranoid null checks. */ +# if defined(__GNUC__) && SECP256K1_GNUC_PREREQ(3, 4) +# define SECP256K1_WARN_UNUSED_RESULT __attribute__ ((__warn_unused_result__)) +# else +# define SECP256K1_WARN_UNUSED_RESULT +# endif +# if !defined(SECP256K1_BUILD) && defined(__GNUC__) && SECP256K1_GNUC_PREREQ(3, 4) +# define SECP256K1_ARG_NONNULL(_x) __attribute__ ((__nonnull__(_x))) +# else +# define SECP256K1_ARG_NONNULL(_x) +# endif + /** Flags to pass to secp256k1_start. */ -#define SECP256K1_START_VERIFY (1 << 0) -#define SECP256K1_START_SIGN (1 << 1) +# define SECP256K1_START_VERIFY (1 << 0) +# define SECP256K1_START_SIGN (1 << 1) /** Initialize the library. This may take some time (10-100 ms). * You need to call this before calling any other function. @@ -34,9 +82,14 @@ void secp256k1_stop(void); * pubkeylen: the length of pubkey * Requires starting using SECP256K1_START_VERIFY. */ -int secp256k1_ecdsa_verify(const unsigned char *msg, int msglen, - const unsigned char *sig, int siglen, - const unsigned char *pubkey, int pubkeylen); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_verify( + const unsigned char *msg, + int msglen, + const unsigned char *sig, + int siglen, + const unsigned char *pubkey, + int pubkeylen +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(5); /** Create an ECDSA signature. * Returns: 1: signature created @@ -50,10 +103,14 @@ int secp256k1_ecdsa_verify(const unsigned char *msg, int msglen, * to contain the actual signature length (<=72). * Requires starting using SECP256K1_START_SIGN. */ -int secp256k1_ecdsa_sign(const unsigned char *msg, int msglen, - unsigned char *sig, int *siglen, - const unsigned char *seckey, - const unsigned char *nonce); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_sign( + const unsigned char *msg, + int msglen, + unsigned char *sig, + int *siglen, + const unsigned char *seckey, + const unsigned char *nonce +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5) SECP256K1_ARG_NONNULL(6); /** Create a compact ECDSA signature (64 byte + recovery id). * Returns: 1: signature created @@ -66,11 +123,14 @@ int secp256k1_ecdsa_sign(const unsigned char *msg, int msglen, * recid: pointer to an int, which will be updated to contain the recovery id (can be NULL) * Requires starting using SECP256K1_START_SIGN. */ -int secp256k1_ecdsa_sign_compact(const unsigned char *msg, int msglen, - unsigned char *sig64, - const unsigned char *seckey, - const unsigned char *nonce, - int *recid); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_sign_compact( + const unsigned char *msg, + int msglen, + unsigned char *sig64, + const unsigned char *seckey, + const unsigned char *nonce, + int *recid +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5); /** Recover an ECDSA public key from a compact signature. * Returns: 1: public key successfully recovered (which guarantees a correct signature). @@ -84,17 +144,22 @@ int secp256k1_ecdsa_sign_compact(const unsigned char *msg, int msglen, * pubkeylen: pointer to an int that will contain the pubkey length (cannot be NULL) * Requires starting using SECP256K1_START_VERIFY. */ -int secp256k1_ecdsa_recover_compact(const unsigned char *msg, int msglen, - const unsigned char *sig64, - unsigned char *pubkey, int *pubkeylen, - int compressed, int recid); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ecdsa_recover_compact( + const unsigned char *msg, + int msglen, + const unsigned char *sig64, + unsigned char *pubkey, + int *pubkeylen, + int compressed, + int recid +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3) SECP256K1_ARG_NONNULL(4) SECP256K1_ARG_NONNULL(5); /** Verify an ECDSA secret key. * Returns: 1: secret key is valid * 0: secret key is invalid * In: seckey: pointer to a 32-byte secret key (cannot be NULL) */ -int secp256k1_ec_seckey_verify(const unsigned char *seckey); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_seckey_verify(const unsigned char *seckey) SECP256K1_ARG_NONNULL(1); /** Just validate a public key. * Returns: 1: valid public key @@ -102,7 +167,7 @@ int secp256k1_ec_seckey_verify(const unsigned char *seckey); * In: pubkey: pointer to a 33-byte or 65-byte public key (cannot be NULL). * pubkeylen: length of pubkey */ -int secp256k1_ec_pubkey_verify(const unsigned char *pubkey, int pubkeylen); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_verify(const unsigned char *pubkey, int pubkeylen) SECP256K1_ARG_NONNULL(1); /** Compute the public key for a secret key. * In: compressed: whether the computed public key should be compressed @@ -115,7 +180,12 @@ int secp256k1_ec_pubkey_verify(const unsigned char *pubkey, int pubkeylen); * 0: secret was invalid, try again. * Requires starting using SECP256K1_START_SIGN. */ -int secp256k1_ec_pubkey_create(unsigned char *pubkey, int *pubkeylen, const unsigned char *seckey, int compressed); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_create( + unsigned char *pubkey, + int *pubkeylen, + const unsigned char *seckey, + int compressed +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Decompress a public key. * In/Out: pubkey: pointer to a 65-byte array to put the decompressed public key. @@ -125,35 +195,58 @@ int secp256k1_ec_pubkey_create(unsigned char *pubkey, int *pubkeylen, const unsi * Returns: 0 if the passed public key was invalid, 1 otherwise. If 1 is returned, the pubkey is replaced with its decompressed version. */ -int secp256k1_ec_pubkey_decompress(unsigned char *pubkey, int *pubkeylen); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_decompress( + unsigned char *pubkey, + int *pubkeylen +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); /** Export a private key in DER format. */ -int secp256k1_ec_privkey_export(const unsigned char *seckey, - unsigned char *privkey, int *privkeylen, - int compressed); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_export( + const unsigned char *seckey, + unsigned char *privkey, + int *privkeylen, + int compressed +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2) SECP256K1_ARG_NONNULL(3); /** Import a private key in DER format. */ -int secp256k1_ec_privkey_import(unsigned char *seckey, - const unsigned char *privkey, int privkeylen); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_import( + unsigned char *seckey, + const unsigned char *privkey, + int privkeylen +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); /** Tweak a private key by adding tweak to it. */ -int secp256k1_ec_privkey_tweak_add(unsigned char *seckey, const unsigned char *tweak); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_add( + unsigned char *seckey, + const unsigned char *tweak +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); /** Tweak a public key by adding tweak times the generator to it. * Requires starting with SECP256K1_START_VERIFY. */ -int secp256k1_ec_pubkey_tweak_add(unsigned char *pubkey, int pubkeylen, const unsigned char *tweak); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_add( + unsigned char *pubkey, + int pubkeylen, + const unsigned char *tweak +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3); /** Tweak a private key by multiplying it with tweak. */ -int secp256k1_ec_privkey_tweak_mul(unsigned char *seckey, const unsigned char *tweak); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_privkey_tweak_mul( + unsigned char *seckey, + const unsigned char *tweak +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(2); /** Tweak a public key by multiplying it with tweak. * Requires starting with SECP256K1_START_VERIFY. */ -int secp256k1_ec_pubkey_tweak_mul(unsigned char *pubkey, int pubkeylen, const unsigned char *tweak); +SECP256K1_WARN_UNUSED_RESULT int secp256k1_ec_pubkey_tweak_mul( + unsigned char *pubkey, + int pubkeylen, + const unsigned char *tweak +) SECP256K1_ARG_NONNULL(1) SECP256K1_ARG_NONNULL(3); -#ifdef __cplusplus +# ifdef __cplusplus } -#endif +# endif #endif diff --git a/src/ecmult_gen_impl.h b/src/ecmult_gen_impl.h index 3b568abc4e7..63c402b1a7b 100644 --- a/src/ecmult_gen_impl.h +++ b/src/ecmult_gen_impl.h @@ -5,7 +5,6 @@ #ifndef _SECP256K1_ECMULT_GEN_IMPL_H_ #define _SECP256K1_ECMULT_GEN_IMPL_H_ -#include #include "scalar.h" #include "group.h" #include "ecmult_gen.h" diff --git a/src/ecmult_impl.h b/src/ecmult_impl.h index c0e4b116b0a..37579741480 100644 --- a/src/ecmult_impl.h +++ b/src/ecmult_impl.h @@ -5,7 +5,6 @@ #ifndef _SECP256K1_ECMULT_IMPL_H_ #define _SECP256K1_ECMULT_IMPL_H_ -#include #include "num.h" #include "group.h" #include "ecmult.h" diff --git a/src/field_10x26_impl.h b/src/field_10x26_impl.h index a567437fedd..c082f249c9e 100644 --- a/src/field_10x26_impl.h +++ b/src/field_10x26_impl.h @@ -6,7 +6,6 @@ #define _SECP256K1_FIELD_REPR_IMPL_H_ #include -#include #include #include "util.h" #include "num.h" diff --git a/src/field_5x52_impl.h b/src/field_5x52_impl.h index 5afabae38d3..c527714ebd6 100644 --- a/src/field_5x52_impl.h +++ b/src/field_5x52_impl.h @@ -9,7 +9,6 @@ #include "libsecp256k1-config.h" #endif -#include #include #include "util.h" #include "num.h" diff --git a/src/num_gmp_impl.h b/src/num_gmp_impl.h index d8442ee03e0..e91f543a8b0 100644 --- a/src/num_gmp_impl.h +++ b/src/num_gmp_impl.h @@ -5,7 +5,6 @@ #ifndef _SECP256K1_NUM_REPR_IMPL_H_ #define _SECP256K1_NUM_REPR_IMPL_H_ -#include #include #include #include diff --git a/src/secp256k1.c b/src/secp256k1.c index 3e37648299a..f0173425f1f 100644 --- a/src/secp256k1.c +++ b/src/secp256k1.c @@ -2,9 +2,10 @@ // Distributed under the MIT/X11 software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#define SECP256K1_BUILD (1) + #include "include/secp256k1.h" -#include #include "util.h" #include "num_impl.h" #include "field_impl.h"