Add direction flag checking to CHECK_ABI.
Linux and Windows ABIs both require that the direction flag be cleared
on function exit, so that functions can rely on it being cleared on
entry. (Some OpenSSL assembly preserves it, which is stronger, but we
only require what is specified by the ABI so CHECK_ABI works with C
compiler output.)
Change-Id: I1a320aed4371176b4b44fe672f1a90167b84160f
Reviewed-on: https://boringssl-review.googlesource.com/c/34187
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/abi_self_test.cc b/crypto/abi_self_test.cc
index 9a1b868..7790ac5 100644
--- a/crypto/abi_self_test.cc
+++ b/crypto/abi_self_test.cc
@@ -174,6 +174,11 @@
CHECK_ABI_NO_UNWIND(abi_test_clobber_xmm14);
CHECK_ABI_NO_UNWIND(abi_test_clobber_xmm15);
#endif
+
+ EXPECT_NONFATAL_FAILURE(CHECK_ABI_NO_UNWIND(abi_test_set_direction_flag),
+ "Direction flag set after return");
+ EXPECT_EQ(0, abi_test_get_and_clear_direction_flag())
+ << "CHECK_ABI did not insulate the caller from direction flag errors";
}
#if defined(OPENSSL_WINDOWS)
diff --git a/crypto/test/abi_test.cc b/crypto/test/abi_test.cc
index e86f2f4..6530ddf 100644
--- a/crypto/test/abi_test.cc
+++ b/crypto/test/abi_test.cc
@@ -90,11 +90,25 @@
unwind &= g_unwind_tests_enabled;
CallerState state2 = state;
crypto_word_t ret = abi_test_trampoline(func, &state2, argv, argc, unwind);
+#if defined(OPENSSL_X86_64) || defined(OPENSSL_X86)
+ // Query and clear the direction flag early, so negative tests do not
+ // interfere with |malloc|.
+ bool direction_flag = abi_test_get_and_clear_direction_flag();
+#endif // OPENSSL_X86_64 || OPENSSL_X86
*out = Result();
ForEachMismatch(state, state2, [&](const char *reg) {
out->errors.push_back(std::string(reg) + " was not restored after return");
});
+#if defined(OPENSSL_X86_64) || defined(OPENSSL_X86)
+ // Linux and Windows ABIs for x86 require the direction flag be cleared on
+ // return. (Some OpenSSL assembly preserves it, which is stronger, but we only
+ // require what is specified by the ABI so |CHECK_ABI| works with C compiler
+ // output.)
+ if (direction_flag) {
+ out->errors.emplace_back("Direction flag set after return");
+ }
+#endif // OPENSSL_X86_64 || OPENSSL_X86
if (unwind) {
ReadUnwindResult(out);
}
diff --git a/crypto/test/abi_test.h b/crypto/test/abi_test.h
index 23f3aa5..370d455 100644
--- a/crypto/test/abi_test.h
+++ b/crypto/test/abi_test.h
@@ -286,6 +286,17 @@
// storage space for a saved register, breaking unwind.
void abi_test_bad_unwind_temporary(void);
+#if defined(OPENSSL_X86_64) || defined(OPENSSL_X86)
+// abi_test_get_and_clear_direction_flag clears the direction flag. If the flag
+// was previously set, it returns one. Otherwise, it returns zero.
+int abi_test_get_and_clear_direction_flag(void);
+
+// abi_test_set_direction_flag sets the direction flag. This does not conform to
+// ABI requirements and must only be called within a |CHECK_ABI| guard to avoid
+// errors later in the program.
+int abi_test_set_direction_flag(void);
+#endif // OPENSSL_X86_64 || OPENSSL_X86
+
} // extern "C"
#endif // SUPPORTS_ABI_TEST
diff --git a/crypto/test/asm/trampoline-x86_64.pl b/crypto/test/asm/trampoline-x86_64.pl
index d41aadf..4af9b2a 100755
--- a/crypto/test/asm/trampoline-x86_64.pl
+++ b/crypto/test/asm/trampoline-x86_64.pl
@@ -366,6 +366,29 @@
ret
.cfi_endproc
.size abi_test_bad_unwind_temporary,.-abi_test_bad_unwind_temporary
+
+# abi_test_get_and_clear_direction_flag clears the direction flag. If the flag
+# was previously set, it returns one. Otherwise, it returns zero.
+# int abi_test_get_and_clear_direction_flag(void);
+.type abi_test_set_direction_flag, \@abi-omnipotent
+.globl abi_test_get_and_clear_direction_flag
+abi_test_get_and_clear_direction_flag:
+ pushfq
+ popq %rax
+ andq \$0x400, %rax
+ shlq \$10, %rax
+ cld
+ ret
+.size abi_test_get_and_clear_direction_flag,.-abi_test_get_and_clear_direction_flag
+
+# abi_test_set_direction_flag sets the direction flag.
+# void abi_test_set_direction_flag(void);
+.type abi_test_set_direction_flag, \@abi-omnipotent
+.globl abi_test_set_direction_flag
+abi_test_set_direction_flag:
+ std
+ ret
+.size abi_test_set_direction_flag,.-abi_test_set_direction_flag
____
if ($win64) {
