[libc][windows] fix small build issues.

[michaelrj]

Summary

The windows build has fallen behind a little, this patch fixes some
issues that were preventing it from building.
Specifically: Some subfolders weren't being included, leading to missing
targets in the cmake.

[lntue]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57

Can you also change other exception flags to the same format too?

[sivachandra]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57

Where is the macro named OVERFLOW coming from?

[sivachandra]

libc/src/CMakeLists.txt

12 ↗

(On Diff #463022)

I think this is a case of inappropriate design somewhere - adding these directories for Windows incorrectly implies that they are available for Windows as well.

[michaelrj]

address comments

libc/src/CMakeLists.txt

12 ↗

(On Diff #463022)

as far as I can tell, the issue is coming from remove_test which is in test/src/stdio. It includes libc.src.unistd.access which doesn't exist unless the unistd folder is included. Putting that inside an "if target is linux" condition also works, but I feel like that is prone to mistakes for future targets that also need linux only pieces.

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57

here's the error:

In file included from C:/src/llvm-project/libc\src/__support/FPUtil/FEnvImpl.h:21:
C:/src/llvm-project/libc\src/__support/FPUtil/x86_64/FEnvImpl.h(271,44): error: expected unqualified-id
    raise_helper(internal::ExceptionFlags::OVERFLOW);
                                           ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\ucrt\corecrt_math.h(965,25): note: expanded from macro 'OVERFLOW'
    #define OVERFLOW    _OVERFLOW
                        ^
C:\Program Files (x86)\Windows Kits\10\include\10.0.19041.0\ucrt\corecrt_math.h(68,21): note: expanded from macro '_OVERFLOW'
#define _OVERFLOW   3   // overflow range error

If I had to guess it's coming in through #include <fenv.h>

[lntue]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

51

This one also?

57

Look like that definition is guarded by _CRT_INTERNAL_NONSTDC_NAMES (https://github.com/tpn/winsdk-10/blob/master/Include/10.0.16299.0/ucrt/corecrt_math.h#L962) internally to Windows C runtimes? Either #undef this or adding some Windows specific build flag might be another solution?

[michaelrj]

fix missed flag

[michaelrj]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57

#undef OVERFLOW does work, but that feels like a dangerous solution

[lntue]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57

What I meant is to detect if _CRT_INTERNAL_NONSTDC_NAMES is defined then #undef _CRT_INTERNAL_NONSTDC_NAMES before including <fenv.h> and maybe redefine it after for Windows.

[sivachandra]

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

57

I thought these macros stay undefined when compiling with clang-cl. Does something from here help: https://clang.llvm.org/docs/MSVCCompatibility.html

[michaelrj]

split fenv changes to separate patch

libc/src/__support/FPUtil/x86_64/FEnvImpl.h

 };
 
 static constexpr uint16_t X87_ROUNDING_CONTROL_BIT_POSITION = 10;
 static constexpr uint16_t MXCSR_ROUNDING_CONTROL_BIT_POSITION = 13;
 
 // The exception flags in the x87 status register and the MXCSR have the same
 // encoding as well as the same bit positions.
 struct ExceptionFlags {
-  static constexpr uint16_t INVALID = 0x1;
+  static constexpr uint16_t INVALID_F = 0x1;
   // Some libcs define __FE_DENORM corresponding to the denormal input
   // exception and include it in FE_ALL_EXCEPTS. We define and use it to
   // support compiling against headers provided by such libcs.
-  static constexpr uint16_t DENORMAL = 0x2;
-  static constexpr uint16_t DIV_BY_ZERO = 0x4;
-  static constexpr uint16_t OVERFLOW = 0x8;
-  static constexpr uint16_t UNDERFLOW = 0x10;
-  static constexpr uint16_t INEXACT = 0x20;
+  static constexpr uint16_t DENORMAL_F = 0x2;
+  static constexpr uint16_t DIV_BY_ZERO_F = 0x4;
+  static constexpr uint16_t OVERFLOW_F = 0x8;
+  static constexpr uint16_t UNDERFLOW_F = 0x10;
+  static constexpr uint16_t INEXACT_F = 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 X87_EXCEPTION_CONTROL_BIT_POSITION = 0;
 static constexpr uint16_t X87_EXCEPTION_CONTROL_BIT_POSITION_HIGH = 24;
 static constexpr uint16_t MXCSR_EXCEPTION_CONTOL_BIT_POISTION = 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 get_status_value_for_except(int excepts) {
   // We will make use of the fact that exception control bits are single
   // bit flags in the control registers.
-  return (excepts & FE_INVALID ? ExceptionFlags::INVALID : 0) |
+  return (excepts & FE_INVALID ? ExceptionFlags::INVALID_F : 0) |
 #ifdef __FE_DENORM
-         (excepts & __FE_DENORM ? ExceptionFlags::DENORMAL : 0) |
+         (excepts & __FE_DENORM ? ExceptionFlags::DENORMAL_F : 0) |
 #endif // __FE_DENORM
-         (excepts & FE_DIVBYZERO ? ExceptionFlags::DIV_BY_ZERO : 0) |
-         (excepts & FE_OVERFLOW ? ExceptionFlags::OVERFLOW : 0) |
-         (excepts & FE_UNDERFLOW ? ExceptionFlags::UNDERFLOW : 0) |
-         (excepts & FE_INEXACT ? ExceptionFlags::INEXACT : 0);
+         (excepts & FE_DIVBYZERO ? ExceptionFlags::DIV_BY_ZERO_F : 0) |
+         (excepts & FE_OVERFLOW ? ExceptionFlags::OVERFLOW_F : 0) |
+         (excepts & FE_UNDERFLOW ? ExceptionFlags::UNDERFLOW_F : 0) |
+         (excepts & FE_INEXACT ? ExceptionFlags::INEXACT_F : 0);
 }
 
 static inline int exception_status_to_macro(uint16_t status) {
-  return (status & ExceptionFlags::INVALID ? FE_INVALID : 0) |
+  return (status & ExceptionFlags::INVALID_F ? FE_INVALID : 0) |
 #ifdef __FE_DENORM
-         (status & ExceptionFlags::DENORMAL ? __FE_DENORM : 0) |
+         (status & ExceptionFlags::DENORMAL_F ? __FE_DENORM : 0) |
 #endif // __FE_DENORM
-         (status & ExceptionFlags::DIV_BY_ZERO ? FE_DIVBYZERO : 0) |
-         (status & ExceptionFlags::OVERFLOW ? FE_OVERFLOW : 0) |
-         (status & ExceptionFlags::UNDERFLOW ? FE_UNDERFLOW : 0) |
-         (status & ExceptionFlags::INEXACT ? FE_INEXACT : 0);
+         (status & ExceptionFlags::DIV_BY_ZERO_F ? FE_DIVBYZERO : 0) |
+         (status & ExceptionFlags::OVERFLOW_F ? FE_OVERFLOW : 0) |
+         (status & ExceptionFlags::UNDERFLOW_F ? FE_UNDERFLOW : 0) |
+         (status & ExceptionFlags::INEXACT_F ? FE_INEXACT : 0);
 }
 
 struct X87StateDescriptor {
   uint16_t control_word;
   uint16_t unused1;
   uint16_t status_word;
   uint16_t unused2;
   // TODO: Elaborate the remaining 20 bytes as required.
auto raise_helper = [](uint16_t singleExceptFlag) {
     mxcsr = internal::get_mxcsr();
     state.status_word |= singleExceptFlag;
     mxcsr |= singleExceptFlag;
     internal::write_x87_state_descriptor(state);
     internal::write_mxcsr(mxcsr);
     internal::fwait();
   };
 
-  if (status_value & internal::ExceptionFlags::INVALID)
-    raise_helper(internal::ExceptionFlags::INVALID);
-  if (status_value & internal::ExceptionFlags::DIV_BY_ZERO)
-    raise_helper(internal::ExceptionFlags::DIV_BY_ZERO);
-  if (status_value & internal::ExceptionFlags::OVERFLOW)
-    raise_helper(internal::ExceptionFlags::OVERFLOW);
-  if (status_value & internal::ExceptionFlags::UNDERFLOW)
-    raise_helper(internal::ExceptionFlags::UNDERFLOW);
-  if (status_value & internal::ExceptionFlags::INEXACT)
-    raise_helper(internal::ExceptionFlags::INEXACT);
+  if (status_value & internal::ExceptionFlags::INVALID_F)
+    raise_helper(internal::ExceptionFlags::INVALID_F);
+  if (status_value & internal::ExceptionFlags::DIV_BY_ZERO_F)
+    raise_helper(internal::ExceptionFlags::DIV_BY_ZERO_F);
+  if (status_value & internal::ExceptionFlags::OVERFLOW_F)
+    raise_helper(internal::ExceptionFlags::OVERFLOW_F);
+  if (status_value & internal::ExceptionFlags::UNDERFLOW_F)
+    raise_helper(internal::ExceptionFlags::UNDERFLOW_F);
+  if (status_value & internal::ExceptionFlags::INEXACT_F)
+    raise_helper(internal::ExceptionFlags::INEXACT_F);
 #ifdef __FE_DENORM
-  if (status_value & internal::ExceptionFlags::DENORMAL) {
-    raise_helper(internal::ExceptionFlags::DENORMAL);
+  if (status_value & internal::ExceptionFlags::DENORMAL_F) {
+    raise_helper(internal::ExceptionFlags::DENORMAL_F);
   }
 #endif // __FE_DENORM
 
   // 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.
 
static_assert(
     sizeof(fenv_t) == sizeof(internal::FPState),
     "Internal floating point state does not match the public fenv_t type.");
 
 #ifdef _WIN32
 
 // The exception flags in the Windows FEnv struct and the MXCSR have almost
 // reversed bit positions.
 struct WinExceptionFlags {
-  static constexpr uint32_t INEXACT = 0x01;
-  static constexpr uint32_t UNDERFLOW = 0x02;
-  static constexpr uint32_t OVERFLOW = 0x04;
-  static constexpr uint32_t DIV_BY_ZERO = 0x08;
-  static constexpr uint32_t INVALID = 0x10;
-  static constexpr uint32_t DENORMAL = 0x20;
+  static constexpr uint32_t INEXACT_WIN = 0x01;
+  static constexpr uint32_t UNDERFLOW_WIN = 0x02;
+  static constexpr uint32_t OVERFLOW_WIN = 0x04;
+  static constexpr uint32_t DIV_BY_ZERO_WIN = 0x08;
+  static constexpr uint32_t INVALID_WIN = 0x10;
+  static constexpr uint32_t DENORMAL_WIN = 0x20;
 
   // The Windows FEnv struct has a second copy of all of these bits in the high
   // byte of the 32 bit control word. These are used as the source of truth when
   // calling fesetenv.
   static constexpr uint32_t HIGH_OFFSET = 24;
 
-  static constexpr uint32_t HIGH_INEXACT = INEXACT << HIGH_OFFSET;
-  static constexpr uint32_t HIGH_UNDERFLOW = UNDERFLOW << HIGH_OFFSET;
-  static constexpr uint32_t HIGH_OVERFLOW = OVERFLOW << HIGH_OFFSET;
-  static constexpr uint32_t HIGH_DIV_BY_ZERO = DIV_BY_ZERO << HIGH_OFFSET;
-  static constexpr uint32_t HIGH_INVALID = INVALID << HIGH_OFFSET;
-  static constexpr uint32_t HIGH_DENORMAL = DENORMAL << HIGH_OFFSET;
+  static constexpr uint32_t HIGH_INEXACT = INEXACT_WIN << HIGH_OFFSET;
+  static constexpr uint32_t HIGH_UNDERFLOW = UNDERFLOW_WIN << HIGH_OFFSET;
+  static constexpr uint32_t HIGH_OVERFLOW = OVERFLOW_WIN << HIGH_OFFSET;
+  static constexpr uint32_t HIGH_DIV_BY_ZERO = DIV_BY_ZERO_WIN << HIGH_OFFSET;
+  static constexpr uint32_t HIGH_INVALID = INVALID_WIN << HIGH_OFFSET;
+  static constexpr uint32_t HIGH_DENORMAL = DENORMAL_WIN << HIGH_OFFSET;
 };
 
 /*
     fenv_t control word format:
 
     Windows (at least for x64) uses a 4 byte control fenv control word stored in
     a 32 bit integer. The first byte contains just the rounding mode and the
     exception masks, while the last two bytes contain that same information as
static inline int get_env(fenv_t *envp) {
   internal::FPState *state = reinterpret_cast<internal::FPState *>(envp);
 
   uint32_t status_word = 0;
   uint32_t control_word = 0;
 
   uint32_t mxcsr = internal::get_mxcsr();
 
   // Set exception flags in the status word
-  status_word |= (mxcsr & (internal::ExceptionFlags::INVALID |
-                           internal::ExceptionFlags::DENORMAL))
+  status_word |= (mxcsr & (internal::ExceptionFlags::INVALID_F |
+                           internal::ExceptionFlags::DENORMAL_F))
                  << 4;
-  status_word |= (mxcsr & internal::ExceptionFlags::DIV_BY_ZERO) << 1;
-  status_word |= (mxcsr & internal::ExceptionFlags::OVERFLOW) >> 1;
-  status_word |= (mxcsr & internal::ExceptionFlags::UNDERFLOW) >> 3;
-  status_word |= (mxcsr & internal::ExceptionFlags::INEXACT) >> 5;
+  status_word |= (mxcsr & internal::ExceptionFlags::DIV_BY_ZERO_F) << 1;
+  status_word |= (mxcsr & internal::ExceptionFlags::OVERFLOW_F) >> 1;
+  status_word |= (mxcsr & internal::ExceptionFlags::UNDERFLOW_F) >> 3;
+  status_word |= (mxcsr & internal::ExceptionFlags::INEXACT_F) >> 5;
   status_word |= status_word << WinExceptionFlags::HIGH_OFFSET;
 
   // Set exception masks in bits 0-5 and 24-29
-  control_word |=
-      (mxcsr &
-       ((internal::ExceptionFlags::INVALID | internal::ExceptionFlags::DENORMAL)
+  control_word |= (mxcsr & ((internal::ExceptionFlags::INVALID_F |
+                             internal::ExceptionFlags::DENORMAL_F)
                             << 7)) >>
                   3;
-  control_word |= (mxcsr & (internal::ExceptionFlags::DIV_BY_ZERO << 7)) >> 6;
-  control_word |= (mxcsr & (internal::ExceptionFlags::OVERFLOW << 7)) >> 8;
-  control_word |= (mxcsr & (internal::ExceptionFlags::UNDERFLOW << 7)) >> 10;
-  control_word |= (mxcsr & (internal::ExceptionFlags::INEXACT << 7)) >> 12;
+  control_word |= (mxcsr & (internal::ExceptionFlags::DIV_BY_ZERO_F << 7)) >> 6;
+  control_word |= (mxcsr & (internal::ExceptionFlags::OVERFLOW_F << 7)) >> 8;
+  control_word |= (mxcsr & (internal::ExceptionFlags::UNDERFLOW_F << 7)) >> 10;
+  control_word |= (mxcsr & (internal::ExceptionFlags::INEXACT_F << 7)) >> 12;
   control_word |= control_word << WinExceptionFlags::HIGH_OFFSET;
 
   // Set rounding in bits 8-9 and 30-31
   control_word |= (mxcsr & 0x6000) >> 5;
   control_word |= (mxcsr & 0x6000) << 17;
 
   // Set flush-to-zero in bit 10
   control_word |= (mxcsr & 0x8000) >> 5;

libc/test/src/stdio/CMakeLists.txt

add_libc_unittest(
   SUITE
     libc_stdio_unittests
   SRCS
     puts_test.cpp
   DEPENDS
     libc.src.stdio.puts
 )
 
+if(${LIBC_TARGET_OS} STREQUAL "linux")
   add_libc_unittest(
     remove_test
     SUITE
       libc_stdio_unittests
     SRCS
       remove_test.cpp
     DEPENDS
       libc.include.errno
       libc.include.unistd
       libc.src.fcntl.open
       libc.src.stdio.remove
       libc.src.sys.stat.mkdirat
       libc.src.unistd.access
       libc.src.unistd.close
   )
+endif()
 
 add_subdirectory(printf_core)
 add_subdirectory(testdata)