Make the dispatch tests opt-in.
The assembly dispatch tests currently assume NDEBUG is consistently
defined between C/C++ and assembly. While this is usually the case for
UNIX, CMake does not pass NDEBUG to NASM. This is giving gRPC some
difficulties in updating BoringSSL, so switch it to an opt-in
-DBORINGSSL_DISPATCH_TEST flag instead.
Update-Note: If you were copying NDEBUG over to assembly files, that's
no longer required (though it's harmless to leave it in). If you want to
run ImplDispatchTest.*, build both C/C++ and assembly with
-DBORINGSSL_DISPATCH_TEST in your debug builds. (Don't enable it in
release builds. It causes assembly to scribble in some globals.)
Change-Id: I9ab3371dc0f0a40b27b44ef93835e007a6346900
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/37764
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index d79d35f..3fe86bf 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -59,9 +59,15 @@
add_definitions(-DBORINGSSL_ALLOW_CXX_RUNTIME)
endif()
-if(CMAKE_BUILD_TYPE STREQUAL "Release")
- # Windows release builds don't set NDEBUG in NASM flags automatically.
- set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DNDEBUG")
+string(TOLOWER "${CMAKE_BUILD_TYPE}" CMAKE_BUILD_TYPE_LOWER)
+if(NOT FIPS)
+ if(CMAKE_BUILD_TYPE_LOWER STREQUAL "relwithassert" OR
+ NOT CMAKE_BUILD_TYPE_LOWER MATCHES "rel")
+ add_definitions(-DBORINGSSL_DISPATCH_TEST)
+ # CMake automatically connects include_directories to the NASM
+ # command-line, but not add_definitions.
+ set(CMAKE_ASM_NASM_FLAGS "${CMAKE_ASM_NASM_FLAGS} -DBORINGSSL_DISPATCH_TEST")
+ endif()
endif()
# Add a RelWithAsserts build configuration. It is the same as Release, except it
diff --git a/crypto/crypto.c b/crypto/crypto.c
index c86206a..297240e 100644
--- a/crypto/crypto.c
+++ b/crypto/crypto.c
@@ -60,7 +60,7 @@
// that tests the capability values will still skip the constructor but, so
// far, the init constructor function only sets the capability variables.
-#if !defined(NDEBUG) && !defined(BORINGSSL_FIPS)
+#if defined(BORINGSSL_DISPATCH_TEST)
// This value must be explicitly initialised to zero in order to work around a
// bug in libtool or the linker on OS X.
//
diff --git a/crypto/fipsmodule/aes/asm/aesni-x86.pl b/crypto/fipsmodule/aes/asm/aesni-x86.pl
index d57127a..b999e5a 100644
--- a/crypto/fipsmodule/aes/asm/aesni-x86.pl
+++ b/crypto/fipsmodule/aes/asm/aesni-x86.pl
@@ -84,7 +84,7 @@
&asm_init($ARGV[0]);
&external_label("OPENSSL_ia32cap_P");
-&preprocessor_ifndef("NDEBUG")
+&preprocessor_ifdef("BORINGSSL_DISPATCH_TEST")
&external_label("BORINGSSL_function_hit");
&preprocessor_endif();
&static_label("key_const");
diff --git a/crypto/fipsmodule/aes/asm/aesni-x86_64.pl b/crypto/fipsmodule/aes/asm/aesni-x86_64.pl
index 15f6805..f4d2ea5 100644
--- a/crypto/fipsmodule/aes/asm/aesni-x86_64.pl
+++ b/crypto/fipsmodule/aes/asm/aesni-x86_64.pl
@@ -275,12 +275,10 @@
.align 16
${PREFIX}_encrypt:
.cfi_startproc
-#ifndef NDEBUG
-#ifndef BORINGSSL_FIPS
+#ifdef BORINGSSL_DISPATCH_TEST
.extern BORINGSSL_function_hit
movb \$1,BORINGSSL_function_hit+1(%rip)
#endif
-#endif
movups ($inp),$inout0 # load input
mov 240($key),$rounds # key->rounds
___
@@ -1205,11 +1203,9 @@
.align 16
${PREFIX}_ctr32_encrypt_blocks:
.cfi_startproc
-#ifndef NDEBUG
-#ifndef BORINGSSL_FIPS
+#ifdef BORINGSSL_DISPATCH_TEST
movb \$1,BORINGSSL_function_hit(%rip)
#endif
-#endif
cmp \$1,$len
jne .Lctr32_bulk
@@ -4354,11 +4350,9 @@
${PREFIX}_set_encrypt_key:
__aesni_set_encrypt_key:
.cfi_startproc
-#ifndef NDEBUG
-#ifndef BORINGSSL_FIPS
+#ifdef BORINGSSL_DISPATCH_TEST
movb \$1,BORINGSSL_function_hit+3(%rip)
#endif
-#endif
.byte 0x48,0x83,0xEC,0x08 # sub rsp,8
.cfi_adjust_cfa_offset 8
mov \$-1,%rax
diff --git a/crypto/fipsmodule/aes/asm/vpaes-x86.pl b/crypto/fipsmodule/aes/asm/vpaes-x86.pl
index 2b40362..14a3105 100644
--- a/crypto/fipsmodule/aes/asm/vpaes-x86.pl
+++ b/crypto/fipsmodule/aes/asm/vpaes-x86.pl
@@ -69,7 +69,7 @@
my ($round, $base, $magic, $key, $const, $inp, $out)=
("eax", "ebx", "ecx", "edx","ebp", "esi","edi");
-&preprocessor_ifndef("NDEBUG")
+&preprocessor_ifdef("BORINGSSL_DISPATCH_TEST")
&external_label("BORINGSSL_function_hit");
&preprocessor_endif();
&static_label("_vpaes_consts");
diff --git a/crypto/fipsmodule/aes/asm/vpaes-x86_64.pl b/crypto/fipsmodule/aes/asm/vpaes-x86_64.pl
index 8cc6a1a..e761eb2 100644
--- a/crypto/fipsmodule/aes/asm/vpaes-x86_64.pl
+++ b/crypto/fipsmodule/aes/asm/vpaes-x86_64.pl
@@ -871,12 +871,10 @@
.align 16
${PREFIX}_set_encrypt_key:
.cfi_startproc
-#ifndef NDEBUG
-#ifndef BORINGSSL_FIPS
+#ifdef BORINGSSL_DISPATCH_TEST
.extern BORINGSSL_function_hit
movb \$1, BORINGSSL_function_hit+5(%rip)
#endif
-#endif
___
$code.=<<___ if ($win64);
@@ -983,12 +981,10 @@
.align 16
${PREFIX}_encrypt:
.cfi_startproc
-#ifndef NDEBUG
-#ifndef BORINGSSL_FIPS
+#ifdef BORINGSSL_DISPATCH_TEST
.extern BORINGSSL_function_hit
movb \$1, BORINGSSL_function_hit+4(%rip)
#endif
-#endif
___
$code.=<<___ if ($win64);
lea -0xb8(%rsp),%rsp
diff --git a/crypto/fipsmodule/modes/asm/aesni-gcm-x86_64.pl b/crypto/fipsmodule/modes/asm/aesni-gcm-x86_64.pl
index d3e3763..48a5560 100644
--- a/crypto/fipsmodule/modes/asm/aesni-gcm-x86_64.pl
+++ b/crypto/fipsmodule/modes/asm/aesni-gcm-x86_64.pl
@@ -664,12 +664,10 @@
.align 32
aesni_gcm_encrypt:
.cfi_startproc
-#ifndef NDEBUG
-#ifndef BORINGSSL_FIPS
+#ifdef BORINGSSL_DISPATCH_TEST
.extern BORINGSSL_function_hit
movb \$1,BORINGSSL_function_hit+2(%rip)
#endif
-#endif
xor $ret,$ret
# We call |_aesni_ctr32_6x| twice, each call consuming 96 bytes of
diff --git a/crypto/impl_dispatch_test.cc b/crypto/impl_dispatch_test.cc
index 2c3a613..8297e22 100644
--- a/crypto/impl_dispatch_test.cc
+++ b/crypto/impl_dispatch_test.cc
@@ -14,8 +14,7 @@
#include <openssl/base.h>
-#if !defined(NDEBUG) && !defined(BORINGSSL_FIPS) && \
- !defined(BORINGSSL_SHARED_LIBRARY)
+#if defined(BORINGSSL_DISPATCH_TEST) && !defined(BORINGSSL_SHARED_LIBRARY)
#include <functional>
#include <utility>
@@ -148,4 +147,4 @@
#endif // X86 || X86_64
-#endif // !NDEBUG && !FIPS && !SHARED_LIBRARY
+#endif // DISPATCH_TEST && !SHARED_LIBRARY
diff --git a/crypto/perlasm/x86asm.pl b/crypto/perlasm/x86asm.pl
index 3278188..24f0fc1 100644
--- a/crypto/perlasm/x86asm.pl
+++ b/crypto/perlasm/x86asm.pl
@@ -34,12 +34,12 @@
}
# record_function_hit(int) writes a byte with value one to the given offset of
-# |BORINGSSL_function_hit|, but only if NDEBUG is not defined. This is used in
-# impl_dispatch_test.cc to test whether the expected assembly functions are
-# triggered by high-level API calls.
+# |BORINGSSL_function_hit|, but only if BORINGSSL_DISPATCH_TEST is defined.
+# This is used in impl_dispatch_test.cc to test whether the expected assembly
+# functions are triggered by high-level API calls.
sub ::record_function_hit
{ my($index)=@_;
- &preprocessor_ifndef("NDEBUG");
+ &preprocessor_ifdef("BORINGSSL_DISPATCH_TEST");
&push("ebx");
&push("edx");
&call(&label("pic"));
diff --git a/crypto/perlasm/x86gas.pl b/crypto/perlasm/x86gas.pl
index 4e19a89..7d0c00f 100644
--- a/crypto/perlasm/x86gas.pl
+++ b/crypto/perlasm/x86gas.pl
@@ -265,9 +265,9 @@
sub ::dataseg
{ push(@out,".data\n"); }
-sub ::preprocessor_ifndef
+sub ::preprocessor_ifdef
{ my($define)=@_;
- push(@out,"#ifndef ${define}\n");
+ push(@out,"#ifdef ${define}\n");
}
sub ::preprocessor_endif
diff --git a/crypto/perlasm/x86masm.pl b/crypto/perlasm/x86masm.pl
index 01bfbf3..50f8aed 100644
--- a/crypto/perlasm/x86masm.pl
+++ b/crypto/perlasm/x86masm.pl
@@ -203,9 +203,9 @@
push(@out,"ENDIF\n");
}
-sub ::preprocessor_ifndef
+sub ::preprocessor_ifdef
{ my($define)=@_;
- push(@out,"%ifndef ${define}\n");
+ push(@out,"%ifdef ${define}\n");
}
sub ::preprocessor_endif
diff --git a/crypto/perlasm/x86nasm.pl b/crypto/perlasm/x86nasm.pl
index a3e0ab7..7bb3347 100644
--- a/crypto/perlasm/x86nasm.pl
+++ b/crypto/perlasm/x86nasm.pl
@@ -191,9 +191,9 @@
push(@out,"%endif\n");
}
-sub ::preprocessor_ifndef
+sub ::preprocessor_ifdef
{ my($define)=@_;
- push(@out,"%ifndef ${define}\n");
+ push(@out,"%ifdef ${define}\n");
}
sub ::preprocessor_endif
diff --git a/include/openssl/cpu.h b/include/openssl/cpu.h
index eb36a57..ae55967 100644
--- a/include/openssl/cpu.h
+++ b/include/openssl/cpu.h
@@ -190,20 +190,19 @@
#endif // OPENSSL_PPC64LE
-#if !defined(NDEBUG) && !defined(BORINGSSL_FIPS)
+#if defined(BORINGSSL_DISPATCH_TEST)
// Runtime CPU dispatch testing support
// BORINGSSL_function_hit is an array of flags. The following functions will
-// set these flags in non-FIPS builds if NDEBUG is not defined.
+// set these flags if BORINGSSL_DISPATCH_TEST is defined.
// 0: aes_hw_ctr32_encrypt_blocks
// 1: aes_hw_encrypt
// 2: aesni_gcm_encrypt
// 3: aes_hw_set_encrypt_key
// 4: vpaes_encrypt
// 5: vpaes_set_encrypt_key
-// 6: bsaes_ctr32_encrypt_blocks
extern uint8_t BORINGSSL_function_hit[7];
-#endif // !NDEBUG && !FIPS
+#endif // BORINGSSL_DISPATCH_TEST
#if defined(__cplusplus)
