Fix: Incomplete Call Graph Due to Early Return in makeAllCG Branch#105
Fix: Incomplete Call Graph Due to Early Return in makeAllCG Branch#105RacerZ-fighting wants to merge 2 commits intoantgroup:mainfrom
Conversation
Summary of ChangesHello @RacerZ-fighting, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue in the call graph generation logic where enabling Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a bug that caused an incomplete call graph when Config.makeAllCG is enabled. The introduction of analyzedFunctionBodies to track analyzed functions prevents premature returns and ensures function bodies are processed. The logic is sound and well-implemented. I've included a couple of suggestions to improve efficiency and code clarity.
| if (!this.ainfo.analyzedFunctionBodies) { | ||
| this.ainfo.analyzedFunctionBodies = new Set() | ||
| } |
There was a problem hiding this comment.
This check and initialization of analyzedFunctionBodies occurs on every call to executeCall, which can be a hot path. For better performance, it would be more efficient to initialize this.ainfo.analyzedFunctionBodies = new Set() once at the beginning of the analysis, for instance, within the startAnalyze method. This would eliminate this redundant check.
| return currentValue.fdef === fdecl ? previousValue + 1 : previousValue | ||
| }, 0) > 0 | ||
| ) { | ||
| // avoid infinite loops, the re-entry should only less than 3 |
There was a problem hiding this comment.
The comment is misleading. It states that re-entry should be less than 3, but the logic (recursionCount > 0) prevents any recursion at all. The comment should be updated to accurately reflect the code's behavior to avoid confusion for future maintainers.
| // avoid infinite loops, the re-entry should only less than 3 | |
| // avoid infinite loops by disallowing recursion |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| }) | ||
| } else { | ||
| this.ainfo.analyzedFunctionBodies.add(funcKey) | ||
| break |
There was a problem hiding this comment.
Double call to checkAtFunctionCallBefore adds duplicate edges
High Severity
When alreadyAnalyzed is false, checkAtFunctionCallBefore is called at line 2043, then the code breaks and falls through to executeSingleCall, which calls checkAtFunctionCallBefore again. This results in the call graph edge being added twice for every function that hasn't been analyzed yet, causing duplicate edges in the call graph.
Additional Locations (1)
| } | ||
|
|
||
| // Best-effort callee name resolution. | ||
| const fclosName = fclos?.name || fclos?.id || fclos?.sid || '' |
There was a problem hiding this comment.
fclosName may be object causing startsWith TypeError
Medium Severity
fclosName is assigned via fclos?.name || fclos?.id || fclos?.sid || '' but these properties can be objects, not strings. The prettyPrint method in the same file explicitly handles non-string fclos.id and fclos.sid. If any is an object, fclosName.startsWith('...') in shouldSkipCall throws a TypeError.


Problem
With
Config.makeAllCGenabled, when a callee function node already exists in the call graph,executeCallmay return early. In that case, the function can appear in the call graph, but its body may never be analyzed, so additional call edges that would be discovered from analyzing its body are missing.This logic is implemented in the executeCall method, where the analyzer checks for existing nodes to optimize performance. For further details on the potential for missing call edges when nodes are reused, see #104.
YASA-Engine/src/engine/analyzer/common/analyzer.ts
Lines 2027 to 2049 in 4d28316
Root Cause
In
executeCall, theConfig.makeAllCGbranch adds a call graph edge but then returns early:Solution
Introduced
analyzedFunctionBodiesSet to track which function bodies have been analyzed:If already analyzed → add edge only, return
SymbolValue(avoid redundant analysis)If not analyzed → mark as analyzed, break to continue with
executeSingleCallFiles Changed
src/engine/analyzer/common/analyzer.tsNote
Medium Risk
Touches core
executeCall/call graph emission logic, which can change analysis results and performance; risk is mainly around unintended call graph/analysis completeness changes rather than runtime behavior.Overview
Fixes incomplete call graphs when
Config.makeAllCGis enabled by tracking analyzed function bodies (ainfo.analyzedFunctionBodies) and only skipping re-analysis after a function body has been processed once; call edges are still emitted for subsequent calls.Improves call graph output for unresolved/external calls by filtering noisy internal symbol paths and emitting simplified
<external>callee labels derived from the parent chain, instead of pretty-printing opaque internal identifiers.Written by Cursor Bugbot for commit d8f3a86. This will update automatically on new commits. Configure here.