GP-4324 Improved Function Editor for Decompiler use to limit full commit and added checkbox to control full commit

This commit is contained in:
ghidra1 2024-07-24 10:43:03 -04:00
parent 9b2fb083ba
commit 9ddc65d7e5
14 changed files with 1287 additions and 718 deletions

View File

@ -29,10 +29,16 @@
<LI>Ordered list of named parameters and associated data types.</LI>
</OL>
<BLOCKQUOTE>
<P><IMG src="help/shared/note.png" alt="Note:">
While certain changes may be made without commiting all return and parameter details, those
changes which are made that require a full commit will cause the
<A href="#CommitDetails">Commit all return/parameter details</A> checkbox to be enabled.
The user may choose to alter this full commit by checking or unchecking the corresponding
checkbox.
</P>
</BLOCKQUOTE>
<H2><A name="Function_Attributes"></A>Function Attributes</H2>
<BLOCKQUOTE>
@ -289,11 +295,9 @@
<OL>
<LI>Place the cursor on a function signature</LI>
<LI>Right-mouse-click, and select <I>Edit Function</I>.</LI>
<LI>Edit any attributes of the function using the dialog.</LI>
<LI>Consider <A href="#CommitDetails">committing all return/parameter details</A></LI>
<LI>Press <I>OK</I> to save your changes.</LI>
</OL>
@ -396,6 +400,35 @@
the compiler specification (*.cspec file) associated with a program. This feature is
typically used when the effects of calling a well-known function need to be simplified so
that the caller can be more easily analyzed and/or understood.</P>
<H3><A name="CommitDetails">Commit all return/parameter details</A></H3>
<P>This checkbox controls whether a complete function update will be performed, including
datatype and optional custom-storage details, for the return and all parameters.
This checkbox will enable itself when
specific changes are made which require a complete commit in order to preserve what is seen
in the editor. Use of custom storage or altering return or parameter datatypes will require
a full commit in order to retain such changes. It is important to note that a full Commit
with storage and/or datatype changes will impose a <I>USER_DEFINED</I> <B>Signature Source</B>
which will lock-in parameter details within the Decompiler. The <B>Signature Source</B>
Function Listing Field can be useful for monitoring this state.</P>
<BLOCKQUOTE>
<P><IMG alt="" src="images/warning.help.png">
If the signature has been autogenerated by the decompiler and changes have been made to
things other than the signature, like calling convention or callfixup, that may affect the
generated signature. Consider changing the calling convention only to check if the
decompiler would generate the same signature unless you are sure the generated signature
is correct.</P>
</BLOCKQUOTE>
<P>Prior to clicking the dialog's <B>OK</B> button the user may uncheck this control to
prevent return/parameter changes from being committed. Once datatype or custom-storage
choices have been changed, any parameter name changes will also require a full commit to
preserve such changes. A full commit is not required if only parameter names have been
changed. The exception to this is when the Function Editor is used within the Decompiler
and parameters have not previously be committed.</P>
</BLOCKQUOTE>
<H2><A name="Edit_Parameter_Storage"></A>Edit Parameter Storage Dialog</H2>

Binary file not shown.

Before

Width:  |  Height:  |  Size: 33 KiB

After

Width:  |  Height:  |  Size: 37 KiB

View File

@ -0,0 +1,354 @@
/* ###
* 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.function.editor;
import java.util.*;
import ghidra.program.model.data.*;
import ghidra.program.model.lang.CompilerSpec;
import ghidra.program.model.lang.PrototypeModel;
import ghidra.program.model.listing.*;
import ghidra.util.exception.AssertException;
import ghidra.util.exception.InvalidInputException;
class FunctionData extends FunctionDataView {
public FunctionData(Function function) {
super(function);
}
/**
* Determine if return/parameter datatype and/or storage has changed and should be comitted.
* Auto params and calling-convention imposed changes are not considered.
* @param originalFunctionData original function data prior to edits
* @return true if non-auto parameters have been modified, else false
*/
boolean hasParameterChanges(FunctionDataView originalFunctionData) {
boolean checkStorage = false;
if (canCustomizeStorage()) {
// if (!originalFunctionData.canCustomizeStorage()) {
// // switched to using custom storage
// return true;
// }
checkStorage = true;
}
if (!returnInfo.getFormalDataType()
.equals(originalFunctionData.returnInfo.getFormalDataType())) {
return true;
}
if (checkStorage &&
!returnInfo.getStorage().equals(originalFunctionData.returnInfo.getStorage())) {
return true;
}
int paramStartIndex = autoParamCount;
int nonAutoParamCount = parameters.size() - paramStartIndex;
int originalParamStartIndex = originalFunctionData.autoParamCount;
int originalNonAutoParamCount =
originalFunctionData.parameters.size() - originalParamStartIndex;
if (nonAutoParamCount != originalNonAutoParamCount) {
return true;
}
for (int i = 0; i < nonAutoParamCount; i++) {
ParamInfo param = parameters.get(paramStartIndex + i);
ParamInfo originalParam =
originalFunctionData.parameters.get(originalParamStartIndex + i);
if (!param.getFormalDataType().equals(originalParam.getFormalDataType())) {
return true;
}
if (checkStorage && !param.getStorage().equals(originalParam.getStorage())) {
return true;
}
// Check for name change without stored param
if (!Objects.equals(param.getName(true), originalParam.getName(true))) {
if (function.getParameter(param.getOrdinal()) == null) {
return true;
}
}
}
return false;
}
/**
* Determine if parameter name(s) have been modified. Note that this method will return false
* if the number of non-auto parameters has changed.
* @param originalFunctionData original function data prior to edits
* @return true if one or more parameter names has changed, else false
*/
boolean hasParameterNamesChanged(FunctionDataView originalFunctionData) {
int paramStartIndex = autoParamCount;
int nonAutoParamCount = parameters.size() - paramStartIndex;
int originalParamStartIndex = originalFunctionData.autoParamCount;
int originalNonAutoParamCount =
originalFunctionData.parameters.size() - originalParamStartIndex;
if (nonAutoParamCount != originalNonAutoParamCount) {
return false;
}
for (int i = 0; i < nonAutoParamCount; i++) {
ParamInfo param = parameters.get(paramStartIndex + i);
ParamInfo originalParam =
originalFunctionData.parameters.get(originalParamStartIndex + i);
if (!Objects.equals(param.getName(true), originalParam.getName(true))) {
return true;
}
}
return false;
}
ParamInfo addNewParameter() {
ParamInfo param = new ParamInfo(this, null, DataType.DEFAULT,
VariableStorage.UNASSIGNED_STORAGE, canCustomizeStorage(), parameters.size());
parameters.add(param);
fixupOrdinals();
updateParameterAndReturnStorage();
return param;
}
void removeAllParameters() {
parameters.clear();
autoParamCount = 0;
}
void removeParameters(List<ParamInfo> paramsToRemove) {
Iterator<ParamInfo> it = parameters.iterator();
while (it.hasNext()) {
ParamInfo p = it.next();
if (paramsToRemove.contains(p)) {
it.remove();
}
}
fixupOrdinals();
updateParameterAndReturnStorage();
}
void moveParameterUp(int paramIndex) {
ParamInfo param = parameters.remove(paramIndex);
parameters.add(paramIndex - 1, param);
fixupOrdinals();
updateParameterAndReturnStorage();
}
void moveParameterDown(int paramIndex) {
ParamInfo param = parameters.remove(paramIndex);
parameters.add(paramIndex + 1, param);
fixupOrdinals();
updateParameterAndReturnStorage();
}
void setName(String n) {
this.name = n;
}
void setInline(boolean enable) {
this.isInLine = enable;
}
void setHasNoReturn(boolean enable) {
this.hasNoReturn = enable;
}
void clearCallFixup() {
callFixupName = null;
}
void setCallFixupName(String cfuName) {
this.callFixupName = cfuName;
}
void setCallingConventionName(String ccName) {
if (Objects.equals(ccName, callingConventionName)) {
return;
}
this.callingConventionName = ccName;
removeExplicitThisParameter();
updateParameterAndReturnStorage();
}
void setVarArgs(boolean enable) {
this.hasVarArgs = enable;
}
/**
* Update dynamic storage and auto-params when custom storage is disasbled.
* Returns immediately if custom storage is enabled.
*/
void updateParameterAndReturnStorage() {
if (allowCustomStorage) {
autoParamCount = 0;
return;
}
PrototypeModel effectiveCallingConvention = getEffectiveCallingConvention();
if (effectiveCallingConvention == null) {
for (ParamInfo info : parameters) {
info.setStorage(VariableStorage.UNASSIGNED_STORAGE);
}
return;
}
DataType[] dataTypes = new DataType[parameters.size() - autoParamCount + 1];
dataTypes[0] = returnInfo.getFormalDataType();
int index = 1;
for (int i = autoParamCount; i < parameters.size(); i++) {
dataTypes[index++] = parameters.get(i).getFormalDataType();
}
VariableStorage[] paramStorage =
effectiveCallingConvention.getStorageLocations(getProgram(), dataTypes, true);
returnInfo.setStorage(paramStorage[0]);
List<ParamInfo> oldParams = parameters;
int oldAutoCount = autoParamCount;
parameters = new ArrayList<>();
autoParamCount = 0;
int ordinal = 0;
for (int i = 1; i < paramStorage.length; i++) {
VariableStorage storage = paramStorage[i];
ParamInfo info;
if (storage.isAutoStorage()) {
DataType dt = VariableUtilities.getAutoDataType(function,
returnInfo.getFormalDataType(), storage);
try {
info = new ParamInfo(this,
new AutoParameterImpl(dt, ++autoParamCount, storage, function));
}
catch (InvalidInputException e) {
throw new AssertException(e); // unexpected
}
}
else {
info = oldParams.get(oldAutoCount + ordinal);
info.setStorage(storage);
++ordinal;
}
parameters.add(info);
}
fixupOrdinals();
}
void clearAutoParams() {
autoParamCount = 0;
}
/**
* Change the enablement of custom storage
* @param enable true if custom storage should be enable, else false to
*/
void setUseCustomStorage(boolean enable) {
if (enable == allowCustomStorage) {
return;
}
allowCustomStorage = enable;
if (!enable) {
removeExplicitThisParameter();
DataType returnDt = removeExplicitReturnStoragePtrParameter();
if (returnDt != null) {
returnInfo.setFormalDataType(returnDt);
returnInfo.setStorage(VariableStorage.UNASSIGNED_STORAGE);
}
updateParameterAndReturnStorage();
}
else {
switchToCustomStorage();
}
}
/**
* Switch to custom storage and perform required transformations
*/
private void switchToCustomStorage() {
try {
VariableStorage returnStorage = returnInfo.getStorage();
DataType returnType = returnInfo.getDataType();
if (returnStorage.isForcedIndirect() && returnStorage.isVoidStorage()) {
returnType = VoidDataType.dataType;
}
returnInfo.setFormalDataType(returnType);
returnInfo.setStorage(returnStorage.clone(getProgram()));
autoParamCount = 0;
for (ParamInfo paramInfo : parameters) {
DataType dt = paramInfo.getDataType();
VariableStorage storage = paramInfo.getStorage();
paramInfo.setFormalDataType(dt);
paramInfo.setStorage(storage.clone(getProgram()));
}
}
catch (InvalidInputException e) {
throw new AssertException(e); // unexpected
}
}
void removeExplicitThisParameter() {
if (!allowCustomStorage &&
CompilerSpec.CALLING_CONVENTION_thiscall.equals(callingConventionName)) {
int thisIndex = findExplicitThisParameter();
if (thisIndex >= 0) {
parameters.remove(thisIndex); // remove explicit 'this' parameter
}
}
}
private DataType removeExplicitReturnStoragePtrParameter() {
int index = findExplicitReturnStoragePtrParameter();
if (index >= 0) {
// remove explicit '__return_storage_ptr__' parameter - should always be a pointer
ParamInfo returnStoragePtrParameter = parameters.remove(index);
DataType dt = returnStoragePtrParameter.getDataType();
if (dt instanceof Pointer ptr) {
return ptr.getDataType();
}
}
return null;
}
private int findExplicitThisParameter() {
for (int i = 0; i < parameters.size(); i++) {
ParamInfo p = parameters.get(i);
if (!p.isAutoParameter() && Function.THIS_PARAM_NAME.equals(p.getName()) &&
(p.getDataType() instanceof Pointer)) {
return i;
}
}
return -1;
}
private int findExplicitReturnStoragePtrParameter() {
for (int i = 0; i < parameters.size(); i++) {
ParamInfo p = parameters.get(i);
if (!p.isAutoParameter() && Function.RETURN_PTR_PARAM_NAME.equals(p.getName()) &&
(p.getDataType() instanceof Pointer)) {
return i;
}
}
return -1;
}
}

View File

@ -0,0 +1,248 @@
/* ###
* 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.function.editor;
import java.util.*;
import ghidra.program.model.data.VoidDataType;
import ghidra.program.model.lang.PrototypeModel;
import ghidra.program.model.listing.*;
import ghidra.program.model.symbol.SymbolUtilities;
/**
* {@link FunctionDataView} provides an immutable view of function data used by the
* Function Editor model.
*/
class FunctionDataView {
Function function;
String name;
boolean hasVarArgs;
ParamInfo returnInfo;
List<ParamInfo> parameters = new ArrayList<>();
int autoParamCount = 0;
boolean isInLine;
boolean hasNoReturn;
String callingConventionName;
boolean allowCustomStorage;
String callFixupName;
/**
* Construct instance from {@link Function} details.
* @param function program function
*/
FunctionDataView(Function function) {
this.function = function;
this.name = function.getName();
allowCustomStorage = function.hasCustomVariableStorage();
hasVarArgs = function.hasVarArgs();
isInLine = function.isInline();
hasNoReturn = function.hasNoReturn();
callingConventionName = function.getCallingConventionName();
callFixupName = function.getCallFixup();
initializeParametersAndReturn();
}
/**
* Construct a duplicate instance from another {@link FunctionDataView} instance.
* @param otherFunctionData function data
*/
FunctionDataView(FunctionDataView otherFunctionData) {
name = otherFunctionData.name;
hasVarArgs = otherFunctionData.hasVarArgs;
returnInfo = otherFunctionData.returnInfo.copy();
for (ParamInfo p : otherFunctionData.parameters) {
parameters.add(p.copy());
}
autoParamCount = otherFunctionData.autoParamCount;
isInLine = otherFunctionData.isInLine;
hasNoReturn = otherFunctionData.hasNoReturn;
callingConventionName = otherFunctionData.callingConventionName;
allowCustomStorage = otherFunctionData.allowCustomStorage;
callFixupName = otherFunctionData.callFixupName;
}
@Override
public boolean equals(Object obj) {
if (!(obj instanceof FunctionDataView otherFunctionData)) {
return false;
}
if (!Objects.equals(name, otherFunctionData.name) ||
!Objects.equals(callingConventionName, otherFunctionData.callingConventionName) ||
hasVarArgs != otherFunctionData.hasVarArgs ||
parameters.size() != otherFunctionData.parameters.size() ||
autoParamCount != otherFunctionData.autoParamCount ||
isInLine != otherFunctionData.isInLine ||
hasNoReturn != otherFunctionData.hasNoReturn ||
allowCustomStorage != otherFunctionData.allowCustomStorage ||
!Objects.equals(callFixupName, otherFunctionData.callFixupName) ||
!returnInfo.isSame(otherFunctionData.returnInfo)) {
return false;
}
int paramCount = parameters.size();
for (int i = 0; i < paramCount; i++) {
ParamInfo param = parameters.get(i);
ParamInfo otherParam = otherFunctionData.parameters.get(i);
if (!param.isSame(otherParam)) {
return false;
}
}
return true;
}
@Override
public int hashCode() {
return getNameString().hashCode();
}
private void initializeParametersAndReturn() {
returnInfo = new ParamInfo(this, function.getReturn());
// check for void storage correction
if (VoidDataType.isVoidDataType(returnInfo.getDataType()) &&
returnInfo.getStorage() != VariableStorage.VOID_STORAGE) {
returnInfo.setStorage(VariableStorage.VOID_STORAGE);
}
autoParamCount = 0;
Parameter[] params = function.getParameters();
for (Parameter parameter : params) {
if (parameter.isAutoParameter()) {
++autoParamCount;
}
parameters.add(new ParamInfo(this, parameter));
}
fixupOrdinals();
}
void fixupOrdinals() {
for (int i = 0; i < parameters.size(); i++) {
parameters.get(i).setOrdinal(i);
}
}
String getFunctionSignatureText() {
StringBuilder buf = new StringBuilder();
buf.append(returnInfo.getFormalDataType().getName()).append(" ");
buf.append(getNameString());
buf.append(" (");
int skipCount = autoParamCount;
int ordinal = 0;
for (ParamInfo param : parameters) {
if (skipCount > 0) {
--skipCount;
continue;
}
if (ordinal++ != 0) {
buf.append(", ");
}
buf.append(param.getFormalDataType().getName());
buf.append(" ");
buf.append(param.getName());
}
if (hasVarArgs()) {
if (!parameters.isEmpty()) {
buf.append(", ");
}
buf.append(FunctionSignature.VAR_ARGS_DISPLAY_STRING);
}
else if (parameters.size() == 0) {
buf.append(FunctionSignature.VOID_PARAM_DISPLAY_STRING);
}
buf.append(')');
return buf.toString();
}
public Program getProgram() {
return function.getProgram();
}
boolean canCustomizeStorage() {
return allowCustomStorage;
}
int getAutoParamCount() {
return autoParamCount;
}
int getParamCount() {
return parameters.size();
}
public String getName() {
return name;
}
String getNameString() {
return name.length() == 0 ? SymbolUtilities.getDefaultFunctionName(function.getEntryPoint())
: name;
}
boolean isInline() {
return isInLine;
}
boolean hasNoReturn() {
return hasNoReturn;
}
String getCallFixupName() {
return callFixupName;
}
boolean hasCallFixup() {
return callFixupName != null;
}
List<ParamInfo> getParameters() {
return parameters;
}
ParamInfo getReturnInfo() {
return returnInfo;
}
PrototypeModel getEffectiveCallingConvention() {
FunctionManager functionManager = getProgram().getFunctionManager();
PrototypeModel effectiveCallingConvention =
functionManager.getCallingConvention(getCallingConventionName());
if (effectiveCallingConvention == null) {
effectiveCallingConvention = functionManager.getDefaultCallingConvention();
}
return effectiveCallingConvention;
}
String getCallingConventionName() {
return callingConventionName;
}
boolean hasVarArgs() {
return hasVarArgs;
}
boolean hasParameters() {
return !parameters.isEmpty();
}
}

View File

@ -57,6 +57,11 @@ import resources.Icons;
public class FunctionEditorDialog extends DialogComponentProvider implements ModelChangeListener {
private static final String COMMIT_FULL_SIGNATURE_WARNING =
"All signature details will be commited (see Commit checkbox above)";
private static final String SIGNATURE_LOSS_WARNING =
"Return/Parameter changes will not be applied (see Commit checkbox above)";
private FunctionEditorModel model;
private DocumentListener nameFieldDocumentListener;
private GTable parameterTable;
@ -77,6 +82,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
private JCheckBox storageCheckBox;
private JScrollPane scroll;
private JPanel previewPanel;
private JCheckBox commitFullParamDetailsCheckBox; // optional: may be null
private FunctionSignatureTextField signatureTextField;
private UndoRedoKeeper signatureFieldUndoRedoKeeper;
@ -84,11 +90,22 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
private MyGlassPane glassPane;
private JPanel centerPanel;
/**
* Construct Function Editor dialog for a specified Function and associated DataType service.
* @param service DataType service
* @param function Function to be modified
*/
public FunctionEditorDialog(DataTypeManagerService service, Function function) {
this(new FunctionEditorModel(service, function));
this(new FunctionEditorModel(service, function), true);
}
public FunctionEditorDialog(FunctionEditorModel model) {
/**
* Construct Function Editor dialog using a specified model
* @param model function detail model
* @param hasOptionalSignatureCommit if true an optional control will be included which
* controls commit of parameter/return details, including Varargs and use of custom storage.
*/
public FunctionEditorDialog(FunctionEditorModel model, boolean hasOptionalSignatureCommit) {
super(createTitle(model.getFunction()));
this.service = model.getDataTypeManagerService();
setRememberLocation(true);
@ -96,7 +113,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
setHelpLocation(new HelpLocation("FunctionPlugin", "Edit_Function"));
this.model = model;
model.setModelChangeListener(this);
addWorkPanel(buildMainPanel());
addWorkPanel(buildMainPanel(hasOptionalSignatureCommit));
addOKButton();
addCancelButton();
glassPane = new MyGlassPane();
@ -157,7 +174,13 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
return;
}
}
if (model.apply()) {
boolean fullCommit = true;
if (commitFullParamDetailsCheckBox != null &&
!commitFullParamDetailsCheckBox.isSelected()) {
fullCommit = false;
}
if (model.apply(fullCommit)) {
close();
}
}
@ -168,24 +191,24 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
super.close();
}
private JComponent buildMainPanel() {
private JComponent buildMainPanel(boolean hasOptionalSignatureCommit) {
JPanel panel = new JPanel(new BorderLayout());
panel.setBorder(BorderFactory.createEmptyBorder(10, 10, 10, 10));
panel.add(buildPreview(), BorderLayout.NORTH);
panel.add(buildCenterPanel(), BorderLayout.CENTER);
panel.add(buildCenterPanel(hasOptionalSignatureCommit), BorderLayout.CENTER);
return panel;
}
private JComponent buildCenterPanel() {
private JComponent buildCenterPanel(boolean hasOptionalSignatureCommit) {
centerPanel = new JPanel(new BorderLayout());
centerPanel.setBorder(BorderFactory.createEmptyBorder(10, 0, 10, 0));
centerPanel.add(buildAttributePanel(), BorderLayout.NORTH);
centerPanel.add(buildTable(), BorderLayout.CENTER);
centerPanel.add(buildBottomPanel(), BorderLayout.SOUTH);
centerPanel.add(buildBottomPanel(hasOptionalSignatureCommit), BorderLayout.SOUTH);
return centerPanel;
}
private Component buildBottomPanel() {
private Component buildBottomPanel(boolean hasOptionalSignatureCommit) {
JPanel panel = new JPanel(new BorderLayout());
Border b = BorderFactory.createEmptyBorder(0, 0, 0, 0);
@ -203,6 +226,29 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
else {
panel.add(new JPanel(), BorderLayout.CENTER);
}
if (hasOptionalSignatureCommit) {
commitFullParamDetailsCheckBox = new JCheckBox("Commit all return/parameter details");
commitFullParamDetailsCheckBox.addActionListener(e -> {
if (!model.isValid()) {
return;
}
if (!commitFullParamDetailsCheckBox.isSelected()) {
if (model.hasSignificantParameterChanges()) {
setStatusText(SIGNATURE_LOSS_WARNING, MessageType.WARNING);
}
else {
clearStatusText();
}
}
else {
setStatusText(COMMIT_FULL_SIGNATURE_WARNING, MessageType.WARNING);
setOkEnabled(true);
}
});
panel.add(commitFullParamDetailsCheckBox, BorderLayout.SOUTH);
}
return panel;
}
@ -250,7 +296,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
signatureTextField.setEscapeListener(e -> {
if (!model.hasChanges()) {
if (!model.hasSignatureTextChanges()) {
// no changes; user wish to close the dialog
cancelCallback();
return;
@ -400,7 +446,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
callFixupComboBox.addItem(element);
}
callFixupComboBox.addItemListener(
e -> model.setCallFixupName((String) callFixupComboBox.getSelectedItem()));
e -> model.setCallFixupChoice((String) callFixupComboBox.getSelectedItem()));
}
else {
callFixupComboBox.setToolTipText("No call-fixups defined by compiler specification");
@ -414,11 +460,11 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
private Component buildTable() {
JPanel panel = new JPanel(new BorderLayout());
panel.setBorder(BorderFactory.createTitledBorder(BorderFactory.createEmptyBorder(),
"Function Variables"));
"Function Return/Parameters"));
paramTableModel = new ParameterTableModel(model);
parameterTable = new ParameterTable(paramTableModel);
selectionListener = e -> model.setSelectedParameterRow(parameterTable.getSelectedRows());
selectionListener = e -> model.setSelectedParameterRows(parameterTable.getSelectedRows());
JScrollPane tableScroll = new JScrollPane(parameterTable);
panel.add(tableScroll, BorderLayout.CENTER);
@ -497,11 +543,18 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
updateTableSelection();
updateTableButtonEnablement();
updateStorageEditingEnabled();
updateOptionalParamCommit();
}
}
private void updateOptionalParamCommit() {
if (commitFullParamDetailsCheckBox != null) {
commitFullParamDetailsCheckBox.setSelected(model.hasSignificantParameterChanges());
}
}
private void updateStorageEditingEnabled() {
boolean canCustomizeStorage = model.canCustomizeStorage();
boolean canCustomizeStorage = model.canUseCustomStorage();
if (storageCheckBox.isSelected() != canCustomizeStorage) {
storageCheckBox.setSelected(canCustomizeStorage);
}
@ -522,7 +575,9 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
selectionModel.removeListSelectionListener(selectionListener);
parameterTable.clearSelection();
for (int i : selectedRows) {
parameterTable.addRowSelectionInterval(i, i);
if (i < parameterTable.getRowCount()) {
parameterTable.addRowSelectionInterval(i, i);
}
}
parameterTable.scrollToSelectedRow();
selectionModel.addListSelectionListener(selectionListener);
@ -539,7 +594,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
}
private void updateCallFixupCombo() {
String callFixupName = model.getCallFixupName();
String callFixupName = model.getCallFixupChoice();
if (!callFixupComboBox.getSelectedItem().equals(callFixupName)) {
callFixupComboBox.setSelectedItem(callFixupName);
if (!callFixupComboBox.getSelectedItem().equals(callFixupName)) {
@ -574,7 +629,7 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
}
private void updateOkButton() {
setOkEnabled(model.isValid());
setOkEnabled(model.isValid() && model.hasChanges());
}
private void updateStatusText() {
@ -778,6 +833,10 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod
setForeground(getErrorForegroundColor(isSelected));
setToolTipText("Invalid Parameter Storage");
}
else if (rowData.hasStorageConflict()) {
setForeground(getErrorForegroundColor(isSelected));
setToolTipText("Conflicting Parameter Storage");
}
else {
setForeground(isSelected ? table.getSelectionForeground() : Colors.FOREGROUND);
setToolTipText("");

View File

@ -36,4 +36,6 @@ interface FunctionVariableData {
public String getName();
public DataType getFormalDataType();
public boolean hasStorageConflict();
}

View File

@ -15,63 +15,98 @@
*/
package ghidra.app.plugin.core.function.editor;
import java.util.Objects;
import ghidra.program.model.data.*;
import ghidra.program.model.listing.*;
import ghidra.program.model.symbol.SourceType;
import ghidra.program.model.symbol.SymbolUtilities;
import ghidra.util.SystemUtilities;
import ghidra.util.exception.AssertException;
import ghidra.util.exception.InvalidInputException;
public class ParamInfo {
public class ParamInfo implements Comparable<ParamInfo> {
private Parameter original;
private String name;
private DataType formalDataType;
private VariableStorage storage;
private boolean isCustomStorage;
private int ordinal;
private FunctionEditorModel model;
ParamInfo(FunctionEditorModel model, Parameter parameter) {
this(model, parameter.getName(), parameter.getFormalDataType(),
parameter.getVariableStorage(), parameter.getOrdinal());
original = parameter;
private boolean hasStorageConflict = false;
private FunctionDataView functionData;
private Program program;
ParamInfo(FunctionDataView functionData, Parameter parameter) {
this(functionData, parameter.getName(), parameter.getFormalDataType(),
parameter.getVariableStorage(), functionData.canCustomizeStorage(),
parameter.getOrdinal());
}
ParamInfo(FunctionEditorModel model, ParameterDefinition paramDefinition) {
this(model, paramDefinition.getName(), paramDefinition.getDataType(),
VariableStorage.UNASSIGNED_STORAGE, paramDefinition.getOrdinal());
ParamInfo(FunctionDataView functionData, ParameterDefinition paramDefinition) {
this(functionData, paramDefinition.getName(), paramDefinition.getDataType(),
VariableStorage.UNASSIGNED_STORAGE, false, paramDefinition.getOrdinal());
this.functionData = functionData;
}
ParamInfo(FunctionEditorModel model, String name, DataType formalDataType,
VariableStorage storage,
int ordinal) {
this.model = model;
ParamInfo(FunctionDataView functionData, String name, DataType formalDataType,
VariableStorage storage, boolean isCustomStorage, int ordinal) {
this.functionData = functionData;
this.program = functionData.getProgram();
this.name = SymbolUtilities.isDefaultParameterName(name) ? null : name;
this.formalDataType = formalDataType;
this.storage = storage;
this.isCustomStorage = isCustomStorage;
this.ordinal = ordinal;
}
ParamInfo copy() {
return new ParamInfo(functionData, name, formalDataType, storage, isCustomStorage, ordinal);
}
public boolean isSame(ParamInfo otherParam) {
if (!Objects.equals(name, otherParam.name) ||
isAutoParameter() != otherParam.isAutoParameter() ||
!formalDataType.equals(otherParam.getFormalDataType())) {
return false;
}
return !isCustomStorage || storage.equals(otherParam.storage);
}
@Override
public boolean equals(Object obj) {
public int compareTo(ParamInfo o) {
int c = ordinal - o.ordinal;
if (c != 0) {
return c;
}
return getName().compareTo(o.getName());
}
@Override
public final boolean equals(Object obj) {
return this == obj;
}
@Override
public int hashCode() {
return getName().hashCode();
public final int hashCode() {
return super.hashCode();
}
public String getName(boolean returnNullForDefault) {
if (returnNullForDefault) {
return name;
}
return getName();
}
public String getName() {
return name != null ? name : SymbolUtilities.getDefaultParamName(ordinal -
model.getAutoParamCount());
return name != null ? name
: SymbolUtilities.getDefaultParamName(ordinal - functionData.getAutoParamCount());
}
DataType getDataType() {
DataType dt = formalDataType;
if (storage.isForcedIndirect()) {
Program program = model.getProgram();
DataTypeManager dtm = program.getDataTypeManager();
int ptrSize = storage.size();
if (ptrSize != dtm.getDataOrganization().getPointerSize()) {
@ -96,6 +131,10 @@ public class ParamInfo {
return storage.isAutoStorage();
}
boolean isReturnParameter() {
return ordinal == Parameter.RETURN_ORIDINAL;
}
boolean isForcedIndirect() {
return storage.isForcedIndirect();
}
@ -110,14 +149,11 @@ public class ParamInfo {
}
void setOrdinal(int i) {
if (original != null && original.getOrdinal() != i) {
original = null;
}
this.ordinal = i;
}
void setName(String name) {
if (name != null && name.length() == 0) {
if (name != null && (name.length() == 0 || SymbolUtilities.isDefaultParameterName(name))) {
name = null;
}
this.name = name;
@ -125,48 +161,27 @@ public class ParamInfo {
void setFormalDataType(DataType formalDataType) {
this.formalDataType = formalDataType;
original = null;
}
void setStorage(VariableStorage storage) {
this.isCustomStorage = functionData.canCustomizeStorage();
this.storage = storage;
if (model.canCustomizeStorage()) {
original = null;
}
}
boolean isModified() {
return original == null;
}
Parameter getParameter(SourceType source) {
boolean isNameModified() {
return original != null && !SystemUtilities.isEqual(original.getName(), getName());
}
/**
* @return unchanged original parameter or null if new or datatype was changed
*/
Parameter getOriginalParameter() {
return original;
}
Parameter getParameter(boolean isCustom) {
if (original != null) {
return original;
}
VariableStorage variableStorage = isCustom ? storage : VariableStorage.UNASSIGNED_STORAGE;
VariableStorage variableStorage =
isCustomStorage ? storage : VariableStorage.UNASSIGNED_STORAGE;
try {
if (ordinal == Parameter.RETURN_ORIDINAL) {
return new ReturnParameterImpl(formalDataType, variableStorage, true,
model.getProgram());
return new ReturnParameterImpl(formalDataType, variableStorage, true, program);
}
// preserve original source type if name unchanged
SourceType source = SourceType.USER_DEFINED;
if (original != null && original.getName().equals(name)) {
source = original.getSource();
String n = name;
if (n == null) {
source = SourceType.DEFAULT;
n = SymbolUtilities.getDefaultParamName(ordinal);
}
return new MyParameter(name, formalDataType, variableStorage, model.getProgram(),
return new ParameterImpl(n, ordinal, formalDataType, variableStorage, true, program,
source);
}
catch (InvalidInputException e) {
@ -174,13 +189,11 @@ public class ParamInfo {
}
}
private static class MyParameter extends ParameterImpl {
MyParameter(String name, DataType dataType, VariableStorage storage, Program program,
SourceType source) throws InvalidInputException {
super(name, UNASSIGNED_ORDINAL, dataType, storage, true, program,
SourceType.USER_DEFINED);
}
boolean hasStorageConflict() {
return hasStorageConflict;
}
void setHasStorageConflict(boolean state) {
hasStorageConflict = state;
}
}

View File

@ -291,6 +291,11 @@ class ParameterTableModel extends AbstractGTableModel<FunctionVariableData> {
public void setStorage(VariableStorage storage) {
functionModel.setParameterStorage(param, storage);
}
@Override
public boolean hasStorageConflict() {
return param.hasStorageConflict();
}
}
private class ReturnRowData implements FunctionVariableData {
@ -336,5 +341,10 @@ class ParameterTableModel extends AbstractGTableModel<FunctionVariableData> {
public void setName(String name) {
// no name for return type
}
@Override
public boolean hasStorageConflict() {
return false;
}
}
}

View File

@ -388,6 +388,11 @@ public class StorageAddressEditorDialog extends DialogComponentProvider
public DataType getFormalDataType() {
return variable.getDataType();
}
@Override
public boolean hasStorageConflict() {
return false;
}
}
}

View File

@ -104,7 +104,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
model.setName("");
assertDataChangedCallback(true);
assertEquals("", model.getName());
assertEquals("void ? (void)", getSignatureText());
assertEquals("void FUN_00001000 (void)", getSignatureText());
assertTrue(!model.isValid());
assertEquals("Missing function name", model.getStatusText());
}
@ -142,11 +142,11 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
@Test
public void testCustomStorage() {
assertFalse(model.canCustomizeStorage());
assertFalse(model.canUseCustomStorage());
model.setUseCustomizeStorage(true);
assertTrue(model.canCustomizeStorage());
assertTrue(model.canUseCustomStorage());
model.setUseCustomizeStorage(false);
assertFalse(model.canCustomizeStorage());
assertFalse(model.canUseCustomStorage());
}
@Test
@ -155,17 +155,17 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
model.setIsInLine(true);
assertTrue(model.isInLine());
assertTrue(model.isValid());
assertEquals("-NONE-", model.getCallFixupName());
assertEquals("-NONE-", model.getCallFixupChoice());
String callFixupName = model.getCallFixupNames()[0];
model.setCallFixupName(callFixupName);
assertEquals(callFixupName, model.getCallFixupName());
model.setCallFixupChoice(callFixupName);
assertEquals(callFixupName, model.getCallFixupChoice());
assertTrue(!model.isInLine());
model.setIsInLine(true);
assertTrue(model.isInLine());
assertTrue(model.isValid());
assertEquals("-NONE-", model.getCallFixupName());
assertEquals("-NONE-", model.getCallFixupChoice());
}
@ -204,14 +204,14 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
model.addParameter();
assertEquals(3, model.getParameters().size());
model.setSelectedParameterRow(new int[0]);
model.setSelectedParameterRows(new int[0]);
// none selected, so can't remove
assertEquals(0, model.getSelectedParameterRows().length);
assertTrue(!model.canRemoveParameters());
// select the first entry
model.setSelectedParameterRow(new int[] { 1 });
model.setSelectedParameterRows(new int[] { 1 });
assertEquals(1, model.getSelectedParameterRows().length);
assertTrue(model.canRemoveParameters());
@ -234,7 +234,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
assertEquals(3, model.getParameters().size());
// select the last entry
model.setSelectedParameterRow(new int[] { 3 });
model.setSelectedParameterRows(new int[] { 3 });
assertEquals(1, model.getSelectedParameterRows().length);
assertTrue(model.canRemoveParameters());
@ -257,7 +257,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
assertEquals(3, model.getParameters().size());
// select the first and last entry
model.setSelectedParameterRow(new int[] { 1, 3 });
model.setSelectedParameterRows(new int[] { 1, 3 });
assertEquals(2, model.getSelectedParameterRows().length);
assertTrue(model.canRemoveParameters());
@ -279,7 +279,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
assertEquals(3, model.getParameters().size());
// select all params
model.setSelectedParameterRow(new int[] { 1, 2, 3 });
model.setSelectedParameterRows(new int[] { 1, 2, 3 });
assertEquals(3, model.getSelectedParameterRows().length);
assertTrue(model.canRemoveParameters());
@ -299,22 +299,22 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
model.addParameter();
// no selection, both buttons disabled
model.setSelectedParameterRow(new int[0]);
model.setSelectedParameterRows(new int[0]);
assertTrue(!model.canMoveParameterUp());
assertTrue(!model.canMoveParameterDown());
// multiple selection, both buttons disabled
model.setSelectedParameterRow(new int[] { 1, 2 });
model.setSelectedParameterRows(new int[] { 1, 2 });
assertTrue(!model.canMoveParameterUp());
assertTrue(!model.canMoveParameterDown());
// select the first param, up button disabled, down button enabled
model.setSelectedParameterRow(new int[] { 1 });
model.setSelectedParameterRows(new int[] { 1 });
assertTrue(!model.canMoveParameterUp());
assertTrue(model.canMoveParameterDown());
// select the middle row, both buttons enabled
model.setSelectedParameterRow(new int[] { 2 });
model.setSelectedParameterRows(new int[] { 2 });
assertTrue(model.canMoveParameterUp());
assertTrue(model.canMoveParameterDown());
@ -338,7 +338,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
assertEquals("p3", parameters.get(2).getName());
// select the last row
model.setSelectedParameterRow(new int[] { 3 });
model.setSelectedParameterRows(new int[] { 3 });
model.moveSelectedParameterUp();
@ -382,7 +382,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
assertEquals("p3", parameters.get(2).getName());
// select the first row
model.setSelectedParameterRow(new int[] { 1 });
model.setSelectedParameterRows(new int[] { 1 });
model.moveSelectedParameterDown();
@ -661,7 +661,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
public void testParsingSignatureWithForcedIndirect() throws Exception {
setupX64windows();
assertFalse(model.canCustomizeStorage());
assertFalse(model.canUseCustomStorage());
assertEquals("void bob (void)", getSignatureText());
assertEquals(0, model.getParameters().size());
@ -707,7 +707,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
public void testParsingSignatureWithForcedIndirectAndAuto() throws Exception {
setupX64windows();
assertFalse(model.canCustomizeStorage());
assertFalse(model.canUseCustomStorage());
model.setCallingConventionName("__thiscall");
@ -826,7 +826,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
assertEquals(1, fun.getParameterCount());
model = new FunctionEditorModel(null /* use default parser*/, fun);
model.setModelChangeListener(new MyModelChangeListener());
model.setSelectedParameterRow(new int[] { 1 });
model.setSelectedParameterRows(new int[] { 1 });
model.removeParameters();
model.apply();
assertEquals(0, fun.getParameterCount());
@ -1703,7 +1703,7 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
model.parseSignatureFieldText();
assertEquals(3, model.getParameters().size());
model.setSelectedParameterRow(new int[] { 1, 3 });
model.setSelectedParameterRows(new int[] { 1, 3 });
model.setCallingConventionName("__thiscall");
assertEquals(4, model.getParameters().size());
@ -1910,14 +1910,14 @@ public class FunctionEditorModelTest extends AbstractGuiTest {
@Test
public void testCantDeleteReturnValueInTable() {
model.setSelectedParameterRow(new int[] { 0 });// select return value row
model.setSelectedParameterRows(new int[] { 0 });// select return value row
assertFalse(model.canRemoveParameters());
}
@Test
public void testCantMoveThisParameterUpOrDown() {
model.setCallingConventionName("__thiscall");
model.setSelectedParameterRow(new int[] { 1 });// select this parameter row
model.setSelectedParameterRows(new int[] { 1 });// select this parameter row
assertFalse(model.canMoveParameterUp());
assertFalse(model.canMoveParameterDown());

View File

@ -60,7 +60,7 @@ public class SpecifyCPrototypeAction extends AbstractDecompilerAction {
int autoParamCnt = modelParamCnt - decompParamCnt;
// make sure decomp params account for injected auto params
boolean useCustom = (decompParamCnt < autoParamCnt);
boolean useCustom = (decompParamCnt < autoParamCnt) | model.canUseCustomStorage();
for (int i = 0; i < autoParamCnt && !useCustom; i++) {
if (i >= decompParamCnt) {
@ -79,7 +79,7 @@ public class SpecifyCPrototypeAction extends AbstractDecompilerAction {
// remove original params which replicate auto params
for (int i = 0; i < autoParamCnt; i++) {
// be sure to select beyond auto-params. First auto-param is on row 1
model.setSelectedParameterRow(new int[] { autoParamCnt + 1 });
model.setSelectedParameterRows(new int[] { autoParamCnt + 1 });
model.removeParameters();
}
@ -167,14 +167,14 @@ public class SpecifyCPrototypeAction extends AbstractDecompilerAction {
// If editing the decompiled function (i.e., not a subfunction) and function
// is not fully locked update the model to reflect the decompiled results
if (function.getEntryPoint().equals(hf.getFunction().getEntryPoint())) {
if (function.getSignatureSource() == SourceType.DEFAULT) {
model.setUseCustomizeStorage(false);
model.setFunctionData(buildSignature(hf));
verifyDynamicEditorModel(hf, model);
}
else if (function.getReturnType() == DataType.DEFAULT) {
model.setFormalReturnType(functionPrototype.getReturnType());
if (model.canCustomizeStorage()) {
if (model.canUseCustomStorage()) {
model.setReturnStorage(functionPrototype.getReturnStorage());
}
}
@ -182,9 +182,9 @@ public class SpecifyCPrototypeAction extends AbstractDecompilerAction {
// make the model think it is not changed, so if the user doesn't change anything,
// we don't save the changes made above.
model.setModelChanged(false);
model.setModelUnchanged();
FunctionEditorDialog dialog = new FunctionEditorDialog(model);
FunctionEditorDialog dialog = new FunctionEditorDialog(model, true);
tool.showDialog(dialog, context.getComponentProvider());
}
}

View File

@ -111,7 +111,7 @@ public interface Function extends Namespace {
/**
* Returns the current call-fixup name set on this instruction or null if one has not been set
* @return the name
* @return the call fixup name or null
*/
public String getCallFixup();

View File

@ -614,7 +614,7 @@ public class VariableUtilities {
}
if (conflicts != null) {
generateConflictException(newStorage, conflicts, 4);
generateConflictException(var, newStorage, conflicts, 4);
}
}
@ -622,8 +622,8 @@ public class VariableUtilities {
* Check for variable storage conflict and optionally remove conflicting variables.
* @param existingVariables variables to check (may contain null entries)
* @param var function variable
* @param conflictHandler variable conflict handler
* @param newStorage variable storage
* @param conflictHandler variable conflict handler
* @throws VariableSizeException if another variable conflicts
*/
public static void checkVariableConflict(List<? extends Variable> existingVariables,
@ -653,7 +653,7 @@ public class VariableUtilities {
if (conflicts != null) {
if (conflictHandler == null || !conflictHandler.resolveConflicts(conflicts)) {
generateConflictException(newStorage, conflicts, 4);
generateConflictException(var, newStorage, conflicts, 4);
}
}
}
@ -667,18 +667,33 @@ public class VariableUtilities {
boolean resolveConflicts(List<Variable> conflicts);
}
private static void generateConflictException(VariableStorage newStorage,
private static void appendVariableStorageDetails(Variable var, VariableStorage storage,
StringBuilder msg) {
if (var != null) {
msg.append(var.getName());
msg.append("{");
msg.append(storage);
msg.append("}");
}
else {
msg.append(storage);
}
}
private static void generateConflictException(Variable var, VariableStorage newStorage,
List<Variable> conflicts, int maxConflictVarDetails) throws VariableSizeException {
maxConflictVarDetails = Math.min(conflicts.size(), maxConflictVarDetails);
StringBuffer msg = new StringBuffer();
msg.append("Variable storage conflict between " + newStorage + " and: ");
StringBuilder msg = new StringBuilder("Variable storage conflict between ");
appendVariableStorageDetails(var, newStorage, msg);
msg.append(" and ");
for (int i = 0; i < maxConflictVarDetails; i++) {
if (i != 0) {
msg.append(", ");
}
msg.append(conflicts.get(i).getVariableStorage().toString());
Variable v = conflicts.get(i);
appendVariableStorageDetails(v, v.getVariableStorage(), msg);
}
if (maxConflictVarDetails < conflicts.size()) {
msg.append(" ... {");