From 20072c55eab0634d7a81472bc6a0c57d6f5d998a Mon Sep 17 00:00:00 2001 From: Grzegorz Latosinski Date: Thu, 29 Feb 2024 18:01:14 +0100 Subject: [PATCH] import: callgrind.ts: Improved collection of file names associated with functions When called function is within an inlined code block, it should use file names provided by 'fi' or 'fe' commands. Also, when 'fn' command is called, it sets the current file name to the value provided in the previous 'fl' call. Also, the file associated with 'fn' should be stored separately so that it is not overwritten by new 'fl' calls (Callgrind does not tell whether there can be multiple 'fl' calls without following 'fn' calls, and this should guard the parser from associating the wrong new file name with the previous 'fn' function). This modification fixes detached call stacks, which were created when one function has been called from inline block without the file source update, which led to duplicated entries, which were later not removed when determining root functions. Signed-off-by: Grzegorz Latosinski --- src/import/callgrind.ts | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/import/callgrind.ts b/src/import/callgrind.ts index dc37c7c1f..f30ced9e0 100644 --- a/src/import/callgrind.ts +++ b/src/import/callgrind.ts @@ -342,6 +342,8 @@ class CallgrindParser { private filename: string | null = null private functionName: string | null = null + private functionFile: string | null = null + private currentFunctionFile: string | null = null private calleeFilename: string | null = null private calleeFunctionName: string | null = null @@ -396,7 +398,7 @@ class CallgrindParser { } private frameInfo(): FrameInfo { - const file = this.filename || '(unknown)' + const file = this.functionFile || '(unknown)' const name = this.functionName || '(unknown)' const key = `${file}:${name}` return {key, name, file} @@ -445,23 +447,30 @@ class CallgrindParser { case 'fe': case 'fi': { // fe/fi are used to indicate the source-file of a function definition - // changed mid-definition. This is for inlined code, but doesn't - // indicate that we've actually switched to referring to a different - // function, so we mostly ignore it. - // - // We still need to do the parseNameWithCompression call in case a name - // is defined and then referenced later on for name compression. - this.parseNameWithCompression(value, this.savedFileNames) + // changed mid-definition. This is for inlined code, but when a function + // is called from within the inlined code, it is indicated that it + // comes from file marked by fe/fi. + this.filename = this.parseNameWithCompression(value, this.savedFileNames) break } case 'fl': { this.filename = this.parseNameWithCompression(value, this.savedFileNames) + // The 'fl' needs to be stored in order to reset current filename upon + // consecutive 'fn' calls + this.currentFunctionFile = this.filename break } case 'fn': { + // the file for a function defined with 'fn' always comes from 'fl' + // It also resets the current filename to previous 'fl' value + // https://github.com/KDE/kcachegrind/blob/ea4314db2785cb8f279fe884ee7f82445642b692/libcore/cachegrindloader.cpp#L808 + if (this.filename !== this.currentFunctionFile) this.filename = this.currentFunctionFile this.functionName = this.parseNameWithCompression(value, this.savedFunctionNames) + // since the format of callgrind does not disallow to define multiple 'fl' without following 'fn', + // it is important to store the associated filename for the current function for frameInfo method + this.functionFile = this.filename break }