-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Node.js 10.1.0 build broken on ppc64[le] #20642
Comments
This might have been “fixed” by v8/v8@7f90312 because that simply removes the DCHECK. @nodejs/v8 |
Yeah, that patch gets quite a bit further. However, it fails further on while trying to build the debugging node with:
(This failure also occurs on s390x) |
@nodejs/platform-ppc @nodejs/platform-s390 |
Hello, which v8 branch/hash are you using? Can V8 be built on your environment? |
The other key question is which compiler level and binutils levels are you using? @jBarz would it be possible for you to find a fedora system to see if you can replicate with the required compiler level. Of note the compiler level requirement was increased for 10.x (actually it was increased for earlier release as well but we only move the community machines up for 10.x) but it may only be more recent levels of v8 that have introduced dependencies. |
This specific one was run with GCC 8.1 and binutils 2.30. I can't build v8 separately from Node.js at the moment as I don't have any build scripts for it that I can use for our buildsystem (since I don't have direct access to the builders for those arches). @jBarz I can help you get access to https://fedoraproject.org/wiki/Test_Machine_Resources_For_Package_Maintainers for a PPC64 or PPC64le machine (I'll get one of them updated to F28 if that's the approach we want to go). For the time being, I've just disabled the debug build on our packages on those arches; it'll have only the release binary |
Ok, those compiler/binutils levels are higher than the ones used in the community so we'd expect it to be ok. Thinking about it more I see its only the debug one that is failing and we don't build that in the community. |
@sgallagher Can we get access to a ppc64le fedora? |
@jBarz Yes, I've got someone working on getting you access to a public test machine. He will reply on this issue as soon as it's ready. Sorry for the delay. |
@sgallagher ping |
Pong. This isn’t forgotten, but it’s taking me longer than anticipated to get you access to a machine. I’m out of the office today, but I’ll look into things tomorrow. Apologies on the delay. |
@mhdawson @jBarz If you could please create a Fedora account at https://admin.fedoraproject.org/accounts and sign the FPCA then I can give you access to one of our test machines. Please reply (or direct email me at sgallagh (at) redhat (dot) com) with the account(s) you created and I'll have them added to the ACL for the machine. (Note: the FPCA does not impact copyright ownership, it is just an assertion that if you make a contribution, you assert that you have the rights to make that contribution and that the contribution is being made under an acceptable open-source license). |
@jBarz any update on this? |
Had some trouble setting up the machine but all set now. |
Sorry about the delay, just had some time to investigate. |
@jBarz so it sounds like it is more an issue with building in debug mode versus the specific OS/compiler? |
Yes I believe it is. |
After some discussions with a chromium developer, the following fix was merged for PPC. I have to submit one more fix myself before this issue is resolved. |
Sounds good, I assume we'll have to float that on 10.X |
One more fix needs to be submitted to v8 because of 64K page size on linux ppc64. --- a/src/base/platform/platform-posix.cc
+++ b/src/base/platform/platform-posix.cc
@@ -252,7 +252,7 @@ void* OS::GetRandomMmapAddr() {
raw_addr &= uint64_t{0x03FFFFFFF000};
#else
// Little-endian Linux: 48 bits of virtual addressing.
- raw_addr &= uint64_t{0x3FFFFFFFF000};
+ raw_addr &= uint64_t{0x3FFFFFFF0000};
#endif
#elif V8_TARGET_ARCH_MIPS64
// We allocate code in 256 MB aligned segments because of optimizations using |
https://chromium-review.googlesource.com/c/v8/v8/+/1194583 fixes alignment issue |
@sgallagher this was fixed in 10.12.0. Can you validate and then close this issue if you agree? |
@mhdawson I've got it on my plate to build 10.13.0 and 11.0.0 for Fedora today or tomorrow; I'll make sure to validate this when I do. I didn't realize it was in 10.12.0 or I'd have dropped my workaround then. |
@mhdawson Looks like it's still an issue, I'm afraid.
|
Full logs for the above can be found at https://koji.fedoraproject.org/koji/taskinfo?taskID=30596168 |
@john-yan can you take another look. |
@sgallagher Hello I have signup a username (johnyan) under https://admin.fedoraproject.org/accounts. Would you give me access to one of your systems please? Thanks. |
@sgallagher ping. |
@john-yan At minimum, you need to sign Fedora's Contributor License Agreement (can be done from the account tool you linked above). Then I need to get you added to some FAS group with access to the testing machines. |
@john-yan Oh, you'll also need to make sure to add an SSH public key to your FAS account so we can use that to grant you access. (Please don't submit your private SSH key... that causes chaos and paperwork 😄) |
@sgallagher OK I just signed it. sorry I created the account without knowing I also need to sign it after. |
@john-yan OK, once you have the SSH key uploaded, I'll get you access. |
@sgallagher Thanks, Just uploaded ssh public key. |
@john-yan OK, you should now have access to any of the machines listed at https://fedoraproject.org/wiki/Test_Machine_Resources_For_Package_Maintainers#Available_Machines.2FInstances |
@sgallagher Thanks, I will have a look. |
@john-yan any update? |
Applying 2 more patches to the above patches will fix the debug compiling, here is a diff:
|
@miladfarca thanks for the update :). I guess the next steps are potentially getting into V8 master if the problem still exists there as a pre-req for floating in Node.js. |
@mhdawson no problem, master already includes these fixes, either with the exact code or a modified version past this branch, I have confirmed this by looking at the v8 master as well v8 in nodejs master. I have also built the latest nodejs on fedora/ppc64 and it builds with no errors. I am going to work on backporting this fix to an abandoned branch of v8 in nodejs 10.1.0 |
PR submitted for backport to v10.x |
@miladfarca thanks for the update and the PRs :) |
@mhdawson Not a problem. |
Looks like PR landed so we just need to wait for the next v10.x release :) |
@sgallagher, #25827 landed and was released in v10.15.3, so I believe this is now fixed. Thanks @miladfarca |
I can confirm that this worked in my Fedora builds. Thanks! |
Attempting to build 10.1.0 on ppc64 and ppc64le architectures results in a build failure. This failure was not present on 10.0.0.
My guess is that one of the assembler changes that came in between v8 6.6.346.27 and 6.6.346.24 broke something.
The text was updated successfully, but these errors were encountered: