Problem compiling OpenMP to Wasm with Emscripten in Windows

[maniatic0]

With the recent (as of Nov 2024) port of OpenMP to Wasm/Emscripten [1, 2, 3], I decided to port some high perf libs with it. While compiling it and building in Windows using CMake/Ninja I found an issue with kmp_i18n_default.inc's generation. It's getting generated with Window's print modifiers rather than Unix ones, which crashes with an OOM when executed.

After tracking how those strings are generated, I found the issue in message-converter.py at

llvm-project/openmp/runtime/tools/message-converter.py

Line 58 in 54dad9e

if platform.system() == "Windows":

It should generate Unix modifiers even if my compilation platform is Windows.

Steps to Reproduce

In a Windows machine, configure OpenMP with Cmake and then check the generated kmp_i18n_default.inc (you don't need to compile unless you want to check the crash). Observe that it has Windows printf modifers rather than the expected Unix ones for Emscripten.

For example, I use https://github.com/maniatic0/wasm-win-openmp-fix/blob/main/build.bat

call emcmake cmake ^
    -DOPENMP_STANDALONE_BUILD=ON ^
    -DOPENMP_ENABLE_OMPT_TOOLS=OFF ^
    -DOPENMP_ENABLE_LIBOMPTARGET=OFF ^
    -DLIBOMP_HAVE_OMPT_SUPPORT=OFF ^
    -DLIBOMP_OMPT_SUPPORT=OFF ^
    -DLIBOMP_OMPD_SUPPORT=OFF ^
    -DLIBOMP_USE_DEBUGGER=OFF ^
    -DLIBOMP_FORTRAN_MODULES=OFF ^
    -DLIBOMP_ENABLE_SHARED=OFF ^
    -DLIBOMP_ARCH=wasm32 ^
    -DCMAKE_INSTALL_PREFIX=%BUILD_DIR% ^
    -G "Ninja Multi-Config" ^
    -B %BUILD_DIR% ^
    -S %OPENMP_DIR%

The file is generated at %BUILD_DIR%\runtime\src\kmp_i18n_default.inc

Suggestion

I think a configuration variable like LIBOMP_ARCH could help fix the issue. Something like LIBOMP_PLATFORM=None|Unix|Windows, which could be checked in message-converter.py (if the variable is None/Empty, it should default to the platform check).

Workaround

After configuring with CMake and before compiling, fix the modifiers. This can be done with a Python script, which is what I do with https://github.com/maniatic0/wasm-win-openmp-fix/blob/main/fix_kmp_i18n_default.py at https://github.com/maniatic0/wasm-win-openmp-fix/blob/d40c146aa0e04ce17686d5ab831b474e94266d39/build.bat#L33

Crash

Not sure if this is an issue in itself, but for completeness. The way I found the above problem with the string generation was due to OOM crashes coming from __kmp_str_buf_vprint in kmp_str.cpp

llvm-project/openmp/runtime/src/kmp_str.cpp

Line 206 in 54dad9e

size = buffer->size * 2;

My comments are marked with <---

int __kmp_str_buf_vprint(kmp_str_buf_t *buffer, char const *format,
                         va_list args) {
  int rc;
  KMP_STR_BUF_INVARIANT(buffer);

  for (;;) {
    int const free = buffer->size - buffer->used;
    int size;

    // Try to format string.
    {
      /* On Linux* OS Intel(R) 64, vsnprintf() modifies args argument, so
         vsnprintf() crashes if it is called for the second time with the same
         args. To prevent the crash, we have to pass a fresh intact copy of args
         to vsnprintf() on each iteration.

         Unfortunately, standard va_copy() macro is not available on Windows*
         OS. However, it seems vsnprintf() does not modify args argument on
         Windows* OS.
      */

#if !KMP_OS_WINDOWS
      va_list _args;
      va_copy(_args, args); // Make copy of args.
#define args _args // Substitute args with its copy, _args.
#endif // KMP_OS_WINDOWS
      rc = KMP_VSNPRINTF(buffer->str + buffer->used, free, format, args); // <--- This fails with -1 (encoding issue) when used with Window's modifiers
#if !KMP_OS_WINDOWS
#undef args // Remove substitution.
      va_end(_args);
#endif // KMP_OS_WINDOWS
    }

    // No errors, string has been formatted.
    if (rc >= 0 && rc < free) {
      buffer->used += rc;
      break;
    }

    // Error occurred, buffer is too small.
    if (rc >= 0) {
      // C99-conforming implementation of vsnprintf returns required buffer size
      size = buffer->used + rc + 1;
    } else {
      // Older implementations just return -1. Double buffer size.
      size = buffer->size * 2; // <---  This treats the constraint/encoding issues as not enough buffer. This grows until crashing
    }

    // Enlarge buffer.
    __kmp_str_buf_reserve(buffer, size);

    // And try again.
  }

  KMP_DEBUG_ASSERT(buffer->size > 0);
  KMP_STR_BUF_INVARIANT(buffer);
  return rc;
} // __kmp_str_buf_vprint

KMP_VSNPRINTF is defined as vsnprintf_s at

llvm-project/openmp/runtime/src/kmp_safe_c_api.h

Line 52 in 54dad9e

#define KMP_VSNPRINTF(dst, cnt, fmt, arg) \

From the standard, the negative return value means that there was a constraint or encoding issue (which would be correct as it's using Windows modifiers rather than Unix ones). This is then treated as an unknown error that the buffer is not big enough, so it's doubled in size which then runs out of memory inside the loop. More info in my notes.

I have to note that there is a comment in the code about older implementations returning negative numbers for errors about the buffer not being big enough, so I'm not sure if this is an issue.

[1] #71297
[2] #95169
[3] https://reviews.llvm.org/D142593?id=492403

[llvmbot]

@llvm/issue-subscribers-openmp

Author: Christian Oliveros (maniatic0)

With the recent (as of Nov 2024) port of OpenMP to Wasm/Emscripten [[1](https://github.com//pull/71297), [2](https://github.com//pull/95169), [3](https://reviews.llvm.org/D142593?id=492403)], I decided to port some high perf libs with it. While compiling it and building in Windows using CMake/Ninja I found an issue with ```kmp_i18n_default.inc```'s generation. It's getting generated with Window's print modifiers rather than Unix ones, which crashes with an OOM when executed.

After tracking how those strings are generated, I found the issue in message-converter.py at

llvm-project/openmp/runtime/tools/message-converter.py

Line 58 in 54dad9e

if platform.system() == "Windows":

It should generate Unix modifiers even if my compilation platform is Windows.

Steps to Reproduce

In a Windows machine, configure OpenMP with Cmake and then check the generated kmp_i18n_default.inc (you don't need to compile unless you want to check the crash). Observe that it has Windows printf modifers rather than the expected Unix ones for Emscripten.

For example, I use https://github.com/maniatic0/wasm-win-openmp-fix/blob/main/build.bat

call emcmake cmake ^
    -DOPENMP_STANDALONE_BUILD=ON ^
    -DOPENMP_ENABLE_OMPT_TOOLS=OFF ^
    -DOPENMP_ENABLE_LIBOMPTARGET=OFF ^
    -DLIBOMP_HAVE_OMPT_SUPPORT=OFF ^
    -DLIBOMP_OMPT_SUPPORT=OFF ^
    -DLIBOMP_OMPD_SUPPORT=OFF ^
    -DLIBOMP_USE_DEBUGGER=OFF ^
    -DLIBOMP_FORTRAN_MODULES=OFF ^
    -DLIBOMP_ENABLE_SHARED=OFF ^
    -DLIBOMP_ARCH=wasm32 ^
    -DCMAKE_INSTALL_PREFIX=%BUILD_DIR% ^
    -G "Ninja Multi-Config" ^
    -B %BUILD_DIR% ^
    -S %OPENMP_DIR%

The file is generated at %BUILD_DIR%\runtime\src\kmp_i18n_default.inc

Suggestion

I think a configuration variable like LIBOMP_ARCH could help fix the issue. Something like LIBOMP_PLATFORM=None|Unix|Windows, which could be checked in message-converter.py (if the variable is None/Empty, it should default to the platform check).

Workaround

After configuring with CMake and before compiling, fix the modifiers. This can be done with a Python script, which is what I do with https://github.com/maniatic0/wasm-win-openmp-fix/blob/main/fix_kmp_i18n_default.py at https://github.com/maniatic0/wasm-win-openmp-fix/blob/d40c146aa0e04ce17686d5ab831b474e94266d39/build.bat#L33

Crash

Not sure if this is an issue in itself, but for completeness. The way I found the above problem with the string generation was due to OOM crashes coming from __kmp_str_buf_vprint in kmp_str.cpp

llvm-project/openmp/runtime/src/kmp_str.cpp

Line 206 in 54dad9e

size = buffer->size * 2;

My comments are marked with <---

int __kmp_str_buf_vprint(kmp_str_buf_t *buffer, char const *format,
                         va_list args) {
  int rc;
  KMP_STR_BUF_INVARIANT(buffer);

  for (;;) {
    int const free = buffer->size - buffer->used;
    int size;

    // Try to format string.
    {
      /* On Linux* OS Intel(R) 64, vsnprintf() modifies args argument, so
         vsnprintf() crashes if it is called for the second time with the same
         args. To prevent the crash, we have to pass a fresh intact copy of args
         to vsnprintf() on each iteration.

         Unfortunately, standard va_copy() macro is not available on Windows*
         OS. However, it seems vsnprintf() does not modify args argument on
         Windows* OS.
      */

#if !KMP_OS_WINDOWS
      va_list _args;
      va_copy(_args, args); // Make copy of args.
#define args _args // Substitute args with its copy, _args.
#endif // KMP_OS_WINDOWS
      rc = KMP_VSNPRINTF(buffer->str + buffer->used, free, format, args); // <--- This fails with -1 (encoding issue) when used with Window's modifiers
#if !KMP_OS_WINDOWS
#undef args // Remove substitution.
      va_end(_args);
#endif // KMP_OS_WINDOWS
    }

    // No errors, string has been formatted.
    if (rc >= 0 && rc < free) {
      buffer->used += rc;
      break;
    }

    // Error occurred, buffer is too small.
    if (rc >= 0) {
      // C99-conforming implementation of vsnprintf returns required buffer size
      size = buffer->used + rc + 1;
    } else {
      // Older implementations just return -1. Double buffer size.
      size = buffer->size * 2; // <---  This treats the constraint/encoding issues as not enough buffer. This grows until crashing
    }

    // Enlarge buffer.
    __kmp_str_buf_reserve(buffer, size);

    // And try again.
  }

  KMP_DEBUG_ASSERT(buffer->size > 0);
  KMP_STR_BUF_INVARIANT(buffer);
  return rc;
} // __kmp_str_buf_vprint

KMP_VSNPRINTF is defined as vsnprintf_s at

llvm-project/openmp/runtime/src/kmp_safe_c_api.h

Line 52 in 54dad9e

#define KMP_VSNPRINTF(dst, cnt, fmt, arg) \

From the standard, the negative return value means that there was a constraint or encoding issue (which would be correct as it's using Windows modifiers rather than Unix ones). This is then treated as an unknown error that the buffer is not big enough, so it's doubled in size which then runs out of memory inside the loop. More info in my notes.

I have to note that there is a comment in the code about older implementations returning negative numbers for errors about the buffer not being big enough, so I'm not sure if this is an issue.

[1] #71297
[2] #95169
[3] https://reviews.llvm.org/D142593?id=492403

[shiltian]

Interesting findings and thanks for sharing. I don't think WASM support is widely used thus tested, even though we do support it. Feel free to post PR to fix the issues you find.

[maniatic0]

Hi @shiltian,

Thanks for your prompt response!

Yep, I could add a minimal PR to fix this, if you think it's alright.

I could add a minimal fix for this that would consist of adding a flag to message-converter.py, something like --platform-system-override="" that could accept values like https://docs.python.org/3/library/platform.html#platform.system (in case it's empty, it would default to platform.system()). Then in CMake I could set up that flag to Emscripten (as that's the sysname returned by uname there) for EMSCRIPTEN only (not sure if other WASM toolchains use different kinds of platforms).

This would be at

llvm-project/openmp/runtime/src/CMakeLists.txt

Line 32 in 4c4a413

--enum=kmp_i18n_id.inc ${LIBOMP_SRC_DIR}/i18n/en_US.txt

and

llvm-project/openmp/runtime/src/CMakeLists.txt

Line 38 in 4c4a413

--default=kmp_i18n_default.inc ${LIBOMP_SRC_DIR}/i18n/en_US.txt

This would leave the door open in case someone wants to implement the suggested LIBOMP_PLATFORM=None|Unix|Windows (but this is probably a much bigger PR).

[maniatic0]

PR created at #116874

[maniatic0]

Fixed by #116874