Merge remote-tracking branch

'origin/GP-0-dragonmacher-test-fixes-4-8-21--SQUASHED'
This commit is contained in:
ghidra1 2021-04-23 10:59:26 -04:00
commit 965dfcaa9b
17 changed files with 986 additions and 246 deletions

View file

@ -615,8 +615,4 @@ public class DBTraceProgramViewSymbolTable implements SymbolTable {
}
}
@Override
public Symbol createSymbolPlaceholder(Address address, long id) {
return new DBTracePlaceholderSymbol(symbolManager, id);
}
}

View file

@ -1,52 +0,0 @@
/* ###
* IP: GHIDRA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package ghidra.trace.database.symbol;
import ghidra.program.model.symbol.SymbolType;
public class DBTracePlaceholderSymbol extends AbstractDBTraceSymbol {
protected final long id;
public DBTracePlaceholderSymbol(DBTraceSymbolManager manager, long id) {
super(manager, null, null);
this.id = id;
}
@Override
public long getID() {
return id;
}
@Override
public SymbolType getSymbolType() {
return SymbolType.getSymbolType(DBTraceSymbolManager.unpackTypeID(id));
}
@Override
public boolean isPrimary() {
throw new UnsupportedOperationException();
}
@Override
public boolean setPrimary() {
throw new UnsupportedOperationException();
}
@Override
public Object getObject() {
throw new UnsupportedOperationException();
}
}

View file

@ -44,6 +44,7 @@ public class OneShotAnalysisCommand extends BackgroundCommand {
public boolean applyTo(DomainObject obj, TaskMonitor monitor) {
Program program = (Program) obj;
try {
monitor.setMessage(analyzer.getName());
return analyzer.added(program, set, monitor, log);
}
catch (CancelledException e) {

View file

@ -0,0 +1,228 @@
/* ###
* IP: GHIDRA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package ghidra.app.plugin.core.symtable;
import ghidra.program.model.address.Address;
import ghidra.program.model.listing.Program;
import ghidra.program.model.symbol.*;
import ghidra.program.util.ProgramLocation;
import ghidra.util.task.TaskMonitor;
/**
* A class that allows the symbol table plugin to locate deleted items by id
*/
class ProxySymbol implements Symbol {
private long id;
private Address address;
ProxySymbol(long id, Address address) {
this.id = id;
this.address = address;
}
@Override
public boolean equals(Object obj) {
if (!(obj instanceof Symbol)) {
return false;
}
if (obj == this) {
return true;
}
// this class is only ever equal if the id matches
Symbol s = (Symbol) obj;
if (getID() == s.getID()) {
return true;
}
return false;
}
@Override
public int hashCode() {
return (int) id;
}
@Override
public long getID() {
return id;
}
@Override
public Address getAddress() {
return address;
}
@Override
public SymbolType getSymbolType() {
throw new UnsupportedOperationException();
}
@Override
public ProgramLocation getProgramLocation() {
throw new UnsupportedOperationException();
}
@Override
public boolean isExternal() {
throw new UnsupportedOperationException();
}
@Override
public Object getObject() {
throw new UnsupportedOperationException();
}
@Override
public boolean isPrimary() {
throw new UnsupportedOperationException();
}
@Override
public boolean isValidParent(Namespace parent) {
throw new UnsupportedOperationException();
}
@Override
public String getName() {
throw new UnsupportedOperationException();
}
@Override
public String[] getPath() {
throw new UnsupportedOperationException();
}
@Override
public Program getProgram() {
throw new UnsupportedOperationException();
}
@Override
public String getName(boolean includeNamespace) {
throw new UnsupportedOperationException();
}
@Override
public Namespace getParentNamespace() {
throw new UnsupportedOperationException();
}
@Override
public Symbol getParentSymbol() {
throw new UnsupportedOperationException();
}
@Override
public boolean isDescendant(Namespace namespace) {
throw new UnsupportedOperationException();
}
@Override
public int getReferenceCount() {
throw new UnsupportedOperationException();
}
@Override
public boolean hasMultipleReferences() {
throw new UnsupportedOperationException();
}
@Override
public boolean hasReferences() {
throw new UnsupportedOperationException();
}
@Override
public Reference[] getReferences(TaskMonitor monitor) {
throw new UnsupportedOperationException();
}
@Override
public Reference[] getReferences() {
throw new UnsupportedOperationException();
}
@Override
public void setName(String newName, SourceType source) {
throw new UnsupportedOperationException();
}
@Override
public void setNamespace(Namespace newNamespace) {
throw new UnsupportedOperationException();
}
@Override
public void setNameAndNamespace(String newName, Namespace newNamespace, SourceType source) {
throw new UnsupportedOperationException();
}
@Override
public boolean delete() {
throw new UnsupportedOperationException();
}
@Override
public boolean isPinned() {
throw new UnsupportedOperationException();
}
@Override
public void setPinned(boolean pinned) {
throw new UnsupportedOperationException();
}
@Override
public boolean isDynamic() {
throw new UnsupportedOperationException();
}
@Override
public boolean setPrimary() {
throw new UnsupportedOperationException();
}
@Override
public boolean isExternalEntryPoint() {
throw new UnsupportedOperationException();
}
@Override
public boolean isGlobal() {
throw new UnsupportedOperationException();
}
@Override
public void setSource(SourceType source) {
throw new UnsupportedOperationException();
}
@Override
public SourceType getSource() {
throw new UnsupportedOperationException();
}
@Override
public boolean isDeleted() {
throw new UnsupportedOperationException();
}
@Override
public String toString() {
return getClass().getSimpleName() + "[id=" + id + ", address=" + address + "]";
}
}

View file

@ -110,6 +110,10 @@ public class SymbolReferenceModel extends AddressBasedTableModel<Reference> {
@Override
public void setProgram(Program prog) {
if (isDisposed) {
return;
}
if (prog == null) {
super.setProgram(null);
refManager = null;

View file

@ -15,71 +15,253 @@
*/
package ghidra.app.plugin.core.symtable;
import static docking.widgets.table.AddRemoveListItem.Type.*;
import java.util.*;
import docking.widgets.table.AddRemoveListItem;
import docking.widgets.table.threaded.TableAddRemoveStrategy;
import docking.widgets.table.threaded.TableData;
import docking.widgets.table.TableSortingContext;
import docking.widgets.table.threaded.*;
import ghidra.program.model.symbol.Symbol;
import ghidra.util.exception.CancelledException;
import ghidra.util.task.TaskMonitor;
/**
* This strategy attempts to optimize removal of db objects that have been deleted. The issue with
* deleted db objects is that they may no longer have their attributes, which means we cannot
* use any of those attributes that may have been used as the basis for sorting. We use the
* table's sort to perform a binary search of existing symbols for removal. If the binary search
* does not work, then removal operations will require slow list traversal. Additionally,
* some clients use proxy objects in add/remove list to signal which object needs to be removed,
* since the original object is no longer available to the client. Using these proxy objects
* in a binary search may lead to exceptions if the proxy has unsupported methods called when
* searching.
* The symbol table users a {@link ThreadedTableModel}. This model does not correctly function
* with data that can change outside of the table. The symbol table's row objects are the
* {@link Symbol} db objects. These db objects can be changed by the user and by analysis
* while table is loaded. The problem with this is that the table's sort can be broken when
* symbols are to be added, removed or re-inserted, as this process requires a binary search which
* will be broken if the criteria used to sort the data has changed. Effectively, a symbol
* change can break the binary search if that symbol stays in a previously sorted position, but
* has updated data that would put the symbol in a new position if sorted again. For example,
* if the table is sorted on name and the name of a symbol changes, then future uses of the
* binary search will be broken while that symbol is still in the position that matches its old
* name.
* <p>
* This issue has been around for quite some time. To completely fix this issue, each row object
* of the symbol table would need to be immutable, at least on the sort criteria. We could fix
* this in the future if the *mostly correct* sorting behavior is not good enough. For now, the
* client can trigger a re-sort (e.g., by opening and closing the table) to fix the slightly
* out-of-sort data.
* <p>
* The likelihood of the sort being inconsistent now relates directly to how many changed symbols
* are in the table at the time of an insert. The more changes symbols, the higher the chance
* of a stale/misplaced symbol being used during a binary search, thus producing an invalid insert
* position.
* <p>
* This strategy is setup to mitigate the number of invalid symbols in the table at the
* time the inserts are applied. The basic workflow of this algorithm is:
* <pre>
* 1) condense the add / remove requests to remove duplicate efforts
* 2) process all removes first
* --all pure removes
* --all removes as part of a re-insert
* 3) process all items that failed to remove due to the sort data changing
* 4) process all adds (this step will fail if the data contains mis-sorted items)
* --all adds as part of a re-insert
* --all pure adds
* </pre>
*
* <P>This strategy will has guilty knowledge of client proxy object usage. The proxy objects
* Step 3, processing failed removals, is done to avoid a brute force lookup at each removal
* request.
*
* <P>This strategy has knowledge of client proxy object usage. The proxy objects
* are coded such that the {@code hashCode()} and {@code equals()} methods will match those
* methods of the data's real objects.
*
* @param <T> the row type
*/
public class SymbolTableAddRemoveStrategy<T> implements TableAddRemoveStrategy<T> {
public class SymbolTableAddRemoveStrategy implements TableAddRemoveStrategy<Symbol> {
@Override
public void process(List<AddRemoveListItem<T>> addRemoveList, TableData<T> tableData,
public void process(List<AddRemoveListItem<Symbol>> addRemoveList, TableData<Symbol> tableData,
TaskMonitor monitor) throws CancelledException {
Set<AddRemoveListItem<Symbol>> items = coalesceAddRemoveItems(addRemoveList);
//
// Hash map the existing values so that we can use any object inside the add/remove list
// as a key into this map to get the matching existing value.
// as a key into this map to get the matching existing value. Using the existing value
// enables the binary search to work when the add/remove item is a proxy object, but the
// existing item still has the data used to sort it. If the sort data has changed, then
// even this step will not allow the TableData to find the item in a search.
//
Map<T, T> hashed = new HashMap<>();
for (T t : tableData) {
hashed.put(t, t);
Map<Symbol, Symbol> hashed = new HashMap<>();
for (Symbol symbol : tableData) {
hashed.put(symbol, symbol);
}
int n = addRemoveList.size();
monitor.setMessage("Adding/Removing " + n + " items...");
Set<Symbol> failedToRemove = new HashSet<>();
int n = items.size();
monitor.setMessage("Removing " + n + " items...");
monitor.initialize(n);
for (int i = 0; i < n; i++) {
AddRemoveListItem<T> item = addRemoveList.get(i);
T value = item.getValue();
Iterator<AddRemoveListItem<Symbol>> it = items.iterator();
while (it.hasNext()) {
AddRemoveListItem<Symbol> item = it.next();
Symbol value = item.getValue();
if (item.isChange()) {
T toRemove = hashed.get(value);
if (toRemove != null) {
tableData.remove(toRemove);
}
tableData.insert(value);
Symbol toRemove = hashed.remove(value);
remove(tableData, toRemove, failedToRemove);
monitor.incrementProgress(1);
}
else if (item.isRemove()) {
T toRemove = hashed.get(value);
if (toRemove != null) {
tableData.remove(toRemove);
Symbol toRemove = hashed.remove(value);
remove(tableData, toRemove, failedToRemove);
it.remove();
}
monitor.checkCanceled();
}
if (!failedToRemove.isEmpty()) {
int size = failedToRemove.size();
String message = size == 1 ? "1 old symbol..." : size + " old symbols...";
monitor.setMessage("Removing " + message);
tableData.process((data, sortContext) -> {
return expungeLostItems(failedToRemove, data, sortContext);
});
}
n = items.size();
monitor.setMessage("Adding " + n + " items...");
it = items.iterator();
while (it.hasNext()) {
AddRemoveListItem<Symbol> item = it.next();
Symbol value = item.getValue();
if (item.isChange()) {
tableData.insert(value);
hashed.put(value, value);
}
else if (item.isAdd()) {
tableData.insert(value);
hashed.put(value, value);
}
monitor.checkCanceled();
monitor.setProgress(i);
monitor.incrementProgress(1);
}
monitor.setMessage("Done adding/removing");
}
private Set<AddRemoveListItem<Symbol>> coalesceAddRemoveItems(
List<AddRemoveListItem<Symbol>> addRemoveList) {
Map<Long, AddRemoveListItem<Symbol>> map = new HashMap<>();
for (AddRemoveListItem<Symbol> item : addRemoveList) {
if (item.isChange()) {
handleChange(item, map);
}
else if (item.isAdd()) {
handleAdd(item, map);
}
else {
handleRemove(item, map);
}
}
return new HashSet<>(map.values());
}
private void handleAdd(AddRemoveListItem<Symbol> item,
Map<Long, AddRemoveListItem<Symbol>> map) {
long id = item.getValue().getID();
AddRemoveListItem<Symbol> existing = map.get(id);
if (existing == null) {
map.put(id, item);
return;
}
if (existing.isChange()) {
return; // change -> add; keep the change
}
if (existing.isAdd()) {
return; // already an add
}
// remove -> add; make a change
map.put(id, new AddRemoveListItem<>(CHANGE, existing.getValue()));
}
private void handleRemove(AddRemoveListItem<Symbol> item,
Map<Long, AddRemoveListItem<Symbol>> map) {
long id = item.getValue().getID();
AddRemoveListItem<Symbol> existing = map.get(id);
if (existing == null) {
map.put(id, item);
return;
}
if (existing.isChange()) {
map.put(id, item); // change -> remove; just do the remove
return;
}
if (existing.isRemove()) {
return;
}
// add -> remove; do no work
map.remove(id);
}
private void handleChange(AddRemoveListItem<Symbol> item,
Map<Long, AddRemoveListItem<Symbol>> map) {
long id = item.getValue().getID();
AddRemoveListItem<Symbol> existing = map.get(id);
if (existing == null) {
map.put(id, item);
return;
}
if (!existing.isChange()) {
// either add or remove followed by a change; keep the change
map.put(id, item);
}
// otherwise, we had a change followed by a change; keep just 1 change
}
private void remove(TableData<Symbol> tableData, Symbol symbol, Set<Symbol> failedToRemove) {
if (symbol == null) {
return;
}
if (!tableData.remove(symbol)) {
failedToRemove.add(symbol);
}
}
/*
* Removes the given set of items that were unsuccessfully removed from the table as part of
* the add/remove process. These items could not be removed because some part of their
* state has changed such that the binary search performed during the normal remove process
* cannot locate the item in the table data. This algorithm will check the given set of
* items against the entire list of table data, locating the item to be removed.
*/
private List<Symbol> expungeLostItems(Set<Symbol> toRemove, List<Symbol> data,
TableSortingContext<Symbol> sortContext) {
if (sortContext.isUnsorted()) {
// this can happen if the data is unsorted and we were asked to remove an item that
// was never in the table for some reason
return data;
}
// Copy to a new list those items that are not marked for removal. This saves the
// list move its items every time a remove takes place
List<Symbol> newList = new ArrayList<>(data.size() - toRemove.size());
for (int i = 0; i < data.size(); i++) {
Symbol rowObject = data.get(i);
if (!toRemove.contains(rowObject)) {
newList.add(rowObject);
}
}
return newList;
}
}

View file

@ -62,7 +62,7 @@ class SymbolTableModel extends AddressBasedTableModel<Symbol> {
private Symbol lastSymbol;
private SymbolFilter filter;
private TableAddRemoveStrategy<Symbol> deletedDbObjectAddRemoveStrategy =
new SymbolTableAddRemoveStrategy<>();
new SymbolTableAddRemoveStrategy();
SymbolTableModel(SymbolProvider provider, PluginTool tool) {
super("Symbols", tool, null, null);

View file

@ -609,8 +609,7 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
protected void doRun() {
SymbolTable symbolTable = currentProgram.getSymbolTable();
Symbol removedSymbol =
symbolTable.createSymbolPlaceholder(address, symbolId);
Symbol removedSymbol = new ProxySymbol(symbolId, address);
symProvider.symbolRemoved(removedSymbol);
refProvider.symbolRemoved(removedSymbol);
@ -741,7 +740,7 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
if (symProvider.isShowingDynamicSymbols()) {
long id = symbolTable.getDynamicSymbolID(reference.getToAddress());
Symbol removedSymbol = symbolTable.createSymbolPlaceholder(toAddr, id);
Symbol removedSymbol = new ProxySymbol(id, toAddr);
symProvider.symbolRemoved(removedSymbol);
refProvider.symbolRemoved(removedSymbol);
}

View file

@ -0,0 +1,375 @@
/* ###
* IP: GHIDRA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package ghidra.app.plugin.core.symtable;
import static docking.widgets.table.AddRemoveListItem.Type.*;
import static org.junit.Assert.*;
import java.util.*;
import org.junit.Before;
import org.junit.Test;
import docking.widgets.table.*;
import docking.widgets.table.threaded.TestTableData;
import ghidra.program.model.address.Address;
import ghidra.program.model.address.TestAddress;
import ghidra.program.model.symbol.Symbol;
import ghidra.util.task.TaskMonitor;
public class SymbolTableAddRemoveStrategyTest {
private SymbolTableAddRemoveStrategy strategy;
private SpyTableData spyTableData;
private List<Symbol> modelData;
@Before
public void setUp() throws Exception {
strategy = new SymbolTableAddRemoveStrategy();
modelData = createModelData();
spyTableData = createTableData();
}
private SpyTableData createTableData() {
Comparator<Symbol> comparator = (s1, s2) -> {
return s1.getName().compareTo(s2.getName());
};
TableSortState sortState = TableSortState.createDefaultSortState(0);
TableSortingContext<Symbol> sortContext =
new TableSortingContext<>(sortState, comparator);
modelData.sort(comparator);
return new SpyTableData(modelData, sortContext);
}
private List<Symbol> createModelData() {
List<Symbol> data = new ArrayList<>();
data.add(new TestSymbol(1, new TestAddress(101)));
data.add(new TestSymbol(2, new TestAddress(102)));
data.add(new TestSymbol(3, new TestAddress(103)));
data.add(new TestSymbol(4, new TestAddress(104)));
return data;
}
@Test
public void testRemove_DifferentInstance_SameId() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol s = new TestSymbol(1, new TestAddress(101));
addRemoves.add(new AddRemoveListItem<>(REMOVE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(0, spyTableData.getInsertCount());
}
@Test
public void testInsert_NewSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol newSymbol = new TestSymbol(10, new TestAddress(1010));
addRemoves.add(new AddRemoveListItem<>(ADD, newSymbol));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
assertEquals(0, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testInsertAndRemove_NewSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol newSymbol = new TestSymbol(10, new TestAddress(1010));
addRemoves.add(new AddRemoveListItem<>(ADD, newSymbol));
addRemoves.add(new AddRemoveListItem<>(REMOVE, newSymbol));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// no work was done, since the insert was followed by a remove
assertEquals(0, spyTableData.getRemoveCount());
assertEquals(0, spyTableData.getInsertCount());
}
@Test
public void testChange_NewSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol newSymbol = new TestSymbol(10, new TestAddress(1010));
addRemoves.add(new AddRemoveListItem<>(CHANGE, newSymbol));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// no remove, since the symbol was not in the table
assertEquals(0, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testChange_ExisingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// no remove, since the symbol was not in the table
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testRemoveAndInsert_NewSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol newSymbol = new TestSymbol(10, new TestAddress(1010));
addRemoves.add(new AddRemoveListItem<>(REMOVE, newSymbol));
addRemoves.add(new AddRemoveListItem<>(ADD, newSymbol));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the remove does not happen, since the time was not in the table
assertEquals(0, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testRemoveAndInsert_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(REMOVE, s));
addRemoves.add(new AddRemoveListItem<>(ADD, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the remove does not happen, since the time was not in the table
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testChangeAndInsert_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
addRemoves.add(new AddRemoveListItem<>(ADD, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the insert portions get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testChangeAndRemove_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
addRemoves.add(new AddRemoveListItem<>(REMOVE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the remove portions get coalesced; no insert takes place
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(0, spyTableData.getInsertCount());
}
@Test
public void testChangeAndChange_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the changes get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testRemoveAndRemove_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(REMOVE, s));
addRemoves.add(new AddRemoveListItem<>(REMOVE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the removes get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(0, spyTableData.getInsertCount());
}
@Test
public void testInsertAndInsert_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(ADD, s));
addRemoves.add(new AddRemoveListItem<>(ADD, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the inserts get coalesced
assertEquals(0, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testInsertAndChange_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(ADD, s));
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the insert portions get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testRemoveAndChange_ExistingSymbol() throws Exception {
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
Symbol s = modelData.get(0);
addRemoves.add(new AddRemoveListItem<>(REMOVE, s));
addRemoves.add(new AddRemoveListItem<>(CHANGE, s));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the remove portions get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
@Test
public void testLostItems_Remove() throws Exception {
//
// Test that symbols get removed when the data up on which they are sorted changes before
// the removal takes place
//
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol symbol = (TestSymbol) modelData.get(0);
symbol.setName("UpdatedName");
addRemoves.add(new AddRemoveListItem<>(REMOVE, symbol));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the insert portions get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(0, spyTableData.getInsertCount());
}
@Test
public void testLostItems_Change() throws Exception {
//
// Test that symbols get removed when the data up on which they are sorted changes before
// the removal takes place
//
List<AddRemoveListItem<Symbol>> addRemoves = new ArrayList<>();
TestSymbol symbol = (TestSymbol) modelData.get(0);
symbol.setName("UpdatedName");
addRemoves.add(new AddRemoveListItem<>(CHANGE, symbol));
strategy.process(addRemoves, spyTableData, TaskMonitor.DUMMY);
// the insert portions get coalesced
assertEquals(1, spyTableData.getRemoveCount());
assertEquals(1, spyTableData.getInsertCount());
}
//==================================================================================================
// Inner Classes
//==================================================================================================
private class SpyTableData extends TestTableData<Symbol> {
private int removeCount;
private int insertCount;
SpyTableData(List<Symbol> data, TableSortingContext<Symbol> sortContext) {
super(data, sortContext);
}
@Override
public boolean remove(Symbol t) {
removeCount++;
return super.remove(t);
}
@Override
public void insert(Symbol value) {
insertCount++;
super.insert(value);
}
int getRemoveCount() {
return removeCount;
}
int getInsertCount() {
return insertCount;
}
}
private class TestSymbol extends ProxySymbol {
private String name;
TestSymbol(long id, Address address) {
super(id, address);
name = id + "@" + address;
}
void setName(String name) {
this.name = name;
}
@Override
public String getName() {
return name;
}
}
}

View file

@ -152,28 +152,6 @@ public class SymbolTablePluginTest extends AbstractGhidraHeadedIntegrationTest {
}
}
private void sortAscending(int column) {
runSwing(() -> symbolModel.setTableSortState(
TableSortState.createDefaultSortState(column, true)));
waitForTableModel(symbolModel);
waitForCondition(() -> {
TableSortState sort = runSwing(() -> symbolModel.getTableSortState());
return sort.getColumnSortState(column).isAscending();
});
}
private void sortDescending(int column) {
runSwing(() -> symbolModel.setTableSortState(
TableSortState.createDefaultSortState(column, false)));
waitForTableModel(symbolModel);
waitForCondition(() -> {
TableSortState sort = runSwing(() -> symbolModel.getTableSortState());
return !sort.getColumnSortState(column).isAscending();
});
}
@Test
public void testColumnDiscovery() throws Exception {
//
@ -449,20 +427,19 @@ public class SymbolTablePluginTest extends AbstractGhidraHeadedIntegrationTest {
// 1 Function label removed (1 dynamic added at function entry)
// Locals were promoted to global.
assertEquals(rowCount, symbolTable.getRowCount());
int newDynamicSymbolRow = findRow("SUB_00000052");
assertFalse(newDynamicSymbolRow == -1);
assertNotEquals(-1, newDynamicSymbolRow);
assertEquals(rowCount, symbolTable.getRowCount());
int anotherLocal_RowIndex = findRow("AnotherLocal");
selectRow(anotherLocal_RowIndex);
performAction(deleteAction, true);
waitForNotBusy();
anotherLocalSymbol = getUniqueSymbol(program, "AnotherLocal");
assertNull("Delete action did not delete symbol: " + anotherLocalSymbol,
anotherLocalSymbol);// AnotherLocal should have been promoted to global since user defined.
waitForNotBusy();
// 1 Data label removed
int expected = rowCount - 1;
assertEquals(expected, symbolTable.getRowCount());
@ -654,6 +631,29 @@ public class SymbolTablePluginTest extends AbstractGhidraHeadedIntegrationTest {
assertEquals(rowCount + 1, symbolModel.getRowCount());
}
@Test
public void testRenameUpdatesSort() throws Exception {
openProgram("sample");
waitForNotBusy();
//
// Functions: 'ghidra', 'func_with_parms'
//
assertEquals(25, symbolTable.getRowCount());
Symbol symbol = getUniqueSymbol(program, "ghidra");
int rowIndex = indexOf(symbol);
setName(symbol, "____ghidra", SourceType.DEFAULT);
assertEquals(25, symbolTable.getRowCount());
int updatedRowIndex = indexOf(symbol);
assertNotEquals(rowIndex, updatedRowIndex);
assertEquals(0, updatedRowIndex);
}
@Test
public void testDefaultFunctionToNamedFunctionWithFilterOn() throws Exception {
openProgram("sample");
@ -758,20 +758,6 @@ public class SymbolTablePluginTest extends AbstractGhidraHeadedIntegrationTest {
assertEquals(3, refCount.intValue());
}
private Reference getReference(Address from, Address to) {
ReferenceManager rm = program.getReferenceManager();
Reference[] refs = rm.getReferencesFrom(from);
for (Reference element : refs) {
if (to.equals(element.getToAddress())) {
return element;
}
}
fail("Did not find expected mem reference between " + from + " and " + to);
return null;
}
@Test
public void testUpdateOnProgramRestore() throws Exception {
openProgram("sample");
@ -1134,6 +1120,46 @@ public class SymbolTablePluginTest extends AbstractGhidraHeadedIntegrationTest {
// Helper methods
//==================================================================================================
private Reference getReference(Address from, Address to) {
ReferenceManager rm = program.getReferenceManager();
Reference[] refs = rm.getReferencesFrom(from);
for (Reference element : refs) {
if (to.equals(element.getToAddress())) {
return element;
}
}
fail("Did not find expected mem reference between " + from + " and " + to);
return null;
}
private void sortAscending(int column) {
runSwing(() -> symbolModel.setTableSortState(
TableSortState.createDefaultSortState(column, true)));
waitForTableModel(symbolModel);
waitForCondition(() -> {
TableSortState sort = runSwing(() -> symbolModel.getTableSortState());
return sort.getColumnSortState(column).isAscending();
});
}
private void sortDescending(int column) {
runSwing(() -> symbolModel.setTableSortState(
TableSortState.createDefaultSortState(column, false)));
waitForTableModel(symbolModel);
waitForCondition(() -> {
TableSortState sort = runSwing(() -> symbolModel.getTableSortState());
return !sort.getColumnSortState(column).isAscending();
});
}
private int indexOf(Symbol symbol) {
return runSwing(() -> symbolModel.getRowIndex(symbol));
}
private void removeReference(String from, String to) {
ReferenceManager rm = program.getReferenceManager();
@ -1634,9 +1660,13 @@ public class SymbolTablePluginTest extends AbstractGhidraHeadedIntegrationTest {
}
private int findRow(String symbolName, String namespace) {
waitForSwing();
int max = symbolTable.getRowCount();
for (int i = 0; i < max; i++) {
Symbol s = (Symbol) symbolTable.getValueAt(i, SymbolTableModel.LABEL_COL);
if (s == null) {
continue; // symbol deleted
}
if (symbolName.equals(s.getName()) &&
namespace.equals(s.getParentNamespace().getName())) {
return i;

View file

@ -15,27 +15,41 @@
*/
package docking.widgets.table;
/**
* An object that represents and add, remove or change operation for one row of a table
*
* @param <T> the row type
*/
public class AddRemoveListItem<T> {
private boolean isAdd;
private boolean isRemove;
private T value;
public AddRemoveListItem(boolean isAdd, boolean isRemove, T value) {
this.isAdd = isAdd;
this.isRemove = isRemove;
public enum Type {
ADD,
REMOVE,
CHANGE
}
private T value;
private Type type;
public AddRemoveListItem(Type type, T value) {
this.type = type;
this.value = value;
}
public boolean isAdd() {
return isAdd;
return type == Type.ADD;
}
public boolean isRemove() {
return isRemove;
return type == Type.REMOVE;
}
public boolean isChange() {
return isAdd && isRemove;
return type == Type.CHANGE;
}
public Type getType() {
return type;
}
public T getValue() {
@ -48,8 +62,7 @@ public class AddRemoveListItem<T> {
//@formatter:off
return "{\n" +
"\tvalue: " + value +",\n" +
"\tisAdd: " + isAdd +",\n" +
"\tisRemove: " + isRemove +"\n" +
"\ttype: " + type +",\n" +
"}";
//@formatter:on
}

View file

@ -16,6 +16,7 @@
package docking.widgets.table.threaded;
import java.util.*;
import java.util.function.BiFunction;
import docking.widgets.table.TableFilter;
import docking.widgets.table.TableSortingContext;
@ -70,7 +71,7 @@ public class TableData<ROW_OBJECT> implements Iterable<ROW_OBJECT> {
// no source; no data; no sort
}
private TableData(List<ROW_OBJECT> data, TableSortingContext<ROW_OBJECT> sortContext) {
TableData(List<ROW_OBJECT> data, TableSortingContext<ROW_OBJECT> sortContext) {
this.data = data;
this.sortContext = sortContext;
}
@ -169,16 +170,30 @@ public class TableData<ROW_OBJECT> implements Iterable<ROW_OBJECT> {
return true;
}
//
// At this point we have one of 2 conditions: the object is not in the list, or the object
// does not work with the current sort comparator.
//
// There are cases where the comparator will not work for the object handed to this method,
// such as when some db objects get deleted and the client uses a proxy object to perform
// the delete. To handle these odd cases, we still have to brute-force search. If this
// proves to be a bottleneck, then we can update how we handle item removal.
//
return data.remove(t);
// We used to have code that pass proxy objects to this class to remove items. That code
// has been updated to no longer pass proxy objects. Leaving this code here for a while
// just in case we find another client doing the same thing.
// return data.remove(t);
return false;
}
/**
* A generic method that allows clients to process the contents of this table data. This
* method is not synchronized and should only be called from a {@link TableUpdateJob} or
* one of its callbacks.
*
* <P>Note: this method will do nothing if the data is not sorted.
*
* @param function the consumer of the data and the current sort context
*/
public void process(
BiFunction<List<ROW_OBJECT>, TableSortingContext<ROW_OBJECT>, List<ROW_OBJECT>> function) {
if (source != null) {
source.process(function);
}
data = function.apply(data, sortContext);
}
/**

View file

@ -15,6 +15,8 @@
*/
package docking.widgets.table.threaded;
import static docking.widgets.table.AddRemoveListItem.Type.*;
import java.util.*;
import javax.swing.SwingUtilities;
@ -499,7 +501,7 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>
* @param obj the object for which to schedule the update
*/
public void updateObject(ROW_OBJECT obj) {
updateManager.addRemove(new AddRemoveListItem<>(true, true, obj));
updateManager.addRemove(new AddRemoveListItem<>(CHANGE, obj));
}
/**
@ -507,7 +509,7 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>
* @param obj the object to add
*/
public void addObject(ROW_OBJECT obj) {
updateManager.addRemove(new AddRemoveListItem<>(true, false, obj));
updateManager.addRemove(new AddRemoveListItem<>(ADD, obj));
}
/**
@ -526,7 +528,7 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>
* @param obj the object to remove
*/
public void removeObject(ROW_OBJECT obj) {
updateManager.addRemove(new AddRemoveListItem<>(false, true, obj));
updateManager.addRemove(new AddRemoveListItem<>(REMOVE, obj));
}
protected void updateNow() {

View file

@ -0,0 +1,32 @@
/* ###
* IP: GHIDRA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package docking.widgets.table.threaded;
import java.util.List;
import docking.widgets.table.TableSortingContext;
/**
* A version of {@link TableData} that can be used for testing
*
* @param <ROW_OBJECT> the row type
*/
public class TestTableData<ROW_OBJECT> extends TableData<ROW_OBJECT> {
public TestTableData(List<ROW_OBJECT> data, TableSortingContext<ROW_OBJECT> sortContext) {
super(data, sortContext);
}
}

View file

@ -30,7 +30,6 @@ import ghidra.program.model.listing.CircularDependencyException;
import ghidra.program.model.listing.Program;
import ghidra.program.model.symbol.*;
import ghidra.program.util.ChangeManager;
import ghidra.program.util.ProgramLocation;
import ghidra.util.Lock;
import ghidra.util.SystemUtilities;
import ghidra.util.exception.DuplicateNameException;
@ -52,20 +51,6 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
private volatile String cachedName;
private volatile long cachedNameModCount;
/**
* Creates a Symbol that is just a placeholder for use when trying to find symbols by using
* {@link Symbol#getID()}. This is useful for locating symbols in Java collections when
* a symbol has been deleted and the only remaining information is that symbol's ID.
*
* @param manager the manager for the new symbol
* @param address the address of the symbol
* @param id the id of the symbol
* @return the fake symbol
*/
static SymbolDB createSymbolPlaceholder(SymbolManager manager, Address address, long id) {
return new PlaceholderSymbolDB(manager, address, id);
}
SymbolDB(SymbolManager symbolMgr, DBObjectCache<SymbolDB> cache, Address address,
DBRecord record) {
super(cache, record.getKey());
@ -941,58 +926,4 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
this.record = record;
keyChanged(record.getKey());
}
private static class PlaceholderSymbolDB extends SymbolDB {
PlaceholderSymbolDB(SymbolManager symbolMgr, Address address, long key) {
super(symbolMgr, null, address, key);
}
@Override
public boolean equals(Object obj) {
if ((obj == null) || (!(obj instanceof Symbol))) {
return false;
}
if (obj == this) {
return true;
}
// this class is only ever equal if the id matches
Symbol s = (Symbol) obj;
if (getID() == s.getID()) {
return true;
}
return false;
}
@Override
public SymbolType getSymbolType() {
throw new IllegalArgumentException();
}
@Override
public ProgramLocation getProgramLocation() {
throw new IllegalArgumentException();
}
@Override
public boolean isExternal() {
throw new IllegalArgumentException();
}
@Override
public Object getObject() {
throw new IllegalArgumentException();
}
@Override
public boolean isPrimary() {
throw new IllegalArgumentException();
}
@Override
public boolean isValidParent(Namespace parent) {
throw new IllegalArgumentException();
}
}
}

View file

@ -2400,11 +2400,6 @@ public class SymbolManager implements SymbolTable, ManagerDB {
}
}
@Override
public Symbol createSymbolPlaceholder(Address address, long id) {
return SymbolDB.createSymbolPlaceholder(this, address, id);
}
/**
* Creates a symbol, specifying all information for the record. This method is not on the
* public interface and is only intended for program API internal use. The user of this

View file

@ -540,17 +540,6 @@ public interface SymbolTable {
public Namespace createNameSpace(Namespace parent, String name, SourceType source)
throws DuplicateNameException, InvalidInputException;
/**
* Creates a Symbol that is just a placeholder for use when trying to find symbols by using
* {@link Symbol#getID()}. This is useful for locating symbols in Java collections when
* a symbol has been deleted and the only remaining information is that symbol's ID.
*
* @param address the address of the symbol
* @param id the id of the symbol
* @return the fake symbol
*/
public Symbol createSymbolPlaceholder(Address address, long id);
/**
* Converts the given namespace to a class namespace
*