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

build: fix arm64 cross-compilation #51256

Merged
merged 1 commit into from
Dec 27, 2023

Conversation

targos
Copy link
Member

@targos targos commented Dec 22, 2023

Alternative to #45756 (@bnoordhuis)

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
Fixes: nodejs/build#3319

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: nodejs#42888
Co-Authored-By: Michaël Zasso <[email protected]>
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels Dec 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@targos targos added the dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. label Dec 22, 2023
@nodejs-github-bot
Copy link
Collaborator

@lpinca lpinca added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 27, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 27, 2023
@nodejs-github-bot nodejs-github-bot merged commit c21b2be into nodejs:main Dec 27, 2023
58 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in c21b2be

RafaelGSS pushed a commit that referenced this pull request Jan 2, 2024
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 <[email protected]>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Jan 2, 2024
robertsLando added a commit to yao-pkg/pkg-fetch that referenced this pull request Jan 3, 2024
richardlau pushed a commit that referenced this pull request Mar 25, 2024
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 <[email protected]>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@spmiller
Copy link

I've tried cross compiling node 21.7.2, which has this patch, on x86_64 targetting aarch64. The build fails:

g++ -MMD -MF obj.host/tools/node_js2c.js2c.o.d -D_GLIBCXX_USE_CXX11_ABI=1 -DNODE_OPENSSL_CONF_NAME=nodejs_conf -DNODE_OPENSSL_HAS_QUIC -DICU_NO_USER_DATA_OVERRIDE -D__STDC_FORMAT_MACROS -DOPENSSL_NO_PINSHARED -DOPENSSL_THREADS -DNODE_JS2C_USE_STRING_LITERALS -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_POSIX_C_SOURCE=200112 -I../../tools -I../../src -I../../deps/simdutf -I../../deps/uv/include -Wall -Wextra -Wno-unused-parameter -mbranch-protection=standard -pthread -Wall -Wextra -Wno-unused-parameter -m64 -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++17  -c ../../tools/js2c.cc -o obj.host/tools/node_js2c.js2c.o
g++: error: unrecognized command-line option ‘-mbranch-protection=standard’

If I remove the node.gyp changes it succeeds, presumably without any branch protection.

@richardlau
Copy link
Member

g++: error: unrecognized command-line option ‘-mbranch-protection=standard’

This means you are using a compiler earlier than gcc 9 -- -mbranch-protection was added to gcc 9 and later.

@targos targos deleted the fix-arm64-pointer branch April 12, 2024 11:46
@spmiller
Copy link

Hi @richardlau, thanks for looking at this.

This means you are using a compiler earlier than gcc 9

I thought this too, but I double-checked and I'd built GCC13. It seems like the build is trying to use the host gcc (it's passing -m64 rather than e.g. armv8-a) which why I'm surprised it's also trying to pass the aarch64 options too.

@richardlau
Copy link
Member

Do you have CXX (pointing to the cross compiler) and CXX.host (host compiler) environment variables set?

@spmiller
Copy link

Hi @richardlau, I was using:

CC=aarch64-linux-gcc CXX=aarch64-linux-g++ CC_host="gcc" CXX_host="g++" ./configure --dest-os linux --dest-cpu arm64
make -j16

@richardlau
Copy link
Member

@spmiller See if #52559 fixes the cross compilation for you.

@devraymondsh
Copy link
Contributor

I was able to cross-compile for ARM64 on my x64 machine with #52559. Here are the commands I used:

# Setting up the env
export CC_host="gcc"
export CXX_host="g++"
export CC="aarch64-linux-gnu-gcc"
export CXX="aarch64-linux-gnu-g++"
export AS="aarch64-linux-gnu-as"
export LD="aarch64-linux-gnu-ld"
export RANLIB="aarch64-linux-gnu-ranlib"

# Building as a shared library also works with --shared
./configure --dest-cpu arm64 --dest-os linux

# Build it
make -j12

@spmiller
Copy link

Thanks a lot @richardlau and @devraymondsh, I got a successful cross-compilation with those patches too.

marco-ippolito pushed a commit that referenced this pull request May 2, 2024
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 <[email protected]>
PR-URL: #51256
Fixes: nodejs/build#3319
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

switch ‘-msign-return-address=all’ is no longer supported Enable pointer authentication on ARM64
7 participants