Skip to content

Commit 1880593

Browse files
committed
Check that VERIFY_CHECK is side-effect free
1 parent 5454534 commit 1880593

File tree

3 files changed

+83
-13
lines changed

3 files changed

+83
-13
lines changed

build-aux/m4/bitcoin_secp.m4

+31
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,37 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
99
AC_MSG_RESULT([$has_64bit_asm])
1010
])
1111

12+
AC_DEFUN([SECP_CHECK_SIDE_EFFECT_FREE],[
13+
AC_MSG_CHECKING(whether compiler detects side effect free statements)
14+
AC_LINK_IFELSE([AC_LANG_SOURCE([[
15+
#include <stdlib.h>
16+
static int my_memcmp(const void *s1, const void *s2, size_t n) {
17+
const unsigned char *p1 = s1, *p2 = s2;
18+
size_t i;
19+
for (i = 0; i < n; i++) {
20+
int diff = p1[i] - p2[i];
21+
if (diff != 0) {
22+
return diff;
23+
}
24+
}
25+
return 0;
26+
}
27+
28+
static int side_effect_free(int val, const int* ptr) {
29+
static const unsigned char foo[32] = {1,2,3};
30+
if (!my_memcmp(ptr, foo, val)) return 0;
31+
return (((val + 13) * ptr[0] ^ 7) * ptr[ptr[1]] / 11) * (int)ptr == 0;
32+
}
33+
34+
extern int should_not_survive;
35+
int main(int argc, char** argv) {
36+
(void)(should_not_survive || side_effect_free(argc, (const int*)argv));
37+
return 0;
38+
}
39+
]])],[has_check_side_effect_free=yes],[has_check_side_effect_free=no])
40+
AC_MSG_RESULT([$has_check_side_effect_free])
41+
])
42+
1243
dnl
1344
AC_DEFUN([SECP_OPENSSL_CHECK],[
1445
has_libcrypto=no

configure.ac

+27
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,11 @@ AC_ARG_ENABLE(external_default_callbacks,
155155
[use_external_default_callbacks=$enableval],
156156
[use_external_default_callbacks=no])
157157

158+
AC_ARG_ENABLE(side_effect_free_check,
159+
AS_HELP_STRING([--enable-side-effect-free-check],[enable checking assertions are side-effect free (yes/no/auto) [default=no]]),
160+
[use_side_effect_free_check=$enableval],
161+
[use_side_effect_free_check=no])
162+
158163
# Test-only override of the (autodetected by the C code) "widemul" setting.
159164
# Legal values are int64 (for [u]int64_t), int128 (for [unsigned] __int128), and auto (the default).
160165
AC_ARG_WITH([test-override-wide-multiply], [] ,[set_widemul=$withval], [set_widemul=auto])
@@ -210,6 +215,18 @@ else
210215
CFLAGS="-O2 $CFLAGS"
211216
fi
212217

218+
set_check_side_effect_free=no
219+
if test x"$use_side_effect_free_check" != x"no"; then
220+
SECP_CHECK_SIDE_EFFECT_FREE
221+
if test x"$has_check_side_effect_free" = x"yes"; then
222+
set_check_side_effect_free=yes
223+
else
224+
if test x"$use_side_effect_free_check" != x"auto"; then
225+
AC_MSG_ERROR([Side effect free checking requested but not compatible with compiler])
226+
fi
227+
fi
228+
fi
229+
213230
if test x"$req_asm" = x"auto"; then
214231
SECP_64BIT_ASM_CHECK
215232
if test x"$has_64bit_asm" = x"yes"; then
@@ -258,6 +275,16 @@ if test x"$use_external_asm" = x"yes"; then
258275
AC_DEFINE(USE_EXTERNAL_ASM, 1, [Define this symbol if an external (non-inline) assembly implementation is used])
259276
fi
260277

278+
case $set_check_side_effect_free in
279+
yes)
280+
AC_DEFINE(CHECK_SIDE_EFFECT_FREE, 1, [Define this symbol to enable checks for side-effect free statements])
281+
;;
282+
no)
283+
;;
284+
*)
285+
AC_MSG_ERROR([invalid selection for check_side_effect_free])
286+
;;
287+
esac
261288

262289
# Select wide multiplication implementation
263290
case $set_widemul in

src/util.h

+25-13
Original file line numberDiff line numberDiff line change
@@ -57,26 +57,38 @@ static SECP256K1_INLINE void secp256k1_callback_call(const secp256k1_callback *
5757
} while(0)
5858
#endif
5959

60-
/* VERIFY_CHECK is like assert(), but gated by -DVERIFY, and always
61-
* evaluated (even without -DVERIFY) to minimize differences between
62-
* the two modes if conditions with side effects are accidentally
63-
* included.
60+
#ifdef CHECK_SIDE_EFFECT_FREE
61+
/* When our compiler is capable to optimizing away references to variables
62+
* that control execution of a side-effect free statement, use this to
63+
* check that VERIFY_CHECK conditions are safe.
64+
*
65+
* Credit to: https://stackoverflow.com/a/35294344 */
66+
extern int secp256k1_not_supposed_to_survive;
67+
#define ASSERT_NO_SIDE_EFFECT(cond) do { (void)(secp256k1_not_supposed_to_survive || (cond)); } while(0)
68+
#else
69+
/* If we can't use that, just run it in an unexecuted branch, to make sure it is still
70+
* syntactically valid. */
71+
#define ASSERT_NO_SIDE_EFFECT(cond) do { if (0) { (void)(cond); } } while(0)
72+
#endif
73+
74+
/* VERIFY_CHECK is like assert(), but gated by -DVERIFY, and verified for
75+
* side effect-freeness under -DCHECK_SIDE_EFFECT_FREE.
6476
*
6577
* VERIFY_CHECK_ONLY is similar, but less safe: it has no effect outside
6678
* of -DVERIFY, and thus can be used with expensive conditions, or even
6779
* conditions that wouldn't compile outside -DVERIFY). */
6880
#if defined(COVERAGE)
69-
#define VERIFY_CHECK(check)
70-
#define VERIFY_CHECK_ONLY(check)
71-
#define VERIFY_SETUP(stmt)
81+
# define VERIFY_CHECK(check)
82+
# define VERIFY_CHECK_ONLY(check)
83+
# define VERIFY_SETUP(stmt)
7284
#elif defined(VERIFY)
73-
#define VERIFY_CHECK CHECK
74-
#define VERIFY_CHECK_ONLY CHECK
75-
#define VERIFY_SETUP(stmt) do { stmt; } while(0)
85+
# define VERIFY_CHECK(cond) do { ASSERT_NO_SIDE_EFFECT(cond); CHECK(cond); } while (0)
86+
# define VERIFY_CHECK_ONLY CHECK
87+
# define VERIFY_SETUP(stmt) do { stmt; } while(0)
7688
#else
77-
#define VERIFY_CHECK(cond) do { (void)(cond); } while(0)
78-
#define VERIFY_CHECK_ONLY(cond)
79-
#define VERIFY_SETUP(stmt)
89+
# define VERIFY_CHECK(cond) do { ASSERT_NO_SIDE_EFFECT(cond); } while (0)
90+
# define VERIFY_CHECK_ONLY(cond)
91+
# define VERIFY_SETUP(stmt)
8092
#endif
8193

8294
/* Define `VG_UNDEF` and `VG_CHECK` when VALGRIND is defined */

0 commit comments

Comments
 (0)