mirror/ghidra
1
0
Fork 0
mirror of https://github.com/NationalSecurityAgency/ghidra.git synced 2026-09-28 17:11:11 -09:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/GP-364-dragonmacher-call-trees-duplicate-bug--SQUASHED' into Ghidra_9.2

Browse Source
This commit is contained in:
ghidravore
2020-11-03 19:27:25 -05:00
parent 27cb4b20c0 66f3718443
commit 9326a5d963
7 changed files with 161 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/calltree/CallNode.java
View File

@@ -20,6 +20,8 @@ import java.util.concurrent.atomic.AtomicInteger;
import javax.swing.tree.TreePath;
import org.apache.commons.collections4.map.LazyMap;
import docking.widgets.tree.GTreeNode;
import docking.widgets.tree.GTreeSlowLoadingNode;
import ghidra.program.model.address.*;
@@ -44,7 +46,13 @@ public abstract class CallNode extends GTreeSlowLoadingNode {
this.filterDepth = filterDepth;
}
public abstract Function getContainingFunction();
/**
* Returns this node's remote function, where remote is the source function for
* an incoming call or a destination function for an outgoing call. May return
* null for nodes that do not have functions.
* @return the function or null
*/
public abstract Function getRemoteFunction();
/**
* Returns a location that represents the caller of the callee.
@@ -68,7 +76,7 @@ public abstract class CallNode extends GTreeSlowLoadingNode {
protected Set<Reference> getReferencesFrom(Program program, AddressSetView addresses,
TaskMonitor monitor) throws CancelledException {
Set<Reference> set = new HashSet<Reference>();
Set<Reference> set = new HashSet<>();
ReferenceManager referenceManager = program.getReferenceManager();
AddressIterator addressIterator = addresses.getAddresses(true);
while (addressIterator.hasNext()) {
@@ -93,15 +101,24 @@ public abstract class CallNode extends GTreeSlowLoadingNode {
this.allowDuplicates = allowDuplicates;
}
protected void addNode(List<GTreeNode> nodes, GTreeNode node) {
protected void addNode(LazyMap<Function, List<GTreeNode>> nodesByFunction,
CallNode node) {
Function function = node.getRemoteFunction();
List<GTreeNode> nodes = nodesByFunction.get(function);
if (nodes.contains(node)) {
return; // never add equal() nodes
}
if (allowDuplicates) {
nodes.add(node);
nodes.add(node); // ok to add multiple nodes for this function with different addresses
}
if (nodes.isEmpty()) {
nodes.add(node); // no duplicates allow; only add if this is the only node
return;
}
if (!nodes.contains(node)) {
nodes.add(node);
}
}
protected class CallNodeComparator implements Comparator<GTreeNode> {
@@ -133,8 +150,8 @@ public abstract class CallNode extends GTreeSlowLoadingNode {
Object[] pathComponents = path.getPath();
for (Object pathComponent : pathComponents) {
CallNode node = (CallNode) pathComponent;
Function nodeFunction = node.getContainingFunction();
Function myFunction = getContainingFunction();
Function nodeFunction = node.getRemoteFunction();
Function myFunction = getRemoteFunction();
if (node != this && nodeFunction.equals(myFunction)) {
return true;
}
@@ -142,14 +159,12 @@ public abstract class CallNode extends GTreeSlowLoadingNode {
return false;
}
// overridden since we may have multiple children with the same function name, but in
// different namespaces
@Override
public boolean equals(Object obj) {
if (this == obj) {
return true;
}
if (obj == null) {
if (!super.equals(obj)) {
return false;
}
if (getClass() != obj.getClass()) {
@@ -157,12 +172,21 @@ public abstract class CallNode extends GTreeSlowLoadingNode {
}
CallNode other = (CallNode) obj;
return getSourceAddress().equals(other.getSourceAddress());
if (!Objects.equals(getSourceAddress(), other.getSourceAddress())) {
return false;
}
return Objects.equals(getRemoteFunction(), other.getRemoteFunction());
}
@Override
public int hashCode() {
return getSourceAddress().hashCode();
final int prime = 31;
int result = super.hashCode();
Function function = getRemoteFunction();
result = prime * result + ((function == null) ? 0 : function.hashCode());
Address sourceAddress = getSourceAddress();
result = prime * result + ((sourceAddress == null) ? 0 : sourceAddress.hashCode());
return result;
}
}

7
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/calltree/CallTreeProvider.java
View File

@@ -283,7 +283,8 @@ public class CallTreeProvider extends ComponentProviderAdapter implements Domain
for (TreePath path : selectionPaths) {
GTreeNode node = (GTreeNode) path.getLastPathComponent();
if (node instanceof GTreeNode) {
if (node instanceof OutgoingCallsRootNode ||
node instanceof IncomingCallsRootNode) {
return false;
}
}
@@ -1095,7 +1096,7 @@ public class CallTreeProvider extends ComponentProviderAdapter implements Domain
//TODO do we need to use a PendingRootNode?
if (root instanceof CallNode) {
CallNode callNode = (CallNode) root;
Function nodeFunction = callNode.getContainingFunction();
Function nodeFunction = callNode.getRemoteFunction();
if (nodeFunction.equals(function)) {
reloadUpdateManager.update();
return true;
@@ -1131,7 +1132,7 @@ public class CallTreeProvider extends ComponentProviderAdapter implements Domain
// first, if the given node represents the function we have, then we don't need to
// go any further
if (function.equals(node.getContainingFunction())) {
if (function.equals(node.getRemoteFunction())) {
GTreeNode parent = node.getParent();
parent.removeNode(node);
parent.addNode(node.recreate());

3
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/calltree/DeadEndNode.java
View File

@@ -1,6 +1,5 @@
/* ###
* IP: GHIDRA
* REVIEWED: YES
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -54,7 +53,7 @@ public class DeadEndNode extends CallNode {
}
@Override
public Function getContainingFunction() {
public Function getRemoteFunction() {
return null; // no function--dead end
}

2
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/calltree/ExternalCallNode.java
View File

@@ -61,7 +61,7 @@ public class ExternalCallNode extends CallNode {
}
@Override
public Function getContainingFunction() {
public Function getRemoteFunction() {
return function;
}

19
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/calltree/IncomingCallNode.java
View File

@@ -17,9 +17,12 @@ package ghidra.app.plugin.core.calltree;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import javax.swing.Icon;
import org.apache.commons.collections4.map.LazyMap;
import docking.widgets.tree.GTreeNode;
import ghidra.app.plugin.core.navigation.locationreferences.ReferenceUtils;
import ghidra.program.model.address.Address;
@@ -70,7 +73,7 @@ public class IncomingCallNode extends CallNode {
}
@Override
public Function getContainingFunction() {
public Function getRemoteFunction() {
return function;
}
@@ -86,7 +89,8 @@ public class IncomingCallNode extends CallNode {
new FunctionSignatureFieldLocation(program, functionAddress);
Set<Address> addresses = ReferenceUtils.getReferenceAddresses(location, monitor);
List<GTreeNode> nodes = new ArrayList<>();
LazyMap<Function, List<GTreeNode>> nodesByFunction =
LazyMap.lazyMap(new HashMap<>(), k -> new ArrayList<>());
FunctionManager functionManager = program.getFunctionManager();
for (Address fromAddress : addresses) {
monitor.checkCanceled();
@@ -97,12 +101,17 @@ public class IncomingCallNode extends CallNode {
IncomingCallNode node = new IncomingCallNode(program, callerFunction, fromAddress,
filterDuplicates, filterDepth);
addNode(nodes, node);
addNode(nodesByFunction, node);
}
Collections.sort(nodes, new CallNodeComparator());
List<GTreeNode> children =
nodesByFunction.values()
.stream()
.flatMap(list -> list.stream())
.collect(Collectors.toList());
Collections.sort(children, new CallNodeComparator());
return nodes;
return children;
}
@Override

22
Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/calltree/OutgoingCallNode.java
View File

@@ -17,10 +17,13 @@ package ghidra.app.plugin.core.calltree;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import javax.swing.Icon;
import javax.swing.tree.TreePath;
import org.apache.commons.collections4.map.LazyMap;
import docking.widgets.tree.GTreeNode;
import ghidra.program.model.address.Address;
import ghidra.program.model.address.AddressSetView;
@@ -66,7 +69,7 @@ public abstract class OutgoingCallNode extends CallNode {
}
@Override
public Function getContainingFunction() {
public Function getRemoteFunction() {
return function;
}
@@ -75,7 +78,8 @@ public abstract class OutgoingCallNode extends CallNode {
AddressSetView functionBody = function.getBody();
Address entryPoint = function.getEntryPoint();
Set<Reference> references = getReferencesFrom(program, functionBody, monitor);
List<GTreeNode> nodes = new ArrayList<GTreeNode>();
LazyMap<Function, List<GTreeNode>> nodesByFunction =
LazyMap.lazyMap(new HashMap<>(), k -> new ArrayList<>());
FunctionManager functionManager = program.getFunctionManager();
for (Reference reference : references) {
monitor.checkCanceled();
@@ -85,15 +89,21 @@ public abstract class OutgoingCallNode extends CallNode {
}
Function calledFunction = functionManager.getFunctionAt(toAddress);
createNode(nodes, reference, calledFunction);
createNode(nodesByFunction, reference, calledFunction);
}
Collections.sort(nodes, new CallNodeComparator());
List<GTreeNode> children =
nodesByFunction.values()
.stream()
.flatMap(list -> list.stream())
.collect(Collectors.toList());
Collections.sort(children, new CallNodeComparator());
return nodes;
return children;
}
private void createNode(List<GTreeNode> nodes, Reference reference, Function calledFunction) {
private void createNode(LazyMap<Function, List<GTreeNode>> nodes, Reference reference,
Function calledFunction) {
if (calledFunction != null) {
if (isExternalCall(calledFunction)) {
CallNode node =

128
Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/calltree/CallTreePluginTest.java
View File

@@ -26,6 +26,7 @@ import java.util.concurrent.atomic.AtomicReference;
import javax.swing.JTree;
import javax.swing.tree.TreePath;
import org.apache.commons.collections4.map.LazyMap;
import org.junit.*;
import docking.ActionContext;
@@ -140,7 +141,10 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
function(0x4000, 0x5000);
function(0x5000, 0x6000);
function(0x5000, 0x6100);
duplicateReference(0x5000, 0x6000);
// second reference inside of function 0x5000 to 0x6000
createReference(0x5020, 0x6000);
function(0x6000, 0x7000);
function(0x6100, 0x7100);
function(0x7000, 0x8000);
@@ -150,14 +154,6 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
return builder.getProgram();
}
private void duplicateReference(int from, int to) {
// a bit of space so the function call is not at the entry point
int offset = from + 10;
while (!createReference(offset, to)) {
offset++;
}
}
private Function function(int addr) throws Exception {
return ensureFunction(addr);
}
@@ -598,7 +594,7 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
}
@Test
public void testFilterOutgoingDuplicates() {
public void testFilterOutgoingDuplicates_DifferentSource_SameDestination() {
//
// Test that the filter action will remove duplicate entries from the child nodes of
// the outgoing tree
@@ -607,6 +603,38 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
setProviderFunction("0x5000");
ToggleDockingAction filterDuplicatesAction =
(ToggleDockingAction) getAction("Filter Duplicates");
setToggleActionSelected(filterDuplicatesAction, new ActionContext(), true);
waitForTree(outgoingTree);
GTreeNode rootNode = getRootNode(outgoingTree);
boolean shouldHaveDuplicates = false;
Map<String, List<GTreeNode>> nameCountMap = createNameCountMap(rootNode);
assertDuplicateChildStatus(nameCountMap, shouldHaveDuplicates);
performAction(filterDuplicatesAction, true);// deselect
waitForTree(outgoingTree);
rootNode = getRootNode(outgoingTree);
nameCountMap = createNameCountMap(rootNode);
shouldHaveDuplicates = true;
assertDuplicateChildStatus(nameCountMap, shouldHaveDuplicates);
}
@Test
public void testFilterOutgoingDuplicates_SameSource_SameDestination() {
//
// Test that 2 references from the same source address to the same function will not get
// added to the tree, regardless of the duplicate filter state
//
// add a second reference (this is already defined in setup)
builder.createMemoryCallReference("0x5020", "0x6000");
setProviderFunction("0x5000");
ToggleDockingAction filterDuplicatesAction =
(ToggleDockingAction) getAction("Filter Duplicates");
@@ -614,15 +642,41 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
waitForTree(outgoingTree);
GTreeNode rootNode = getRootNode(outgoingTree);
List<GTreeNode> children = rootNode.getChildren();
assertTrue(
"Outgoing tree does not have callers as expected for function: " + getListingFunction(),
children.size() > 0);
Map<String, List<GTreeNode>> nameCountMap = createNameCountMap(rootNode);
// copy the names of the children into a map so that we can verify that there are
// no duplicates
// 1, not 2 entries (the exact duplicate and the duplicate destination are ignored)
assertEquals(1, nameCountMap.get("Function_6000").size());
performAction(filterDuplicatesAction, true);// deselect
waitForTree(outgoingTree);
rootNode = getRootNode(outgoingTree);
nameCountMap = createNameCountMap(rootNode);
// 2, not 3 entries (the exact duplicate is ignored)
assertEquals(2, nameCountMap.get("Function_6000").size());
}
@Test
public void testFilterOutgoingDuplicates_SameSource_DifferentDestination() {
//
// Test that 2 references from the same source address to the same function will not get
// added to the tree, regardless of the duplicate filter state
//
// add a second reference from 0x5020 to a function already called (see setup)
builder.createMemoryCallReference("0x5020", "0x6000");
setProviderFunction("0x5000");
ToggleDockingAction filterDuplicatesAction =
(ToggleDockingAction) getAction("Filter Duplicates");
setToggleActionSelected(filterDuplicatesAction, new ActionContext(), true);
waitForTree(outgoingTree);
GTreeNode rootNode = getRootNode(outgoingTree);
boolean shouldHaveDuplicates = false;
Map<GTreeNode, Integer> nameCountMap = createNameCountMap(rootNode);
Map<String, List<GTreeNode>> nameCountMap = createNameCountMap(rootNode);
assertDuplicateChildStatus(nameCountMap, shouldHaveDuplicates);
performAction(filterDuplicatesAction, true);// deselect
@@ -663,16 +717,10 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
setToggleActionSelected(filterDuplicatesAction, new ActionContext(), true);
waitForTree(incomingTree);
// copy the names of the children into a map so that we can verify no duplicates
GTreeNode rootNode = getRootNode(incomingTree);
List<GTreeNode> children = rootNode.getChildren();
assertTrue(
"Outgoing tree does not have callers as expected for function: " + getListingFunction(),
children.size() > 0);
// copy the names of the children into a map so that we can verify that there are
// no duplicates
boolean shouldHaveDuplicates = false;
Map<GTreeNode, Integer> nameCountMap = createNameCountMap(rootNode);
boolean shouldHaveDuplicates = true;
Map<String, List<GTreeNode>> nameCountMap = createNameCountMap(rootNode);
assertDuplicateChildStatus(nameCountMap, shouldHaveDuplicates);
}
@@ -1135,34 +1183,32 @@ public class CallTreePluginTest extends AbstractGhidraHeadedIntegrationTest {
return codeBrowserPlugin.getCurrentAddress();
}
private Map<GTreeNode, Integer> createNameCountMap(GTreeNode node) {
Map<GTreeNode, Integer> map = new HashMap<>();
private Map<String, List<GTreeNode>> createNameCountMap(GTreeNode node) {
Map<String, List<GTreeNode>> map = LazyMap.lazyMap(new HashMap<>(), k -> new ArrayList<>());
List<GTreeNode> children = node.getChildren();
for (GTreeNode child : children) {
Integer integer = map.get(child);
if (integer == null) {
integer = 0;
}
int asInt = integer;
asInt++;
map.put(child, asInt);
map.get(child.getName()).add(child);
}
return map;
}
private void assertDuplicateChildStatus(Map<GTreeNode, Integer> childCountMap,
private void assertDuplicateChildStatus(Map<String, List<GTreeNode>> childCountMap,
boolean shouldHaveDuplicates) {
boolean foundDuplicates = false;
Set<Entry<GTreeNode, Integer>> entrySet = childCountMap.entrySet();
for (Entry<GTreeNode, Integer> entry : entrySet) {
int value = entry.getValue();
if (value != 1) {
Set<Entry<String, List<GTreeNode>>> entrySet = childCountMap.entrySet();
String duplicateName = null;
for (Entry<String, List<GTreeNode>> entry : entrySet) {
List<GTreeNode> list = entry.getValue();
if (list.size() != 1) {
duplicateName = entry.getKey();
foundDuplicates = true;
break;
}
}
String errorMessage =
(shouldHaveDuplicates ? "Did not find " : "Found") + " duplicate child entries";
(shouldHaveDuplicates ? "Did not find " : "Found") + " duplicate child entries for '" +
duplicateName + "'";
assertEquals(errorMessage, shouldHaveDuplicates, foundDuplicates);
}