GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — build: fix arm64 cross-compilation by bnoordhuis · Pull Request #45756 · nodejs/node

Conversation

@bnoordhuis

@bnoordhuis bnoordhuis commented Dec 6, 2022

Copy link
Copy Markdown
Member

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.

Fixes: #42888

cc @nxhack @josh-hemphill - can you confirm it works for you?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
wenerme, nikolaik, and staltz reacted with thumbs up emoji bricss, baparham, and wenerme reacted with eyes emoji
All reactions
  • 👍 3 reactions
  • 👀 3 reactions
@bnoordhuis
build: fix arm64 cross-compilation
6bdf9dc 
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.

Fixes: nodejs#42888
@nodejs-github-bot

nodejs-github-bot commented Dec 6, 2022

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/gyp
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run. labels Dec 6, 2022
@bnoordhuis bnoordhuis mentioned this pull request Dec 6, 2022
Enable pointer authentication on ARM64 #42888
Closed
@josh-hemphill

josh-hemphill commented Dec 6, 2022

Copy link
Copy Markdown

I haven't finished building yet, but looks like I just get a whole lot of warnings that switch '-msign-return-address=all' is no longer supported
I think it will build though, so I think it's working.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented Dec 6, 2022

Copy link
Copy Markdown
Member

FWIW V8 used to pass -mbranch-protection=standard (we also do it in v8_gypfiles/toolchain.gypi), but removed it in https://chromium-review.googlesource.com/c/v8/v8/+/3829068 (not sure why).
nxhack reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau

richardlau commented Dec 6, 2022

Copy link
Copy Markdown
Member

I haven't finished building yet, but looks like I just get a whole lot of warnings that switch '-msign-return-address=all' is no longer supported I think it will build though, so I think it's working.

You're on a newer version of gcc. We're currently using gcc 8 in the CI and to build the release binaries. gcc 9 introduced a replacement option, -mbranch-protection and deprecated the old one.
josh-hemphill, gengjiawen, nxhack, and gudmundur reacted with thumbs up emoji
All reactions
  • 👍 4 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@josh-hemphill

josh-hemphill commented Dec 7, 2022

Copy link
Copy Markdown

I ran into other dependency issues when using gcc 8, which I could probably get around, but I'm working with docker containers and want to limit to a single gcc version installed if I can to keep the image size down.
It seems to build just fine with gcc 9 if I sed replace the flag.
Anyone know if there's other gotchas using gcc 9? Just want to make sure I'm not sabotaging myself doing that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nxhack

nxhack commented Dec 8, 2022

Copy link
Copy Markdown

@bnoordhuis

My gcc cross-compiler version is 11.3.0, so I tested with
flag with branch-protection=standard.

It builds and runs without any problems.

However, I am concerned about the v8 upstream fix.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented Dec 8, 2022

Copy link
Copy Markdown
Member Author

It's part of Chromium's (and therefore V8's) push towards CFI (Control Flow Integrity.)

The design doc is here if you're interested: https://docs.google.com/document/d/1O2jwK4dxI3nRcOJuPYkonhTkNQfbmwdvxQMyXgeaRHo/edit

I'm going to guess they view arm64 branch protection as a special case that's been subsumed by the (more general and cross-platform) CFI work.
gengjiawen and wenerme reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau richardlau mentioned this pull request Feb 20, 2023
add recipe to compile arm64-musl version nodejs/unofficial-builds#59
Merged
@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Feb 20, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Feb 20, 2023
@nodejs-github-bot

nodejs-github-bot commented Feb 20, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/49783/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
This was referenced Feb 21, 2023
CI Reliability 2023-02-21 nodejs/reliability#502
Open
CI Reliability 2023-02-22 nodejs/reliability#503
Open
@nxhack nxhack mentioned this pull request Feb 22, 2023
node: cross-compile build issue openwrt/packages#20549
Closed
@richardlau richardlau mentioned this pull request Feb 24, 2023
build: enable pointer authentication on arm64 for branch protection #43200
Merged
baparham added a commit to baparham/pkg-fetch that referenced this pull request Feb 24, 2023
@baparham
patches: add cross compiling fix to node 19
04bf044 
based on suggested change from nodejs/node#45756
baparham added a commit to baparham/pkg-fetch that referenced this pull request Mar 8, 2023
@baparham
patches: add cross compiling fix to node 19
139aa8f 
based on suggested change from nodejs/node#45756
baparham added a commit to baparham/pkg-fetch that referenced this pull request Mar 8, 2023
@baparham
patches: add cross compiling fix to node 19
1c43dd6 
based on suggested change from nodejs/node#45756
@baparham baparham mentioned this pull request Mar 8, 2023
Revive cross compile and bump patches to latest vercel/pkg-fetch#280
Merged
3 tasks done
@PeterBurner

PeterBurner commented Mar 14, 2023

Copy link
Copy Markdown

any news on this?
nikolaik reacted with thumbs up emoji p0fi reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
baparham added a commit to vercel/pkg-fetch that referenced this pull request Mar 15, 2023
@baparham
patches: add cross compiling fix to node 19
61b0b1c 
based on suggested change from nodejs/node#45756
@targos targos mentioned this pull request Apr 21, 2023
switch ‘-msign-return-address=all’ is no longer supported nodejs/build#3319
Closed
This was referenced Apr 30, 2023
arm64 Support! nikolaik/docker-python-nodejs#38
Closed
arm64 on alpine nikolaik/docker-python-nodejs#70
Open
theofficialgman added a commit to theofficialgman/unofficial-builds that referenced this pull request Jul 18, 2023
@theofficialgman
add patch until upstream arm64 cross compilation is fixed
87a44f5 
see nodejs/node#45756
theofficialgman added a commit to theofficialgman/unofficial-builds that referenced this pull request Aug 7, 2023
@theofficialgman
add patch until upstream arm64 cross compilation is fixed
9583ed0 
see nodejs/node#45756
theofficialgman added a commit to theofficialgman/unofficial-builds that referenced this pull request Aug 7, 2023
@theofficialgman
add patch until upstream arm64 cross compilation is fixed
c45cc02 
see nodejs/node#45756
staltz added a commit to nodejs-mobile/nodejs-mobile that referenced this pull request Sep 12, 2023
@staltz
android,build: dont set a certain cflag globally
bcb194c 
This fixes "msign-return-address=all" unrecognized by g++ for obj.host
Basically copied from nodejs/node#45756
staltz added a commit to nodejs-mobile/nodejs-mobile that referenced this pull request Sep 13, 2023
@staltz
android,build: dont set a certain cflag globally
9e6c250 
This fixes "msign-return-address=all" unrecognized by g++ for obj.host
Basically copied from nodejs/node#45756
@aduh95

aduh95 commented Sep 20, 2023

Copy link
Copy Markdown
Contributor

Ping @nodejs/build
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sxa
Merge branch 'main' into fix42888
30afee8
@nodejs-github-bot

nodejs-github-bot commented Sep 20, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54083/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Sep 20, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/54084/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sxa

sxa commented Sep 20, 2023

Copy link
Copy Markdown
Member

Resolved the obvious merge conflict but looks like a little more remedial work will be needed before this can be progressed.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot mentioned this pull request Sep 21, 2023
CI Reliability 2023-09-21 nodejs/reliability#672
Open
22 tasks
staltz added a commit to nodejs-mobile/nodejs-mobile that referenced this pull request Oct 3, 2023
@staltz
android,build: dont set a certain cflag globally
650062b 
This fixes "msign-return-address=all" unrecognized by g++ for obj.host
Basically copied from nodejs/node#45756
staltz added a commit to nodejs-mobile/nodejs-mobile that referenced this pull request Oct 12, 2023
@staltz
android,build: dont set a certain cflag globally
7d6236e 
This fixes "msign-return-address=all" unrecognized by g++ for obj.host
Basically copied from nodejs/node#45756
@robertsLando robertsLando mentioned this pull request Dec 4, 2023
feat: support for latest NodeJS v20 yao-pkg/pkg-fetch#11
Merged
@robertsLando

robertsLando commented Dec 4, 2023

Copy link
Copy Markdown
Contributor

News on this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos targos mentioned this pull request Dec 22, 2023
build: fix arm64 cross-compilation #51256
Merged
@nikolaik

nikolaik commented Dec 28, 2023

Copy link
Copy Markdown

This seems to have been fixed in #51256 or am I misunderstanding?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented Dec 29, 2023

Copy link
Copy Markdown
Member

Correct. Superseded by #51256.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos targos closed this Dec 29, 2023
@robertsLando robertsLando mentioned this pull request Jan 3, 2024
fix: patches for arm64 yao-pkg/pkg-fetch#15
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable pointer authentication on ARM64

Uh oh!

There was an error while loading. Please reload this page.

12 participants

@bnoordhuis @nodejs-github-bot @josh-hemphill @targos @richardlau @nxhack @PeterBurner @aduh95 @sxa @robertsLando @nikolaik @mcollina
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.