Skip to content

Don't cap the similarity score when neither binary has flow graph edges - #93

Open
rootkiller6788 wants to merge 2 commits into
google:mainfrom
rootkiller6788:fix-similarity-no-flow-graph-edges
Open

rootkiller6788 wants to merge 2 commits into
google:mainfrom
rootkiller6788:fix-similarity-no-flow-graph-edges

Conversation

@rootkiller6788

Copy link
Copy Markdown

Fixes #90.

When neither binary has any flow graph edges (every function is a single basic block with no calls), the edge term in the call graph similarity score is necessarily 0, which caps the whole score at 0.65. I made that term count as a match when both sides have zero edges, which is what was suggested in the issue.

Where only one side has edges I left the term at zero. The two binaries really do differ in control flow there, so they shouldn't get the credit for it.

To check it I pulled the function body out of differ.cc and compiled it standalone against the real statistics.h (stubbing out CallGraph and GetConfidence, which the formula only reads through). A self-diff with no edges goes 0.650000 -> 1.000000, the all-edges-match case stays 1.000000, and the one-sided case stays 0.650000.

One thing worth flagging: differ_test.cc doesn't look like it's built by the OSS CMakeLists -- the test targets there cover call_graph, change_classifier, config, database_writer, match, prime_signature, sqlite and writer, and differ_benchmark.cc (which CMakeLists does reference) isn't in the tree either. So the test I added won't actually run on GitHub CI. I left CMakeLists alone since I can't build it here to verify a change. Happy to wire it up in a separate PR if you want it running.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the CallGraph version of GetSimilarityScore in differ.cc to handle cases where both sides have zero flow graph edges, preventing the similarity score from being capped at 0.65. It also adds a unit test in differ_test.cc to verify this behavior. The reviewer points out that the same issue exists in the FlowGraph overload of GetSimilarityScore and suggests applying a similar fix there to avoid penalizing single-block functions.

Comment thread differ.cc
counts[Counts::kFlowGraphEdgesPrimaryNonLibrary];
const uint64_t edges_secondary =
counts[Counts::kFlowGraphEdgesSecondaryNonLibrary];
if (edges_primary == 0 && edges_secondary == 0) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this change correctly addresses the issue for the CallGraph similarity score, the same problem also exists in the FlowGraph overload of GetSimilarityScore (defined at line 490). If two functions have some differences in basic blocks or instructions (so they do not trigger the return 1.0 shortcut on line 518) but both have zero flow graph edges, the edge term (which contributes 55% of the score) will be evaluated as 0.55 * 0 / 1.0 = 0. This heavily penalizes the similarity score of single-block functions. Consider applying a similar check in the FlowGraph overload of GetSimilarityScore to avoid capping the score when both flow graphs have zero edges.

@rootkiller6788
rootkiller6788 marked this pull request as ready for review October 16, 2026 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Similarity score is capped at 0.65 for binaries with no flow graph edges

1 participant