[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