[llvm-branch-commits] [libc] ab3cbe4 - [libc] Raise x87 exceptions by synchronizing with "fwait".
Siva Chandra Reddy via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Dec 8 13:21:28 PST 2020
Author: Siva Chandra Reddy
Date: 2020-12-08T13:16:19-08:00
New Revision: ab3cbe4bc0d952ca58a3e2263629591ef898de3f
URL: https://github.com/llvm/llvm-project/commit/ab3cbe4bc0d952ca58a3e2263629591ef898de3f
DIFF: https://github.com/llvm/llvm-project/commit/ab3cbe4bc0d952ca58a3e2263629591ef898de3f.diff
LOG: [libc] Raise x87 exceptions by synchronizing with "fwait".
Couple of helper functions enableExcept and disableExcept have been
added. In a later round, they will be used to implemented the GNU
extension functions feenableexcept and fedisableexcept.
Differential Revision: https://reviews.llvm.org/D92821
Added:
libc/test/src/fenv/enabled_exceptions_test.cpp
Modified:
libc/cmake/modules/LLVMLibCTestRules.cmake
libc/test/src/fenv/CMakeLists.txt
libc/test/src/fenv/exception_status_test.cpp
libc/utils/FPUtil/x86_64/FEnv.h
Removed:
################################################################################
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index a30259c05382..33dc2cc7ca74 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -137,7 +137,7 @@ function(add_libc_unittest target_name)
)
if(LIBC_UNITTEST_COMPILE_OPTIONS)
target_compile_options(
- ${target_name}
+ ${fq_target_name}
PRIVATE ${LIBC_UNITTEST_COMPILE_OPTIONS}
)
endif()
diff --git a/libc/test/src/fenv/CMakeLists.txt b/libc/test/src/fenv/CMakeLists.txt
index fb9666e2ed91..09e37b15a7f2 100644
--- a/libc/test/src/fenv/CMakeLists.txt
+++ b/libc/test/src/fenv/CMakeLists.txt
@@ -21,4 +21,23 @@ add_libc_unittest(
libc.src.fenv.feclearexcept
libc.src.fenv.feraiseexcept
libc.src.fenv.fetestexcept
+ libc.utils.FPUtil.fputil
)
+
+if (NOT LLVM_USE_SANITIZER)
+ # Sanitizers don't like SIGFPE. So, we will run the
+ # tests which raise SIGFPE only in non-sanitizer builds.
+ add_libc_unittest(
+ enabled_exceptions_test
+ SUITE
+ libc_fenv_unittests
+ SRCS
+ enabled_exceptions_test.cpp
+ DEPENDS
+ libc.include.signal
+ libc.src.fenv.feclearexcept
+ libc.src.fenv.feraiseexcept
+ libc.src.fenv.fetestexcept
+ libc.utils.FPUtil.fputil
+ )
+endif()
diff --git a/libc/test/src/fenv/enabled_exceptions_test.cpp b/libc/test/src/fenv/enabled_exceptions_test.cpp
new file mode 100644
index 000000000000..23151286a907
--- /dev/null
+++ b/libc/test/src/fenv/enabled_exceptions_test.cpp
@@ -0,0 +1,50 @@
+//===-- Unittests for feraiseexcept with exceptions enabled ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/fenv/feclearexcept.h"
+#include "src/fenv/feraiseexcept.h"
+#include "src/fenv/fetestexcept.h"
+
+#include "utils/FPUtil/FEnv.h"
+#include "utils/UnitTest/Test.h"
+
+#include <fenv.h>
+#include <signal.h>
+
+// This test enables an exception and verifies that raising that exception
+// triggers SIGFPE.
+TEST(ExceptionStatusTest, RaiseAndCrash) {
+ // TODO: Install a floating point exception handler and verify that the
+ // the expected exception was raised. One will have to longjmp back from
+ // that exception handler, so such a testing can be done after we have
+ // longjmp implemented.
+
+ int excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
+ FE_UNDERFLOW};
+
+ for (int e : excepts) {
+ ASSERT_DEATH(
+ [=] {
+ __llvm_libc::fputil::disableExcept(FE_ALL_EXCEPT);
+ __llvm_libc::fputil::enableExcept(e);
+ ASSERT_EQ(__llvm_libc::feclearexcept(FE_ALL_EXCEPT), 0);
+ // Raising all exceptions except |e| should not call the
+ // SIGFPE handler. They should set the exception flag though,
+ // so we verify that.
+ int others = FE_ALL_EXCEPT & ~e;
+ ASSERT_EQ(__llvm_libc::feraiseexcept(others), 0);
+ ASSERT_EQ(__llvm_libc::fetestexcept(others), others);
+
+ // We don't check the return value when raising |e| as
+ // feraiseexcept will not return when it raises an enabled
+ // exception.
+ __llvm_libc::feraiseexcept(e);
+ },
+ SIGFPE);
+ }
+}
diff --git a/libc/test/src/fenv/exception_status_test.cpp b/libc/test/src/fenv/exception_status_test.cpp
index 353e5aa6b0e7..193bc58298e1 100644
--- a/libc/test/src/fenv/exception_status_test.cpp
+++ b/libc/test/src/fenv/exception_status_test.cpp
@@ -10,11 +10,18 @@
#include "src/fenv/feraiseexcept.h"
#include "src/fenv/fetestexcept.h"
+#include "utils/FPUtil/FEnv.h"
#include "utils/UnitTest/Test.h"
#include <fenv.h>
TEST(ExceptionStatusTest, RaiseAndTest) {
+ // This test raises a set of exceptions and checks that the exception
+ // status flags are updated. The intention is really not to invoke the
+ // exception handler. Hence, we will disable all exceptions at the
+ // beginning.
+ __llvm_libc::fputil::disableExcept(FE_ALL_EXCEPT);
+
int excepts[] = {FE_DIVBYZERO, FE_INVALID, FE_INEXACT, FE_OVERFLOW,
FE_UNDERFLOW};
for (int e : excepts) {
diff --git a/libc/utils/FPUtil/x86_64/FEnv.h b/libc/utils/FPUtil/x86_64/FEnv.h
index 92ae8049e1d5..9e0e2be84614 100644
--- a/libc/utils/FPUtil/x86_64/FEnv.h
+++ b/libc/utils/FPUtil/x86_64/FEnv.h
@@ -48,6 +48,12 @@ struct ExceptionFlags {
static constexpr uint16_t Inexact = 0x20;
};
+// The exception control bits occupy six bits, one bit for each exception.
+// In the x87 control word, they occupy the first 6 bits. In the MXCSR
+// register, they occupy bits 7 to 12.
+static constexpr uint16_t X87ExceptionControlBitPosition = 0;
+static constexpr uint16_t MXCSRExceptionContolBitPoistion = 7;
+
// Exception flags are individual bits in the corresponding registers.
// So, we just OR the bit values to get the full set of exceptions.
static inline uint16_t getStatusValueForExcept(int excepts) {
@@ -68,6 +74,15 @@ static inline int exceptionStatusToMacro(uint16_t status) {
(status & ExceptionFlags::Inexact ? FE_INEXACT : 0);
}
+struct X87State {
+ uint16_t ControlWord;
+ uint16_t Unused1;
+ uint16_t StatusWord;
+ uint16_t Unused2;
+ // TODO: Elaborate the remaining 20 bytes as required.
+ uint32_t _[5];
+};
+
static inline uint16_t getX87ControlWord() {
uint16_t w;
__asm__ __volatile__("fnstcw %0" : "=m"(w)::);
@@ -98,8 +113,65 @@ static inline void writeMXCSR(uint32_t w) {
__asm__ __volatile__("ldmxcsr %0" : : "m"(w) :);
}
+static inline void getX87State(X87State &s) {
+ __asm__ __volatile__("fnstenv %0" : "=m"(s));
+}
+
+static inline void writeX87State(const X87State &s) {
+ __asm__ __volatile__("fldenv %0" : : "m"(s) :);
+}
+
+static inline void fwait() { __asm__ __volatile__("fwait"); }
+
} // namespace internal
+static inline int enableExcept(int excepts) {
+ // In the x87 control word and in MXCSR, an exception is blocked
+ // if the corresponding bit is set. That is the reason for all the
+ // bit-flip operations below as we need to turn the bits to zero
+ // to enable them.
+
+ uint16_t bitMask = internal::getStatusValueForExcept(excepts);
+
+ uint16_t x87CW = internal::getX87ControlWord();
+ uint16_t oldExcepts = ~x87CW & 0x3F; // Save previously enabled exceptions.
+ x87CW &= ~bitMask;
+ internal::writeX87ControlWord(x87CW);
+
+ // Enabling SSE exceptions via MXCSR is a nice thing to do but
+ // might not be of much use practically as SSE exceptions and the x87
+ // exceptions are independent of each other.
+ uint32_t mxcsr = internal::getMXCSR();
+ mxcsr &= ~(bitMask << internal::MXCSRExceptionContolBitPoistion);
+ internal::writeMXCSR(mxcsr);
+
+ // Since the x87 exceptions and SSE exceptions are independent of each,
+ // it doesn't make much sence to report both in the return value. Most
+ // often, the standard floating point functions deal with FPU operations
+ // so we will retrun only the old x87 exceptions.
+ return internal::exceptionStatusToMacro(oldExcepts);
+}
+
+static inline int disableExcept(int excepts) {
+ // In the x87 control word and in MXCSR, an exception is blocked
+ // if the corresponding bit is set.
+
+ uint16_t bitMask = internal::getStatusValueForExcept(excepts);
+
+ uint16_t x87CW = internal::getX87ControlWord();
+ uint16_t oldExcepts = ~x87CW & 0x3F; // Save previously enabled exceptions.
+ x87CW |= bitMask;
+ internal::writeX87ControlWord(x87CW);
+
+ // Just like in enableExcept, it is not clear if disabling SSE exceptions
+ // is required. But, we will still do it only as a "nice thing to do".
+ uint32_t mxcsr = internal::getMXCSR();
+ mxcsr |= (bitMask << internal::MXCSRExceptionContolBitPoistion);
+ internal::writeMXCSR(mxcsr);
+
+ return internal::exceptionStatusToMacro(oldExcepts);
+}
+
static inline int clearExcept(int excepts) {
// An instruction to write to x87 status word ins't available. So, we
// just clear all of the x87 exceptions.
@@ -123,14 +195,62 @@ static inline int testExcept(int excepts) {
}
static inline int raiseExcept(int excepts) {
- // It is enough to set the exception flags in MXCSR.
- // TODO: Investigate if each exception has to be raised one at a time
- // followed with an fwait instruction before writing the flag for the
- // next exception.
uint16_t statusValue = internal::getStatusValueForExcept(excepts);
- uint32_t mxcsr = internal::getMXCSR();
- mxcsr = mxcsr | statusValue;
- internal::writeMXCSR(mxcsr);
+
+ // We set the status flag for exception one at a time and call the
+ // fwait instruction to actually get the processor to raise the
+ // exception by calling the exception handler. This scheme is per
+ // the description in in "8.6 X87 FPU EXCEPTION SYNCHRONIZATION"
+ // of the "Intel 64 and IA-32 Architectures Software Developer's
+ // Manual, Vol 1".
+
+ // FPU status word is read for each exception seperately as the
+ // exception handler can potentially write to it (typically to clear
+ // the corresponding exception flag). By reading it separately, we
+ // ensure that the writes by the exception handler are maintained
+ // when raising the next exception.
+
+ if (statusValue & internal::ExceptionFlags::Invalid) {
+ internal::X87State state;
+ internal::getX87State(state);
+ state.StatusWord |= internal::ExceptionFlags::Invalid;
+ internal::writeX87State(state);
+ internal::fwait();
+ }
+ if (statusValue & internal::ExceptionFlags::DivByZero) {
+ internal::X87State state;
+ internal::getX87State(state);
+ state.StatusWord |= internal::ExceptionFlags::DivByZero;
+ internal::writeX87State(state);
+ internal::fwait();
+ }
+ if (statusValue & internal::ExceptionFlags::Overflow) {
+ internal::X87State state;
+ internal::getX87State(state);
+ state.StatusWord |= internal::ExceptionFlags::Overflow;
+ internal::writeX87State(state);
+ internal::fwait();
+ }
+ if (statusValue & internal::ExceptionFlags::Underflow) {
+ internal::X87State state;
+ internal::getX87State(state);
+ state.StatusWord |= internal::ExceptionFlags::Underflow;
+ internal::writeX87State(state);
+ internal::fwait();
+ }
+ if (statusValue & internal::ExceptionFlags::Inexact) {
+ internal::X87State state;
+ internal::getX87State(state);
+ state.StatusWord |= internal::ExceptionFlags::Inexact;
+ internal::writeX87State(state);
+ internal::fwait();
+ }
+
+ // There is no special synchronization scheme available to
+ // raise SEE exceptions. So, we will ignore that for now.
+ // Just plain writing to the MXCSR register does not guarantee
+ // the exception handler will be called.
+
return 0;
}
More information about the llvm-branch-commits
mailing list