src: fix --prof-process CLI argument handling

addaleax · targos · commit 23f8b0251335 · 2018-09-16T09:44:37.000+02:00
Make sure that options after `--prof-process` are not treated as Node.js options. Fixes: #22786 PR-URL: #22790 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -694,9 +694,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/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
@@ -0,0 +1,34 @@
+'use strict';
+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');
+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);
+
+// 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,
+// 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 ],
+  { encoding: 'utf8' });
+
+// Make sure that the result is valid JSON.
+JSON.parse(stdout);
