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.

Sign up for GitHub

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

Jump to bottom

switch ‘-msign-return-address=all’ is no longer supported #3319

Closed
targos opened this issue Apr 21, 2023 · 2 comments · Fixed by nodejs/node#51256
Closed

switch ‘-msign-return-address=all’ is no longer supported #3319

targos opened this issue Apr 21, 2023 · 2 comments · Fixed by nodejs/node#51256
Labels
ci-release platform:arm

Comments

@targos
Copy link
Member

targos commented Apr 21, 2023

https://ci-release.nodejs.org/job/iojs+release/9304/nodes=rhel8-arm64-release/console

I wonder if it means that we build without this protection at the moment.

Related to nodejs/node#42888 and nodejs/node#45756
@richardlau
Copy link
Member

richardlau commented Apr 21, 2023 •
edited
Loading

We could change it to -mbranch-protection -- it will break anything using gcc 8 to compile but for Node.js 20 we bumped the requirement to gcc 10. Would probably need #3317 to be addressed for the Ubuntu arm64 hosts.

@richardlau
Copy link
Member

I did think about altering configure.py to choose the appropriate flag based on gcc version (since configure.py does check that) but nodejs/node#45756 would move setting the flag out of configure.py into the gyp files.

@watson watson mentioned this issue Jun 14, 2023
Closed
@targos targos mentioned this issue Dec 22, 2023
Merged
nodejs-github-bot pushed a commit to nodejs/node that referenced this issue Dec 27, 2023
@targos
build: fix arm64 cross-compilation
c21b2be 
Commit 938212f added -msign-return-address=all to _all_ cflags but that
is wrong when cross-compiling, it should only be added to the target's
cflags.

The flag being deprecated, it is also changed to
`-mbranch-protection=standard`.

Fixes: #42888
Co-Authored-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@targos targos closed this as completed Dec 27, 2023
RafaelGSS pushed a commit to nodejs/node that referenced this issue Jan 2, 2024
@targos @RafaelGSS
build: fix arm64 cross-compilation
e03ac83 
Commit 938212f added -msign-return-address=all to _all_ cflags but that
is wrong when cross-compiling, it should only be added to the target's
cflags.

The flag being deprecated, it is also changed to
`-mbranch-protection=standard`.

Fixes: #42888
Co-Authored-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
richardlau pushed a commit to nodejs/node that referenced this issue Mar 25, 2024
@targos @richardlau
build: fix arm64 cross-compilation
27f403c 
Commit 938212f added -msign-return-address=all to _all_ cflags but that
is wrong when cross-compiling, it should only be added to the target's
cflags.

The flag being deprecated, it is also changed to
`-mbranch-protection=standard`.

Fixes: #42888
Co-Authored-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
marco-ippolito pushed a commit to nodejs/node that referenced this issue May 2, 2024
@targos @marco-ippolito
build: fix arm64 cross-compilation
297368a 
Commit 938212f added -msign-return-address=all to _all_ cflags but that
is wrong when cross-compiling, it should only be added to the target's
cflags.

The flag being deprecated, it is also changed to
`-mbranch-protection=standard`.

Fixes: #42888
Co-Authored-By: Michaël Zasso <targos@protonmail.com>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ci-release platform:arm
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

build: fix arm64 cross-compilation targos/node
2 participants
@targos @richardlau