Merge remote-tracking branch

'origin/GP-7220_dragonmacher_PR-9556_axd1x8a_feat_vt-conflict-option'
(Closes #9556)
This commit is contained in:
Ryan Kurtz
2026-09-16 13:12:23 +00:00
7 changed files with 244 additions and 35 deletions

Binary file not shown.

Before

Width:  |  Height:  |  Size: 58 KiB

After

Width:  |  Height:  |  Size: 73 KiB

View File

@@ -25,6 +25,7 @@ import ghidra.feature.vt.api.stringable.DataTypeStringable;
import ghidra.feature.vt.api.util.Stringable;
import ghidra.feature.vt.api.util.VersionTrackingApplyException;
import ghidra.feature.vt.gui.util.VTMatchApplyChoices;
import ghidra.feature.vt.gui.util.VTMatchApplyChoices.DataTypeConflictChoices;
import ghidra.feature.vt.gui.util.VTMatchApplyChoices.ReplaceDataChoices;
import ghidra.feature.vt.gui.util.VTOptionDefines;
import ghidra.framework.options.Options;
@@ -206,7 +207,8 @@ public class DataTypeMarkupType extends VTMarkupType {
}
private boolean setDataType(Program program, Address startAddress, DataType dataType,
int dataLength, VTMatchApplyChoices.ReplaceDataChoices replaceChoice)
int dataLength, VTMatchApplyChoices.ReplaceDataChoices replaceChoice,
DataTypeConflictHandler conflictHandler)
throws CodeUnitInsertionException, VersionTrackingApplyException {
Listing listing = program.getListing();
@@ -260,15 +262,26 @@ public class DataTypeMarkupType extends VTMarkupType {
}
if (replaceFirstOnly && hasOtherDefinedData) {
// Just return since we are only replacing first data and this has some defined
// Just return since we are only replacing first data and this has some defined
// data that would be overwritten following the first data in the destination.
return false;
}
/*
Note: the resolve may add a .conflict type. That will not be removed if an exception is
thrown. Also if an unapply is executed, .conflict types will not be removed. For now,
we leave this up to the user to fix, should they care. Trying to remove .conflict types
during an unapply may have unintended side-effects if the user added new uses of that
conflict type.
*/
ProgramBasedDataTypeManager dtm = program.getDataTypeManager();
DataType resolvedDataType = dtm.resolve(dataType, conflictHandler);
listing.clearCodeUnits(startAddress, endAddress, false);
try {
listing.createData(startAddress, dataType, dataLength);
listing.createData(startAddress, resolvedDataType, dataLength);
}
catch (CodeUnitInsertionException e) {
tryToRestoreOriginalData(listing, startAddress, originalDataType, originalDataLength);
@@ -334,9 +347,17 @@ public class DataTypeMarkupType extends VTMarkupType {
sourceDataLength = sourceData.getLength();
}
DataTypeConflictChoices conflictChoice = markupOptions.getEnum(
VTOptionDefines.DATA_TYPE_CONFLICT_HANDLER,
VTOptionDefines.DEFAULT_OPTION_FOR_DATA_TYPE_CONFLICT_HANDLER);
DataTypeConflictHandler conflictHandler = switch (conflictChoice) {
case USE_EXISTING -> DataTypeConflictHandler.KEEP_HANDLER;
case RENAME_AND_ADD -> DataTypeConflictHandler.DEFAULT_HANDLER;
};
try {
return setDataType(destinationProgram, destinationAddress, sourceDataType,
sourceDataLength, replaceChoice);
sourceDataLength, replaceChoice, conflictHandler);
}
catch (CodeUnitInsertionException e) {
throw new VersionTrackingApplyException(getApplyFailedMessage(sourceAddress,
@@ -404,7 +425,8 @@ public class DataTypeMarkupType extends VTMarkupType {
try {
setDataType(destinationProgram, destinationAddress, originalDataType,
originalDataLength, VTMatchApplyChoices.ReplaceDataChoices.REPLACE_ALL_DATA);
originalDataLength, VTMatchApplyChoices.ReplaceDataChoices.REPLACE_ALL_DATA,
DataTypeConflictHandler.DEFAULT_HANDLER);
}
catch (CodeUnitInsertionException e) {
throw new VersionTrackingApplyException("Couldn't unapply data type markup @ " +

View File

@@ -51,6 +51,9 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
// help tooltips
private static final String DATA_MATCH_DATA_TYPE_TOOLTIP =
"<html>The apply action for the <b>data type on a data match</b> when performing bulk apply operations</html>";
private static final String DATA_TYPE_CONFLICT_HANDLER_TOOLTIP =
"<html>How to resolve a conflict when the <b>data type being applied</b> already exists " +
"but differs in the destination program</html>";
private static final String LABELS_TOOLTIP =
"<html>The apply action for <b>labels</b> when performing bulk apply operations</html>";
private static final String FUNCTION_NAME_TOOLTIP =
@@ -116,6 +119,7 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
private JComponent editorComponent;
private JLabel dataMatchDataTypeLabel;
private JLabel dataTypeConflictHandlerLabel;
private JLabel functionNameLabel;
private JLabel functionSignatureLabel;
private JLabel useFunctionNamespaceLabel;
@@ -137,6 +141,7 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
private JLabel postCommentsLabel;
private JComboBox<Enum<?>> dataMatchDataTypeComboBox;
private JComboBox<Enum<?>> dataTypeConflictHandlerComboBox;
private JComboBox<Enum<?>> functionNameComboBox;
private JComboBox<Enum<?>> functionSignatureComboBox;
private JCheckBox useFunctionNamespaceCheckBox;
@@ -418,6 +423,8 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
panel.add(dataMatchDataTypeLabel);
panel.add(dataMatchDataTypeComboBox);
panel.add(dataTypeConflictHandlerLabel);
panel.add(dataTypeConflictHandlerComboBox);
panel.add(labelsLabel);
panel.add(labelsComboBox);
panel.add(functionNameLabel);
@@ -439,6 +446,10 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
dataMatchDataTypeLabel = new GDLabel("Data Match Data Type", SwingConstants.RIGHT);
dataMatchDataTypeLabel.setToolTipText(DATA_MATCH_DATA_TYPE_TOOLTIP);
dataTypeConflictHandlerLabel =
new GDLabel("Data Type Conflict Handler", SwingConstants.RIGHT);
dataTypeConflictHandlerLabel.setToolTipText(DATA_TYPE_CONFLICT_HANDLER_TOOLTIP);
labelsLabel = new GDLabel("Labels", SwingConstants.RIGHT);
labelsLabel.setToolTipText(LABELS_TOOLTIP);
@@ -460,6 +471,10 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
DEFAULT_OPTION_FOR_DATA_MATCH_DATA_TYPE);
dataMatchDataTypeComboBox.setToolTipText(DATA_MATCH_DATA_TYPE_TOOLTIP);
dataTypeConflictHandlerComboBox = createComboBox(VTOptionDefines.DATA_TYPE_CONFLICT_HANDLER,
DEFAULT_OPTION_FOR_DATA_TYPE_CONFLICT_HANDLER);
dataTypeConflictHandlerComboBox.setToolTipText(DATA_TYPE_CONFLICT_HANDLER_TOOLTIP);
labelsComboBox = createComboBox(VTOptionDefines.LABELS, DEFAULT_OPTION_FOR_LABELS);
labelsComboBox.setToolTipText(LABELS_TOOLTIP);
@@ -556,6 +571,10 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
(ReplaceDataChoices) dataMatchDataTypeComboBox.getSelectedItem();
options.setEnum(DATA_MATCH_DATA_TYPE, dataMatchDataTypeChoice);
DataTypeConflictChoices dataTypeConflictHandlerChoice =
(DataTypeConflictChoices) dataTypeConflictHandlerComboBox.getSelectedItem();
options.setEnum(DATA_TYPE_CONFLICT_HANDLER, dataTypeConflictHandlerChoice);
LabelChoices labelsChoice = (LabelChoices) labelsComboBox.getSelectedItem();
options.setEnum(LABELS, labelsChoice);
@@ -666,6 +685,12 @@ public class ApplyMarkupPropertyEditor implements OptionsEditor {
dataMatchDataTypeComboBox.setSelectedItem(dataMatchDataTypeChoice);
}
DataTypeConflictChoices dataTypeConflictHandlerChoice = options
.getEnum(DATA_TYPE_CONFLICT_HANDLER, DEFAULT_OPTION_FOR_DATA_TYPE_CONFLICT_HANDLER);
if (dataTypeConflictHandlerChoice != dataTypeConflictHandlerComboBox.getSelectedItem()) {
dataTypeConflictHandlerComboBox.setSelectedItem(dataTypeConflictHandlerChoice);
}
LabelChoices labelsChoice = options.getEnum(LABELS, DEFAULT_OPTION_FOR_LABELS);
if (labelsChoice != labelsComboBox.getSelectedItem()) {
labelsComboBox.setSelectedItem(labelsChoice);

View File

@@ -686,6 +686,11 @@ public class VTMatchTableProvider extends ComponentProviderAdapter
null,
"The default apply action <b>for the data type on a data match</b> when performing bulk apply operations");
vtOptions.registerOption(DATA_TYPE_CONFLICT_HANDLER,
DEFAULT_OPTION_FOR_DATA_TYPE_CONFLICT_HANDLER, null,
"How to resolve a conflict when the data type being applied already exists but " +
"differs in the destination program");
vtOptions.registerOption(LABELS, DEFAULT_OPTION_FOR_LABELS, null,
"The default apply action <b>for labels</b> when performing bulk apply operations");

View File

@@ -4,9 +4,9 @@
* 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.
@@ -239,4 +239,20 @@ public class VTMatchApplyChoices {
}
}
public static enum DataTypeConflictChoices {
USE_EXISTING("Use Existing Data Type"),
RENAME_AND_ADD("Rename New Data Type");
private String optionDisplayString;
private DataTypeConflictChoices(String optionDisplayString) {
this.optionDisplayString = optionDisplayString;
}
@Override
public String toString() {
return optionDisplayString;
}
}
}

View File

@@ -36,6 +36,8 @@ public class VTOptionDefines {
public static boolean DEFAULT_OPTION_FOR_PARAMETER_NAMES_REPLACE_IF_SAME_PRIORITY = false;
public static ReplaceDataChoices DEFAULT_OPTION_FOR_DATA_MATCH_DATA_TYPE =
ReplaceDataChoices.REPLACE_UNDEFINED_DATA_ONLY;
public static DataTypeConflictChoices DEFAULT_OPTION_FOR_DATA_TYPE_CONFLICT_HANDLER =
DataTypeConflictChoices.USE_EXISTING;
public static FunctionNameChoices DEFAULT_OPTION_FOR_FUNCTION_NAME =
FunctionNameChoices.ADD_AS_PRIMARY;
public static FunctionSignatureChoices DEFAULT_OPTION_FOR_FUNCTION_SIGNATURE =
@@ -84,6 +86,8 @@ public class VTOptionDefines {
public static final String POST_COMMENT = APPLY_MARKUP_OPTIONS_NAME + ".Post Comment";
public static final String DATA_MATCH_DATA_TYPE = APPLY_MARKUP_OPTIONS_NAME +
".Data Match Data Type";
public static final String DATA_TYPE_CONFLICT_HANDLER = APPLY_MARKUP_OPTIONS_NAME +
".Data Type Conflict Handler";
public static final String FUNCTION_SIGNATURE = APPLY_MARKUP_OPTIONS_NAME +
".Function Signature";
public static final String CALLING_CONVENTION = APPLY_MARKUP_OPTIONS_NAME +

View File

@@ -25,9 +25,11 @@ import org.junit.Test;
import ghidra.feature.vt.api.main.*;
import ghidra.feature.vt.api.markuptype.DataTypeMarkupType;
import ghidra.feature.vt.gui.util.VTMatchApplyChoices.DataTypeConflictChoices;
import ghidra.feature.vt.gui.util.VTMatchApplyChoices.ReplaceDataChoices;
import ghidra.feature.vt.gui.util.VTOptionDefines;
import ghidra.framework.options.ToolOptions;
import ghidra.program.database.data.ProgramDataTypeManager;
import ghidra.program.model.address.Address;
import ghidra.program.model.data.*;
import ghidra.program.model.lang.*;
@@ -85,6 +87,57 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
doTestFindAndApplyMarkupItem(validator);
}
@Test
public void testReplace_ConflictInDtm_ChooseExistingType() throws Exception {
Address srcAddr = addr("0x010074e6", sourceProgram);
StructureDataType coolStruct1 = createCoolStruct1();
Data sourceData = setDataType(sourceProgram, srcAddr, coolStruct1, coolStruct1.getLength());
// Add a type to the destination that has the same data type path, but is not equivalent
StructureDataType coolStruct2 = createCoolStruct2();
addToDestinationDtm(coolStruct2);
// make room for the type to be applied
Address destAddr = addr("0x010074e6", destinationProgram);
clear(destinationProgram, destAddr, coolStruct2.getLength());
StringDataType destDt = new StringDataType();
Data destData = setDataType(destinationProgram, destAddr, destDt, 4);
DataTypeValidator validator = new DataTypeValidator(sourceData, destData,
ReplaceDataChoices.REPLACE_FIRST_DATA_ONLY);
validator.setConflictChoice(DataTypeConflictChoices.USE_EXISTING);
validator.setKeepExistingType(true);
doTestFindAndApplyMarkupItem(validator);
assertConflictTypeInDestinationDtm(coolStruct1, false);
}
@Test
public void testReplace_ConflictInDtm_ChooseRenameAndAdd() throws Exception {
Address srcAddr = addr("0x010074e6", sourceProgram);
StructureDataType coolStruct1 = createCoolStruct1();
Data sourceData = setDataType(sourceProgram, srcAddr, coolStruct1, coolStruct1.getLength());
// Add a type to the destination that has the same data type path, but is not equivalent
StructureDataType coolStruct2 = createCoolStruct2();
addToDestinationDtm(coolStruct2);
Address destAddr = addr("0x010074e6", destinationProgram);
StringDataType destDt = new StringDataType();
Data destData = setDataType(destinationProgram, destAddr, destDt, 4);
setDataType(destinationProgram, destAddr.add(4), destDt, 6);
DataTypeValidator validator = new DataTypeValidator(sourceData, destData,
ReplaceDataChoices.REPLACE_FIRST_DATA_ONLY);
validator.setConflictChoice(DataTypeConflictChoices.RENAME_AND_ADD);
doTestFindAndApplyMarkupItem(validator);
assertConflictTypeInDestinationDtm(coolStruct1, true);
}
@Test
public void testReplaceLargerWithSmaller() throws Exception {
@@ -305,6 +358,38 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
doTestFindAndApplyMarkupItem_NoEffect(validator);
}
@Test
public void testRejectedApplyDoesNotMutateDestinationDataTypeManager() throws Exception {
Address sourceAddress = addr("0x010074e6", sourceProgram); // LoadCursorW
StructureDataType sourceDataType = new StructureDataType("RejectedApplyStruct", 0);
sourceDataType.add(new DWordDataType());
Data sourceData =
setDataType(sourceProgram, sourceAddress, sourceDataType, sourceDataType.getLength());
Address destinationAddress = addr("0x010074e6", destinationProgram); // LoadCursorW
StringDataType destinationDataType = new StringDataType();
Data destinationData =
setDataType(destinationProgram, destinationAddress, destinationDataType, 4); // Get "Load".
setDataType(destinationProgram, destinationAddress.add(4), destinationDataType, 6); // Get "Cursor".
DataTypeManager destinationDTM = destinationProgram.getDataTypeManager();
assertNull("Test setup invalid - destination should not already have this data type",
destinationDTM.getDataType(sourceDataType.getCategoryPath(),
sourceDataType.getName()));
DataTypeValidator validator = new DataTypeValidator(sourceData, destinationData,
ReplaceDataChoices.REPLACE_UNDEFINED_DATA_ONLY);
validator.setConflictChoice(DataTypeConflictChoices.RENAME_AND_ADD);
doTestFindAndApplyMarkupItem_NoEffect(validator);
assertNull(
"Rejected apply must not add the source data type to the destination data type " +
"manager",
destinationDTM.getDataType(sourceDataType.getCategoryPath(),
sourceDataType.getName()));
}
@Test
public void testReplaceUndefinedOnlyWithLargerWhenBlockedByInstruction() throws Exception {
@@ -413,6 +498,48 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
// Private Methods
//==================================================================================================
private void addToDestinationDtm(StructureDataType struct) {
ProgramDataTypeManager destDtm = destinationProgram.getDataTypeManager();
tx(destDtm, () -> {
destDtm.resolve(struct, null);
});
DataTypePath dtp = struct.getDataTypePath();
DataType resolvedDtm = destDtm.getDataType(dtp);
assertNotNull(resolvedDtm);
}
private StructureDataType createCoolStruct1() {
StructureDataType struct = new StructureDataType("CoolStructure", 0);
struct.add(new DWordDataType());
return struct;
}
// 'CoolStructure' that is slightly different than that made in createCoolStruct1()
private StructureDataType createCoolStruct2() {
StructureDataType struct = new StructureDataType("CoolStructure", 0);
struct.add(new DWordDataType());
struct.add(new DWordDataType());
return struct;
}
private void assertConflictTypeInDestinationDtm(DataType dt, boolean expectConflict) {
DataTypePath dtp = dt.getDataTypePath();
String name = dt.getName() + ".conflict";
CategoryPath cp = dtp.getCategoryPath();
DataTypePath conflictPath = new DataTypePath(cp, name);
ProgramDataTypeManager destDtm = destinationProgram.getDataTypeManager();
DataType conflictType = destDtm.getDataType(conflictPath);
if (expectConflict) {
assertNotNull(conflictType);
}
else {
assertNull(conflictType);
}
}
private Structure createGadgetStruct() {
Structure gadgetStruct = new StructureDataType("Gadget", 0);
@@ -428,14 +555,13 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
private Data setDataType(Program program, Address address, DataType dataType, int length) {
int txID = program.startTransaction("Change Data Type");
boolean commit = false;
try {
return tx(program, () -> {
Listing listing = program.getListing();
Data sourceData = listing.getDataAt(address);
if (sourceData == null) {
return null;
}
listing.clearCodeUnits(address, sourceData.getMaxAddress(), false);
Data data;
if (length > 0) {
@@ -444,23 +570,20 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
else {
data = listing.createData(address, dataType);
}
commit = true;
return data;
}
catch (Exception e) {
// Commit is false by default so nothing else to do.
return null;
}
finally {
program.endTransaction(txID, commit);
}
});
}
private void clear(Program p, Address a, int length) {
tx(p, () -> {
Listing listing = p.getListing();
listing.clearCodeUnits(a, a.add(length), false);
});
}
private Instruction createInstruction(Program program, Address atAddress) {
int txID = program.startTransaction("Create Instruction");
boolean commit = false;
try {
return tx(program, () -> {
Listing listing = program.getListing();
Memory memory = program.getMemory();
MemBuffer buf = new DumbMemBufferImpl(memory, atAddress);
@@ -469,16 +592,8 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
InstructionPrototype proto = program.getLanguage().parse(buf, context, false);
Instruction createdInstruction =
listing.createInstruction(atAddress, proto, buf, context, 0);
commit = true;
return createdInstruction;
}
catch (Exception e) {
// Commit is false by default so nothing else to do.
return null;
}
finally {
program.endTransaction(txID, commit);
}
});
}
//==================================================================================================
@@ -494,6 +609,8 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
private int sourceLength;
private int originalDestinationLength;
private ReplaceDataChoices dataTypeChoice;
private DataTypeConflictChoices conflictChoice;
private boolean keepExistingType;
DataTypeValidator(Data sourceData, Data destinationData,
ReplaceDataChoices dataTypeChoice) {
@@ -510,6 +627,14 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
this.originalDestinationLength = destinationData.getLength();
}
void setConflictChoice(DataTypeConflictChoices conflictChoice) {
this.conflictChoice = conflictChoice;
}
void setKeepExistingType(boolean keep) {
this.keepExistingType = keep;
}
@Override
protected Address getDestinationApplyAddress() {
return getDestinationMatchAddress();
@@ -549,10 +674,19 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
Data currentDestinationData = listing.getDataAt(getDestinationApplyAddress());
DataType currentDestinationDataType = currentDestinationData.getDataType();
int currentDestinationLength = currentDestinationData.getLength();
assertTrue("Data type was not applied",
sourceDataType.isEquivalent(currentDestinationDataType));
assertTrue("Data type was not set to the source data type's size",
sourceLength == currentDestinationLength);
if (keepExistingType) {
// guilty knowledge: keeping the existing type is used when the types have the same
// data type path, but are not equivalent
assertEquals(sourceDataType.getDataTypePath(),
currentDestinationDataType.getDataTypePath());
}
else {
assertTrue("Data type was not applied",
sourceDataType.isEquivalent(currentDestinationDataType));
assertTrue("Data type was not set to the source data type's size",
sourceLength == currentDestinationLength);
}
}
@Override
@@ -571,6 +705,9 @@ public class DataTypeMarkupItemTest extends AbstractVTMarkupItemTest {
public ToolOptions getOptions() {
ToolOptions vtOptions = super.getOptions();
vtOptions.setEnum(VTOptionDefines.DATA_MATCH_DATA_TYPE, dataTypeChoice);
if (conflictChoice != null) {
vtOptions.setEnum(VTOptionDefines.DATA_TYPE_CONFLICT_HANDLER, conflictChoice);
}
return vtOptions;
}