Skip to content
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

Disable strict aliasing #315

Merged
merged 1 commit into from
Sep 17, 2024
Merged

Conversation

bnoordhuis
Copy link
Collaborator

@bnoordhuis bnoordhuis commented Sep 17, 2024

Harmonize C++ compiler flags with Node.js (-fno-rtti, -fno-exceptions) and, critically, disable strict aliasing, because V8 is not the least bit strict-aliasing safe.

Upstream turned it off in nodejs/node#54339, like it always should have been, and I suspect the mismatch may be the cause of segfaults that show up in production.

Remove -fpermissive, it's not needed and something to discourage.

@bnoordhuis
Copy link
Collaborator Author

macos failures:

curl: (28) Failed to connect to nodejs.org port 443 after 150033 ms: Couldn't connect to server

i.e., network outage. It's trying to build libv8-node from source, is that expected?

musl failures:

/usr/lib/gcc/aarch64-alpine-linux-musl/13.2.1/../../../../aarch64-alpine-linux-musl/bin/ld: cannot find /usr/local/bundle/gems/libv8-node-22.7.0.4-aarch64-linux/vendor/v8/aarch64-linux-musl/libv8/obj/libv8_monolith.a: No such file or directory

Pre-existing, same build error on main. Started with commit 9ef1982, I think.

Harmonize C++ compiler flags with Node.js (-fno-rtti, -fno-exceptions)
and, critically, disable strict aliasing, because V8 is not the least
bit strict-aliasing safe.

Upstream turned it off in nodejs/node#54339,
like it always should have been, and I suspect the mismatch may be the
cause of segfaults that show up in production.
@tisba
Copy link
Collaborator

tisba commented Sep 17, 2024

musl error is most likely because of rubyjs/libv8-node#60

@SamSaffron SamSaffron merged commit 5b9d19e into rubyjs:main Sep 17, 2024
22 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants