Skip to content

Commit

Permalink
tickprocessor: apply c++filt manually on mac
Browse files Browse the repository at this point in the history
`/bin/sh -c` trick wasn't working for several reasons:

* `/bin/sh -c "..."` expects the first argument after `"..."` to be a
  `$0`, not a `$1`. Previously `-n` wasn't passed to `nm` because of
  this, and many symbols were ordered improperly
* `c++filt` was applied not only to the names of the functions but to
  their `nm` prefixes like `t` and `a` (`t xxx` turns into
  `unsigned char xxx`).

Instead of applying `c++filt` wide and using `sh -c`, execute `nm` as
requested by `deps/v8/tools/tickprocessor.js` and apply `c++filt` to all
matching entries manually.

Included test demonstrates where previous approach failed: all builtins
were merged into `v8::internal::Builtins::~Builtins`, because they were
prefixed by `t` in `nm` output.

PR-URL: #8480
Reviewed-By: Matthew Loring <mattloring@google.com>
  • Loading branch information
indutny authored and Fishrock123 committed Sep 14, 2016
1 parent 3358861 commit eff771f
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 6 deletions.
36 changes: 32 additions & 4 deletions lib/internal/v8_prof_polyfill.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@ const os = {
/^[0-9a-f]+-[0-9a-f]+$/.test(arg)) {
return '';
}
} else if (process.platform === 'darwin') {
args.unshift('-c', name);
name = '/bin/sh';
}
return cp.spawnSync(name, args).stdout.toString();
let out = cp.spawnSync(name, args).stdout.toString();
// Auto c++filt names, but not [iItT]
if (process.platform === 'darwin' && name === 'nm')
out = macCppfiltNm(out);
return out;
}
};
const print = console.log;
Expand Down Expand Up @@ -100,3 +101,30 @@ function versionCheck() {
}
}
}

function macCppfiltNm(out) {
// Re-grouped copy-paste from `tickprocessor.js`
const FUNC_RE = /^([0-9a-fA-F]{8,16} [iItT] )(.*)$/gm;
let entries = out.match(FUNC_RE);
if (entries === null)
return out;

entries = entries.map((entry) => {
return entry.replace(/^[0-9a-fA-F]{8,16} [iItT] /, '')
});

let filtered;
try {
filtered = cp.spawnSync('c++filt', [ '-p' , '-i' ], {
input: entries.join('\n')
}).stdout.toString();
} catch (e) {
return out;
}

let i = 0;
filtered = filtered.split(/\n/g);
return out.replace(FUNC_RE, (all, prefix, postfix) => {
return prefix + (filtered[i++] || postfix);
});
}
3 changes: 1 addition & 2 deletions lib/internal/v8_prof_processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ scriptFiles.forEach(function(s) {

var tickArguments = [];
if (process.platform === 'darwin') {
const nm = 'foo() { nm "$@" | (c++filt -p -i || cat) }; foo $@';
tickArguments.push('--mac', '--nm=' + nm);
tickArguments.push('--mac');
} else if (process.platform === 'win32') {
tickArguments.push('--windows');
}
Expand Down
8 changes: 8 additions & 0 deletions test/parallel/test-tick-processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ runTest(/RunInDebugContext/,
setTimeout(function() { process.exit(0); }, 2000);
f();`);

runTest(/Builtin_DateNow/,
`function f() {
this.ts = Date.now();
setImmediate(function() { new f(); });
};
setTimeout(function() { process.exit(0); }, 2000);
f();`);

function runTest(pattern, code) {
cp.execFileSync(process.execPath, ['-prof', '-pe', code]);
var matches = fs.readdirSync(common.tmpDir);
Expand Down

0 comments on commit eff771f

Please sign in to comment.