Fixing #3159 - Method call graph ignores constructors and non-parametric void methods#3160
Conversation
JuditKnoll
left a comment
There was a problem hiding this comment.
Thank you for your contribution!
Can you please add one more test, where one of the functions called multiple times in a way that it could have affect on the order?
Can you please add an entry to the CHANGELOG.md file's unreleased part?
spotbugs-tests/src/test/java/edu/umd/cs/findbugs/classfile/MethodsCallOrderTest.java
Outdated
Show resolved
Hide resolved
spotbugs/src/main/java/edu/umd/cs/findbugs/classfile/engine/SelfMethodCalls.java
Outdated
Show resolved
Hide resolved
Hi @JuditKnoll , just to confirm my understanding, I can think of 2 different kinds that impact the order. First case is when Another case is Thank you |
I was thinking about the second scenario. |
|
Hi @JuditKnoll, please see the new commits. Thanks. |
JuditKnoll
left a comment
There was a problem hiding this comment.
Can you please add an entry to the CHANGELOG.md file's unreleased part?
|
Ops, forgot. Thank you @JuditKnoll. |
…d methods. All these methods are missing to be properly sorted by TopologicalSort (spotbugs#3159)
4cae481 to
085de4f
Compare
|
There was a conflict in CHANGELOG.md. Rebased. |
Looks like the problem is there from the very beginnings where the logic ignored all non-parametric methods as "not interesting".
Omitting those methods from the MultiMap doesn't give TopologySort any information about the graph orientation and these methods are randomly misplaced when calling
ClassContext#getMethodsInCallOrder.Part of the PR is a test that has a simple call stack from the default
static{}constructor<clinit>()Vthrough several static methods to a non-parametric class constructor, one more constructor and then instantiated class methods which creates a very simple DAG that any topological sorting algorithm should have no problem analyzing.After fixing the
interestingSignaturemethod fromreturn !"()V".equals(signature);toreturn true;, it works! :)FYI: required for proper FindSecurityBugs taint analysis of derived methods. Without proper order they are skipped by the analysis engine as unknown.