From d86e38e2d2a7883d91603dbd73cd8cde6bbd751b Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 10 Sep 2018 12:06:45 +0200 Subject: [PATCH 1/4] src: fix `--prof-process` CLI argument handling Make sure that options after `--prof-process` are not treated as Node.js options. Fixes: https://github.com/nodejs/node/issues/22786 --- src/node_options.cc | 2 ++ .../parallel/test-tick-processor-arguments.js | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 test/parallel/test-tick-processor-arguments.js diff --git a/src/node_options.cc b/src/node_options.cc index c1cbb59e291044..8027ee677c1e12 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -108,6 +108,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { AddOption("--prof-process", "process V8 profiler output generated using --prof", &EnvironmentOptions::prof_process); + // Options after --prof-process are passed through to the prof processor. + AddAlias("--prof-process", { "--prof-process", "--" }); AddOption("--redirect-warnings", "write warnings to file instead of stderr", &EnvironmentOptions::redirect_warnings, diff --git a/test/parallel/test-tick-processor-arguments.js b/test/parallel/test-tick-processor-arguments.js new file mode 100644 index 00000000000000..921b851cc297b2 --- /dev/null +++ b/test/parallel/test-tick-processor-arguments.js @@ -0,0 +1,24 @@ +'use strict'; +require('../common'); +const tmpdir = require('../common/tmpdir'); +const fs = require('fs'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +tmpdir.refresh(); +process.chdir(tmpdir.path); + +// Generate log file. +spawnSync(process.execPath, [ '--prof', '-p', '42' ]); + +const logfile = fs.readdirSync('.').filter((name) => name.endsWith('.log'))[0]; +assert(logfile); + +// Make sure that the --preprocess argument is passed through correctly. +const { stdout } = spawnSync( + process.execPath, + [ '--prof-process', '--preprocess', logfile ], + { encoding: 'utf8' }); + +// Make sure that the result is valid JSON. +JSON.parse(stdout); From 496e215468b3642ba7643574d36c06c556e780cf Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 10 Sep 2018 12:51:31 +0200 Subject: [PATCH 2/4] fixup! src: fix `--prof-process` CLI argument handling --- lib/internal/bootstrap/node.js | 4 +++- test/parallel/test-tick-processor-arguments.js | 10 ++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 265bc81d0c8711..ecdb7a4029095e 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -693,9 +693,11 @@ for (const [ from, expansion ] of aliases) { let isAccepted = true; for (const to of expansion) { - if (!to.startsWith('-')) continue; + if (!to.startsWith('-') || to === '--') continue; const recursiveExpansion = aliases.get(to); if (recursiveExpansion) { + if (recursiveExpansion[0] === to) + recursiveExpansion.splice(0, 1); expansion.push(...recursiveExpansion); continue; } diff --git a/test/parallel/test-tick-processor-arguments.js b/test/parallel/test-tick-processor-arguments.js index 921b851cc297b2..5c813645a2ca34 100644 --- a/test/parallel/test-tick-processor-arguments.js +++ b/test/parallel/test-tick-processor-arguments.js @@ -1,10 +1,13 @@ 'use strict'; -require('../common'); +const common = require('../common'); const tmpdir = require('../common/tmpdir'); const fs = require('fs'); const assert = require('assert'); const { spawnSync } = require('child_process'); +if (!common.isMainThread) + common.skip('chdir not available in workers'); + tmpdir.refresh(); process.chdir(tmpdir.path); @@ -14,7 +17,10 @@ spawnSync(process.execPath, [ '--prof', '-p', '42' ]); const logfile = fs.readdirSync('.').filter((name) => name.endsWith('.log'))[0]; assert(logfile); -// Make sure that the --preprocess argument is passed through correctly. +// Make sure that the --preprocess argument is passed through correctly, +// as an example flag listed in deps/v8/tools/tickprocessor.js. +// Any of the other flags there should work for this test too, if --preprocess +// is ever removed. const { stdout } = spawnSync( process.execPath, [ '--prof-process', '--preprocess', logfile ], From fe0949f6dccf4ef2ac650477c44f436a2b16bc53 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 11 Sep 2018 13:08:27 +0200 Subject: [PATCH 3/4] fixup! src: fix `--prof-process` CLI argument handling --- test/parallel/test-tick-processor-arguments.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/test-tick-processor-arguments.js b/test/parallel/test-tick-processor-arguments.js index 5c813645a2ca34..218261a14f3de3 100644 --- a/test/parallel/test-tick-processor-arguments.js +++ b/test/parallel/test-tick-processor-arguments.js @@ -7,6 +7,8 @@ const { spawnSync } = require('child_process'); if (!common.isMainThread) common.skip('chdir not available in workers'); +if (!common.enoughTestMem) + common.skip('skipped due to memory requirements'); tmpdir.refresh(); process.chdir(tmpdir.path); From 5348ef3e346f8f8fbd978f60d56ea8f9add952bd Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 12 Sep 2018 12:26:58 +0200 Subject: [PATCH 4/4] fixup! src: fix `--prof-process` CLI argument handling --- test/parallel/test-tick-processor-arguments.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/parallel/test-tick-processor-arguments.js b/test/parallel/test-tick-processor-arguments.js index 218261a14f3de3..606dbd03c8f6c4 100644 --- a/test/parallel/test-tick-processor-arguments.js +++ b/test/parallel/test-tick-processor-arguments.js @@ -9,6 +9,8 @@ if (!common.isMainThread) common.skip('chdir not available in workers'); if (!common.enoughTestMem) common.skip('skipped due to memory requirements'); +if (common.isAIX) + common.skip('does not work on AIX'); tmpdir.refresh(); process.chdir(tmpdir.path);