-
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
//===-- 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. | ||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
#ifndef LLVM_LIBC_SRC___SUPPORT_STRINGUTIL_TABLES_EMSCRPITEN_PLATFORM_ERRORS_H | ||
#define LLVM_LIBC_SRC___SUPPORT_STRINGUTIL_TABLES_EMSCRPITEN_PLATFORM_ERRORS_H | ||
|
||
#include "posix_errors.h" | ||
#include "src/__support/macros/config.h" | ||
#include "stdc_errors.h" | ||
|
||
namespace LIBC_NAMESPACE_DECL { | ||
|
||
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; | ||
|
||
} // namespace LIBC_NAMESPACE_DECL | ||
|
||
#endif // LLVM_LIBC_SRC___SUPPORT_STRINGUTIL_TABLES_EMSCRPITEN_PLATFORM_ERRORS_H |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 commentThe 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 commentThe 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 commentThe 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:
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 commentThe 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 commentThe 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 commentThe 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 commentThe 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 commentThe 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. |
||
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"), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
//===-- Implementation of sigsetjmp ---------------------------------------===// | ||
// | ||
// 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/setjmp/sigsetjmp.h" | ||
google-yfyang marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#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); | ||
} | ||
} |
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