Skip to content

Conditionalize use of POSIX features missing on WASI/WebAssembly #92677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1578,7 +1578,7 @@ bool Driver::getCrashDiagnosticFile(StringRef ReproCrashFilename,
CrashDiagDir = "/";
path::append(CrashDiagDir, "Library/Logs/DiagnosticReports");
int PID =
#if LLVM_ON_UNIX
#if LLVM_ON_UNIX && !defined(__wasi__)
getpid();
#else
0;
Expand Down
5 changes: 4 additions & 1 deletion lld/Common/Filesystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,11 @@ void lld::unlinkAsync(StringRef path) {
if (!sys::fs::exists(path) || !sys::fs::is_regular_file(path))
return;

// If threads are disabled, remove the file synchronously.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's already a check below,

  if (parallel::strategy.ThreadsRequested == 1)
    return;

Probably that should be a constant in no-threads builds, then maybe this change isn't needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, I don't quite understand. Are you asking me to change ThreadPoolStrategy::ThreadsRequested to be a constant? I could do that but it seems like an invasive change. Looking at other uses of ThreadsRequested, it looks like many of them are guarded in essentially the same way as what this patch introduces, e.g. here:

#if LLVM_ENABLE_THREADS
: Parallel((parallel::strategy.ThreadsRequested != 1) &&
(threadIndex == UINT_MAX)) {}
#else
: Parallel(false) {}
#endif

or here:

void llvm::parallelFor(size_t Begin, size_t End,
llvm::function_ref<void(size_t)> Fn) {
#if LLVM_ENABLE_THREADS
if (parallel::strategy.ThreadsRequested != 1) {

#if !LLVM_ENABLE_THREADS
sys::fs::remove(path);
// Removing a file is async on windows.
#if defined(_WIN32)
#elif defined(_WIN32)
// On Windows co-operative programs can be expected to open LLD's
// output in FILE_SHARE_DELETE mode. This allows us to delete the
// file (by moving it to a temporary filename and then deleting
Expand Down
1 change: 1 addition & 0 deletions llvm/cmake/config-ix.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ check_symbol_exists(getrlimit "sys/types.h;sys/time.h;sys/resource.h" HAVE_GETRL
check_symbol_exists(posix_spawn spawn.h HAVE_POSIX_SPAWN)
check_symbol_exists(pread unistd.h HAVE_PREAD)
check_symbol_exists(sbrk unistd.h HAVE_SBRK)
check_symbol_exists(setjmp setjmp.h HAVE_SETJMP)
check_symbol_exists(strerror_r string.h HAVE_STRERROR_R)
check_symbol_exists(strerror_s string.h HAVE_DECL_STRERROR_S)
check_symbol_exists(setenv stdlib.h HAVE_SETENV)
Expand Down
4 changes: 4 additions & 0 deletions llvm/cmake/modules/HandleLLVMOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,10 @@ elseif(FUCHSIA OR UNIX)
else()
set(LLVM_HAVE_LINK_VERSION_SCRIPT 1)
endif()
elseif(WASI)
set(LLVM_ON_WIN32 0)
set(LLVM_ON_UNIX 1)
set(LLVM_HAVE_LINK_VERSION_SCRIPT 0)
elseif(CMAKE_SYSTEM_NAME STREQUAL "Generic")
set(LLVM_ON_WIN32 0)
set(LLVM_ON_UNIX 0)
Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/ADT/bit.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

#if defined(__linux__) || defined(__GNU__) || defined(__HAIKU__) || \
defined(__Fuchsia__) || defined(__EMSCRIPTEN__) || defined(__NetBSD__) || \
defined(__OpenBSD__) || defined(__DragonFly__)
defined(__OpenBSD__) || defined(__DragonFly__) || defined(__wasm__)
#include <endian.h>
#elif defined(_AIX)
#include <sys/machine.h>
Expand Down
4 changes: 4 additions & 0 deletions llvm/include/llvm/Config/config.h.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@
/* Define to 1 if you have the `sbrk' function. */
#cmakedefine HAVE_SBRK ${HAVE_SBRK}

/* Define to 1 if you have the `setjmp' function. */
/* This function is expected to be present everywhere except for a subset of WebAssembly builds. */
#cmakedefine HAVE_SETJMP ${HAVE_SETJMP}

/* Define to 1 if you have the `setenv' function. */
#cmakedefine HAVE_SETENV ${HAVE_SETENV}

Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/ExecutionEngine/Interpreter/ExternalFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
#include "llvm/Support/raw_ostream.h"
#include <cassert>
#include <cmath>
#if !defined(__wasi__)
#include <csignal>
#endif
#include <cstdint>
#include <cstdio>
#include <cstring>
Expand Down Expand Up @@ -340,7 +342,11 @@ static GenericValue lle_X_exit(FunctionType *FT, ArrayRef<GenericValue> Args) {
static GenericValue lle_X_abort(FunctionType *FT, ArrayRef<GenericValue> Args) {
//FIXME: should we report or raise here?
//report_fatal_error("Interpreted program raised SIGABRT");
#if defined(__wasi__)
abort();
#else
raise (SIGABRT);
#endif
return GenericValue();
}

Expand Down
40 changes: 36 additions & 4 deletions llvm/lib/Support/CrashRecoveryContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,17 @@
#include "llvm/Support/Signals.h"
#include "llvm/Support/thread.h"
#include <cassert>
#if !defined(__wasi__)
#include <csignal>
#endif
#if LLVM_ENABLE_THREADS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunate; there's no reason (in theory) that mutex shouldn't work fine in single-threaded mode.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it actually does work, since I ended up building LLVM always with the wasm32-wasip1-threads target, and this is a remnant of an earlier patch.

#include <mutex>
#endif
#if HAVE_SETJMP
// We can rely on setjmp to exist everywhere except for a subset of WebAssembly
// builds.
#include <setjmp.h>
#endif

using namespace llvm;

Expand All @@ -31,7 +40,9 @@ struct CrashRecoveryContextImpl {
const CrashRecoveryContextImpl *Next;

CrashRecoveryContext *CRC;
#ifdef HAVE_SETJMP
::jmp_buf JumpBuffer;
#endif
volatile unsigned Failed : 1;
unsigned SwitchedThread : 1;
unsigned ValidJumpBuffer : 1;
Expand All @@ -50,7 +61,7 @@ struct CrashRecoveryContextImpl {
/// Called when the separate crash-recovery thread was finished, to
/// indicate that we don't need to clear the thread-local CurrentContext.
void setSwitchedThread() {
#if defined(LLVM_ENABLE_THREADS) && LLVM_ENABLE_THREADS != 0
#if LLVM_ENABLE_THREADS
SwitchedThread = true;
#endif
}
Expand All @@ -72,19 +83,23 @@ struct CrashRecoveryContextImpl {

CRC->RetCode = RetCode;

#if HAVE_SETJMP
// Jump back to the RunSafely we were called under.
if (ValidJumpBuffer)
longjmp(JumpBuffer, 1);
#endif

// Otherwise let the caller decide of the outcome of the crash. Currently
// this occurs when using SEH on Windows with MSVC or clang-cl.
}
};

std::mutex &getCrashRecoveryContextMutex() {
#if LLVM_ENABLE_THREADS
static std::mutex &getCrashRecoveryContextMutex() {
static std::mutex CrashRecoveryContextMutex;
return CrashRecoveryContextMutex;
}
#endif

static bool gCrashRecoveryEnabled = false;

Expand Down Expand Up @@ -138,7 +153,9 @@ CrashRecoveryContext *CrashRecoveryContext::GetCurrent() {
}

void CrashRecoveryContext::Enable() {
#if LLVM_ENABLE_THREADS
std::lock_guard<std::mutex> L(getCrashRecoveryContextMutex());
#endif
// FIXME: Shouldn't this be a refcount or something?
if (gCrashRecoveryEnabled)
return;
Expand All @@ -147,7 +164,9 @@ void CrashRecoveryContext::Enable() {
}

void CrashRecoveryContext::Disable() {
#if LLVM_ENABLE_THREADS
std::lock_guard<std::mutex> L(getCrashRecoveryContextMutex());
#endif
if (!gCrashRecoveryEnabled)
return;
gCrashRecoveryEnabled = false;
Expand Down Expand Up @@ -329,7 +348,16 @@ static void uninstallExceptionOrSignalHandlers() {
}
}

#else // !_WIN32
#elif defined(__wasi__)

// WASI implementation.
//
// WASI traps are always fatal, and recovery is not possible. Do nothing.

static void installExceptionOrSignalHandlers() {}
static void uninstallExceptionOrSignalHandlers() {}

#else // !_WIN32 && !__wasi__

// Generic POSIX implementation.
//
Expand Down Expand Up @@ -417,10 +445,12 @@ bool CrashRecoveryContext::RunSafely(function_ref<void()> Fn) {
CrashRecoveryContextImpl *CRCI = new CrashRecoveryContextImpl(this);
Impl = CRCI;

#if HAVE_SETJMP
CRCI->ValidJumpBuffer = true;
if (setjmp(CRCI->JumpBuffer) != 0) {
return false;
}
#endif
}

Fn();
Expand Down Expand Up @@ -467,7 +497,9 @@ bool CrashRecoveryContext::isCrash(int RetCode) {
bool CrashRecoveryContext::throwIfCrash(int RetCode) {
if (!isCrash(RetCode))
return false;
#if defined(_WIN32)
#if defined(__wasi__)
abort();
#elif defined(_WIN32)
::RaiseException(RetCode, 0, 0, NULL);
#else
llvm::sys::unregisterHandlers();
Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Support/InitLLVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
// Bring stdin/stdout/stderr into a known state.
sys::AddSignalHandler(CleanupStdHandles, nullptr);
#endif
#if !defined(__wasi__)
if (InstallPipeSignalExitHandler)
// The pipe signal handler must be installed before any other handlers are
// registered. This is because the Unix \ref RegisterHandlers function does
Expand All @@ -59,6 +60,7 @@ InitLLVM::InitLLVM(int &Argc, const char **&Argv,
StackPrinter.emplace(Argc, Argv);
sys::PrintStackTraceOnErrorSignal(Argv[0]);
install_out_of_memory_new_handler();
#endif

#ifdef __MVS__

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Support/LockFileManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ static std::error_code getHostID(SmallVectorImpl<char> &HostID) {
StringRef UUIDRef(UUIDStr);
HostID.append(UUIDRef.begin(), UUIDRef.end());

#elif LLVM_ON_UNIX
#elif !defined(__wasi__)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're changing behavior for !LLVM_ON_UNIX (like Windows) to use this branch now. That seems probably unintended. Meant #elif LLVM_ON_UNIX && !defined(__wasi__)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is the cause of buildbot failures that I missed during my own review.

char HostName[256];
HostName[255] = 0;
HostName[0] = 0;
Expand All @@ -111,7 +111,7 @@ static std::error_code getHostID(SmallVectorImpl<char> &HostID) {
}

bool LockFileManager::processStillExecuting(StringRef HostID, int PID) {
#if LLVM_ON_UNIX && !defined(__ANDROID__)
#if LLVM_ON_UNIX && !defined(__ANDROID__) && !defined(__wasi__)
SmallString<256> StoredHostID;
if (getHostID(StoredHostID))
return true; // Conservatively assume it's executing on error.
Expand Down
16 changes: 13 additions & 3 deletions llvm/lib/Support/Signals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,19 @@ static bool printMarkupStackTrace(StringRef Argv0, void **StackTrace, int Depth,
}

// Include the platform-specific parts of this class.
#ifdef LLVM_ON_UNIX
#if defined(__wasi__)
// WASI does not have signals.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move these stubs to WASI/Signals.inc for consistency with the other platform support files.

void llvm::sys::AddSignalHandler(sys::SignalHandlerCallback FnPtr,
void *Cookie) {}
void llvm::sys::RunInterruptHandlers() {}
void sys::CleanupOnSignal(uintptr_t Context) {}
bool llvm::sys::RemoveFileOnSignal(StringRef Filename, std::string *ErrMsg) {
return false;
}
void llvm::sys::DontRemoveFileOnSignal(StringRef Filename) {}
void llvm::sys::DisableSystemDialogsOnCrash() {}
#elif defined(LLVM_ON_UNIX)
#include "Unix/Signals.inc"
#endif
#ifdef _WIN32
#elif defined(_WIN32)
#include "Windows/Signals.inc"
#endif
5 changes: 5 additions & 0 deletions llvm/lib/Support/Unix/Memory.inc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ std::error_code Memory::releaseMappedMemory(MemoryBlock &M) {

std::error_code Memory::protectMappedMemory(const MemoryBlock &M,
unsigned Flags) {
#if defined(__wasi__)
// Wasm does not allow making memory read-only or executable.
return std::error_code(ENOSYS, std::generic_category());
#endif

static const Align PageSize = Align(Process::getPageSizeEstimate());
if (M.Address == nullptr || M.AllocatedSize == 0)
return std::error_code();
Expand Down
Loading
Loading