Page MenuHomeFreeBSD

[PowerPC64] fix www/node build and runtime when using ELFv2 ABI
ClosedPublic

Authored by alfredo on Sep 10 2019, 4:04 PM.

Details

Summary

This patch allows www/node to build and run on powerpc64. It fixes incorrect assumptions between ELFv1/ELFv2 and LE/BE logic.

Test Plan

Commands on server/node console:

root@alfredo-2:~/src/freebsd-ports/www/node # node
Welcome to Node.js v12.11.1.
Type ".help" for more information.
> var http = require('http');
undefined
> http.createServer(function (req, res) { res.writeHead(200, {'Content-Type': 'text/plain'});  res.end('Hello Node.js World!'); }).listen(8080);
Server {
  _events: [Object: null prototype] {
    request: [Function],
    connection: [Function: connectionListener]
  },
  _eventsCount: 2,
  _maxListeners: undefined,
  _connections: 0,
  _handle: TCP {
    reading: false,
    onconnection: [Function: onconnection],
    [Symbol(owner)]: [Circular]
  },
  _usingWorkers: false,
  _workers: [],
  _unref: false,
  allowHalfOpen: true,
  pauseOnConnect: false,
  httpAllowHalfOpen: false,
  timeout: 120000,
  keepAliveTimeout: 5000,
  maxHeadersCount: null,
  headersTimeout: 40000,
  _connectionKey: '6::::8080',
  [Symbol(IncomingMessage)]: [Function: IncomingMessage],
  [Symbol(ServerResponse)]: [Function: ServerResponse],
  [Symbol(asyncId)]: 209
}
>

From client:

# curl http://localhost:8080 -v
*   Trying 127.0.0.1:8080...
* TCP_NODELAY set
* Connected to localhost (127.0.0.1) port 8080 (#0)
> GET / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.66.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< Content-Type: text/plain
< Date: Mon, 21 Oct 2019 21:43:27 GMT
< Connection: keep-alive
< Transfer-Encoding: chunked
< 
* Connection #0 to host localhost left intact
Hello Node.js World!

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

alfredo added reviewers: mikael, bhughes.
alfredo added a subscriber: luporl.
alfredo added reviewers: linimon, pkubaj.
pkubaj requested changes to this revision.Sep 21 2019, 3:00 PM

It would be nice if you could apply files/patch-common.gypi only for elfv2, to have node available for elfv1.

This revision now requires changes to proceed.Sep 21 2019, 3:00 PM
pkubaj edited the summary of this revision. (Show Details)

Remove -mminimal-toc only on 13.

To be committed after the switch to clang.

This revision is now accepted and ready to land.Sep 21 2019, 7:04 PM

node isn't building with llvm90 anymore due to https://bugs.llvm.org/show_bug.cgi?id=43527, so patch is updated to use llvm-devel instead.

Note: this requires PPC_ABI variable with value set to "ELFv2" in order to build on powerpc64/elfv2

This revision now requires review to proceed.Oct 9 2019, 1:16 PM
www/node/files/extra-patch-common.gypi
8 ↗(On Diff #63076)

cflags should have -mabi=elfv2 appended. Lack of it killed the build for me.

9 ↗(On Diff #63076)

Same as above.

www/node/Makefile
79 ↗(On Diff #63076)

extra-patch-common.gypi is valid only for elfv2 and elfv2 will be only 13 at the moment.

This means that we can drop EXTRA_PATCHES_13 and do directly:
EXTRA_PATCHES=${PATCHDIR}/extra-patch-common.gypi

81–84 ↗(On Diff #63076)

Those are not necessary. With changes in this review, this port will build on ppc64 elfv2.

pkubaj requested changes to this revision.Oct 16 2019, 6:14 PM
This revision now requires changes to proceed.Oct 16 2019, 6:14 PM
alfredo added inline comments.
www/node/Makefile
81–84 ↗(On Diff #63076)

Let me double check this since this dependency was added due to a bug on llvm9 and the fix candidate is still under review.

www/node/files/extra-patch-common.gypi
8 ↗(On Diff #63076)

-mabi=elfv2 should not be necessary since it's the default ABI in this contex

www/node/files/extra-patch-common.gypi
8 ↗(On Diff #63076)

Without this, I got linking errors. Do you build the current version of www/node? I built www/node 12.10.0 before without adding ABI version, but 12.11.1 doesn't build without specifying ABI.

alfredo edited the summary of this revision. (Show Details)
alfredo edited the test plan for this revision. (Show Details)

Updated patches after tests with llvm90 as base compiler.

alfredo added inline comments.
www/node/Makefile
81–84 ↗(On Diff #63076)

it is still required, or I hit PPC CRT loop issue.

www/node/files/extra-patch-common.gypi
8 ↗(On Diff #63076)

I couldn't reproduce it here being both base llvm and ports llvm-devel patched for ELFv2-default.
Apparently the compiler your're using isn't generating ELFv2 by default.

Needs to be updated for 12.12.0:

1 out of 1 hunks failed--saving rejects to tools/v8_gypfiles/v8.gyp.rej
=> FreeBSD patch patch-tools_v8__gypfiles_v8.gyp failed to apply cleanly.
=> Patch(es)  patch-deps_openssl_config_archs_linux-elf_no-asm_openssl-cl.gypi patch-deps_openssl_config_archs_linux-elf_no-asm_openssl.gypi patch-deps_openssl_openssl-cl__no__asm.gypi patch-deps_openssl_openssl__no__asm.gypi patch-deps_v8_src_base_platform_platform-freebsd.cc patch-deps_v8_src_codegen_arm_cpu-arm.cc patch-deps_v8_src_codegen_ppc_constants-ppc.h patch-deps_v8_src_execution_simulator.h patch-deps_v8_src_libsampler_sampler.cc patch-node.gypi applied cleanly.

Other than that, fine with me.

This revision is now accepted and ready to land.Oct 23 2019, 1:32 PM

Node compiles just fine after removing files/patch-tools_v8__gypfiles_v8.gyp. Looks like it's now not necessary.

pkubaj requested changes to this revision.Oct 29 2019, 12:25 PM
This revision now requires changes to proceed.Oct 29 2019, 12:25 PM
alfredo marked an inline comment as done.
alfredo edited the summary of this revision. (Show Details)

updated patch for node-13.0

llvm-devel dependency removed since https://bugs.llvm.org/show_bug.cgi?id=43527 should be fixed in FreeBSD base by commit 354339.

This revision is now accepted and ready to land.Nov 5 2019, 2:16 PM
www/node/Makefile
80 ↗(On Diff #63937)

extra blank lines