Set --define wasm=wasmtime for macOS by default #23235
Labels
triage
Issue requires triage
Comments
|
Seems like @keith's patch (mentioned in #20889) https://chromium.googlesource.com/v8/v8.git/+log/refs/tags/10.4.132.18/bazel/config is included in Caught on: https://chromium.googlesource.com/v8/v8.git/+/refs/tags/10.4.132.18/src/base/platform/platform-posix.cc#478 (which indicates a bug in the caller ( We expect ENOMEM (12), but got EACCES (13). The |
|
Checking on https://github.com/nodejs/node/blob/dc4398c9cfb86754f69631a9e69da4314ac0cbea/deps/v8/src/base/platform/platform-posix.cc#L468 and compare it with the current linked v8 version (10.4.132.18): https://chromium.googlesource.com/v8/v8.git/+/refs/tags/10.4.132.18/src/base/platform/platform-posix.cc#468, the later has the following code (https://chromium.googlesource.com/v8/v8.git/+/refs/tags/10.4.132.18/src/base/platform/platform-posix.cc#478, due to: v8/v8@18159ba) but the node's one doesn't have it: if (ret != 0) CHECK_EQ(ENOMEM, errno);So I tested locally to remove that line, by appending the following diff --git a/src/base/platform/platform-posix.cc b/src/base/platform/platform-posix.cc
index 131ff9614e..a43d247c8d 100644
--- a/src/base/platform/platform-posix.cc
+++ b/src/base/platform/platform-posix.cc
@@ -472,10 +472,12 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
int prot = GetProtectionFromMemoryPermission(access);
int ret = mprotect(address, size, prot);
+#if not defined(V8_OS_DARWIN)
// Any failure that's not OOM likely indicates a bug in the caller (e.g.
// using an invalid mapping) so attempt to catch that here to facilitate
// debugging of these failures.
if (ret != 0) CHECK_EQ(ENOMEM, errno);
+#endif
// MacOS 11.2 on Apple Silicon refuses to switch permissions from
// rwx to none. Just use madvise instead.I can load the Do you think updating |
|
Ah probably the right patch is to switch the place of checking (worth submitting to upstream?): diff --git a/src/base/platform/platform-posix.cc b/src/base/platform/platform-posix.cc
index 131ff9614e..8699699861 100644
--- a/src/base/platform/platform-posix.cc
+++ b/src/base/platform/platform-posix.cc
@@ -472,11 +472,6 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
int prot = GetProtectionFromMemoryPermission(access);
int ret = mprotect(address, size, prot);
- // Any failure that's not OOM likely indicates a bug in the caller (e.g.
- // using an invalid mapping) so attempt to catch that here to facilitate
- // debugging of these failures.
- if (ret != 0) CHECK_EQ(ENOMEM, errno);
-
// MacOS 11.2 on Apple Silicon refuses to switch permissions from
// rwx to none. Just use madvise instead.
#if defined(V8_OS_DARWIN)
@@ -486,6 +481,11 @@ bool OS::SetPermissions(void* address, size_t size, MemoryPermission access) {
}
#endif
+ // Any failure that's not OOM likely indicates a bug in the caller (e.g.
+ // using an invalid mapping) so attempt to catch that here to facilitate
+ // debugging of these failures.
+ if (ret != 0) CHECK_EQ(ENOMEM, errno);
+
if (ret == 0 && access == OS::MemoryPermission::kNoAccess) {
// This is advisory; ignore errors and continue execution.
USE(DiscardSystemPages(address, size)); |
|
Closed in favor of: #23243 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, since #20889 is merged, macOS binaries v1.23.x (e.g. fetched via Homebrew) by default doesn't support Wasm. Should we use
build:macos --define wasm=wasmtimeinstead? I think at least people getting envoy binary from Homebrew can play with Wasm filter too.cc. @keith @cpakulski
The text was updated successfully, but these errors were encountered: