-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libc] Add wasm support for some submodules in llvm-libc. #154383
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-libc Author: None (google-yfyang) ChangesIntroducing several emscripten-specific changes to the llvm-libc headers as well as adding a wasm sigsetjmp.cpp. Full diff: https://github.com/llvm/llvm-project/pull/154383.diff 5 Files Affected:
diff --git a/libc/src/__support/StringUtil/platform_errors.h b/libc/src/__support/StringUtil/platform_errors.h
index 32e8414b3e3de..5f83865482e72 100644
--- a/libc/src/__support/StringUtil/platform_errors.h
+++ b/libc/src/__support/StringUtil/platform_errors.h
@@ -9,7 +9,7 @@
#ifndef LLVM_LIBC_SRC___SUPPORT_STRINGUTIL_PLATFORM_ERRORS_H
#define LLVM_LIBC_SRC___SUPPORT_STRINGUTIL_PLATFORM_ERRORS_H
-#if defined(__linux__) || defined(__Fuchsia__)
+#if defined(__linux__) || defined(__Fuchsia__) || defined(__EMSCRIPTEN__)
#include "tables/linux_platform_errors.h"
#else
#include "tables/minimal_platform_errors.h"
diff --git a/libc/src/__support/StringUtil/tables/posix_errors.h b/libc/src/__support/StringUtil/tables/posix_errors.h
index b21f28f0b1321..d386a48c21d44 100644
--- a/libc/src/__support/StringUtil/tables/posix_errors.h
+++ b/libc/src/__support/StringUtil/tables/posix_errors.h
@@ -63,7 +63,12 @@ LIBC_INLINE_VAR constexpr MsgTable<76> POSIX_ERRORS = {
MsgMapping(EPROTO, "Protocol error"),
MsgMapping(EMULTIHOP, "Multihop attempted"),
MsgMapping(EBADMSG, "Bad message"),
+#ifdef __EMSCRIPTEN__
+ // For now, match the musl string
+ MsgMapping(EOVERFLOW, "Value too large for data type"),
+#else
MsgMapping(EOVERFLOW, "Value too large for defined data type"),
+#endif
MsgMapping(ENOTSOCK, "Socket operation on non-socket"),
MsgMapping(EDESTADDRREQ, "Destination address required"),
MsgMapping(EMSGSIZE, "Message too long"),
diff --git a/libc/src/__support/StringUtil/tables/stdc_errors.h b/libc/src/__support/StringUtil/tables/stdc_errors.h
index a326616f20ef5..9a23d0718ea13 100644
--- a/libc/src/__support/StringUtil/tables/stdc_errors.h
+++ b/libc/src/__support/StringUtil/tables/stdc_errors.h
@@ -16,7 +16,12 @@
namespace LIBC_NAMESPACE_DECL {
LIBC_INLINE_VAR constexpr const MsgTable<4> STDC_ERRORS = {
+#ifdef __EMSCRIPTEN__
+ // For now, match the musl name for errno 0.
+ MsgMapping(0, "No error information"),
+#else
MsgMapping(0, "Success"),
+#endif
MsgMapping(EDOM, "Numerical argument out of domain"),
MsgMapping(ERANGE, "Numerical result out of range"),
MsgMapping(EILSEQ, "Invalid or incomplete multibyte or wide character"),
diff --git a/libc/src/__support/macros/properties/architectures.h b/libc/src/__support/macros/properties/architectures.h
index ecc93196be286..21e9bc4288cd7 100644
--- a/libc/src/__support/macros/properties/architectures.h
+++ b/libc/src/__support/macros/properties/architectures.h
@@ -41,6 +41,10 @@
#define LIBC_TARGET_ARCH_IS_ARM
#endif
+#if defined(__wasm__)
+#define LIBC_TARGET_ARCH_IS_WASM
+#endif
+
#if defined(__aarch64__) || defined(__arm64__) || defined(_M_ARM64)
#define LIBC_TARGET_ARCH_IS_AARCH64
#endif
diff --git a/libc/src/setjmp/wasm/sigsetjmp.cpp b/libc/src/setjmp/wasm/sigsetjmp.cpp
new file mode 100644
index 0000000000000..809356d4577eb
--- /dev/null
+++ b/libc/src/setjmp/wasm/sigsetjmp.cpp
@@ -0,0 +1,14 @@
+#include "src/setjmp/sigsetjmp.h"
+#include "hdr/offsetof_macros.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/config.h"
+
+#if !defined(LIBC_TARGET_ARCH_IS_WASM)
+#error "Invalid file include"
+#endif
+
+namespace LIBC_NAMESPACE_DECL {
+[[gnu::returns_twice]] int sigsetjmp(jmp_buf sigjmp_buf, int savesigs) {
+ return setjmp(sigjmp_buf);
+}
+}
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- libc/src/__support/StringUtil/tables/emscripten_platform_errors.h libc/src/setjmp/wasm/sigsetjmp.cpp libc/src/__support/StringUtil/platform_errors.h libc/src/__support/StringUtil/tables/posix_errors.h libc/src/__support/StringUtil/tables/stdc_errors.h libc/src/__support/macros/properties/architectures.h View the diff from clang-format here.diff --git a/libc/src/__support/StringUtil/tables/emscripten_platform_errors.h b/libc/src/__support/StringUtil/tables/emscripten_platform_errors.h
index c9316aa7e..9e06c6f64 100644
--- a/libc/src/__support/StringUtil/tables/emscripten_platform_errors.h
+++ b/libc/src/__support/StringUtil/tables/emscripten_platform_errors.h
@@ -1,4 +1,5 @@
-//===-- Map of error numbers to strings for the Emscripten platform --*- C++ -*-===//
+//===-- Map of error numbers to strings for the Emscripten platform --*- C++
+//-*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -15,8 +16,7 @@
namespace LIBC_NAMESPACE_DECL {
-LIBC_INLINE_VAR constexpr auto PLATFORM_ERRORS =
- STDC_ERRORS + POSIX_ERRORS;
+LIBC_INLINE_VAR constexpr auto PLATFORM_ERRORS = STDC_ERRORS + POSIX_ERRORS;
LIBC_INLINE_VAR constexpr auto PLATFORM_ERRNO_NAMES =
STDC_ERRNO_NAMES + POSIX_ERRNO_NAMES;
diff --git a/libc/src/setjmp/wasm/sigsetjmp.cpp b/libc/src/setjmp/wasm/sigsetjmp.cpp
index a921108b9..abac4d3c3 100644
--- a/libc/src/setjmp/wasm/sigsetjmp.cpp
+++ b/libc/src/setjmp/wasm/sigsetjmp.cpp
@@ -17,6 +17,6 @@
namespace LIBC_NAMESPACE_DECL {
[[gnu::returns_twice]] int sigsetjmp(jmp_buf sigjmp_buf, int savesigs) {
- return setjmp(sigjmp_buf);
-}
+ return setjmp(sigjmp_buf);
}
+} // namespace LIBC_NAMESPACE_DECL
|
@@ -16,7 +16,12 @@ | |||
namespace LIBC_NAMESPACE_DECL { | |||
|
|||
LIBC_INLINE_VAR constexpr const MsgTable<4> STDC_ERRORS = { | |||
#ifdef __EMSCRIPTEN__ | |||
// For now, match the musl name for errno 0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the intention for emscripten to use these other error strings long-term or do you intend to switch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that we will clean this up once we can significant real users of llvm-libc on emscripten. @sbc100 probably knows about it more than I do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it depends on the timeline. We have unit tests that currently depend on the explict strings used in musl. We have a new options if we want to remove this patch:
- Update the unit tests to accept both string options
- Update our musl fork to use the llvm-libc strings
- Remove our support for musl completely and switch the tests to expect llvm-libc strings.
BTW, where did these specific string come from? Is there a reason to prefer "No error information" over "Success"? I suppose the former is technically more accurate but the later might be more inline with user expectations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaning towards (2) since we already maintain a musl fork anyway and have no plans to upstream it (unlike our llvm-libc changed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. That sounds reasonable. But I presume that no users are going to be affected by this musl change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only users whose code is sensitive the these exact strings like our unit tests are. Seems unlikely, but conceivable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the existing strings in LLVM-libc are meant to match glibc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying to land the change to update the musl fork in emscripten first so that we no longer need these lines.
@@ -0,0 +1,26 @@ | |||
//===-- Map of error numbers to strings for the Emscripten platform --*- C++ -*-===// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line is over 80 columns, meaning it's going to get messed up by auto-formatters. Please make it exactly 80 columns
Introducing several emscripten-specific changes to the llvm-libc headers as well as adding a wasm sigsetjmp.cpp.