diff --git a/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/Variables.htm b/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/Variables.htm index fbdeb90b49..5b3c50ee17 100644 --- a/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/Variables.htm +++ b/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/Variables.htm @@ -29,10 +29,16 @@
  • Ordered list of named parameters and associated data types.
    • +
    +

    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 + Commit all return/parameter details checkbox to be enabled. + The user may choose to alter this full commit by checking or unchecking the corresponding + checkbox. +

    +
    - - -

    Function Attributes

    @@ -289,11 +295,9 @@
    1. Place the cursor on a function signature
      2. -
    2. Right-mouse-click, and select Edit Function.
      3. -
    3. Edit any attributes of the function using the dialog.
      4. - +
    4. Consider committing all return/parameter details
    5. Press OK to save your changes.
    @@ -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.

    + +

    Commit all return/parameter details

    + +

    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 USER_DEFINED Signature Source + which will lock-in parameter details within the Decompiler. The Signature Source + Function Listing Field can be useful for monitoring this state.

    + +
    +

    + 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.

    +
    + +

    Prior to clicking the dialog's OK 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.

    +

    Edit Parameter Storage Dialog

    diff --git a/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/images/FunctionEditor.png b/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/images/FunctionEditor.png index 93d8eff837..9913c311e2 100644 Binary files a/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/images/FunctionEditor.png and b/Ghidra/Features/Base/src/main/help/help/topics/FunctionPlugin/images/FunctionEditor.png differ diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionData.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionData.java new file mode 100644 index 0000000000..a66dc66688 --- /dev/null +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionData.java @@ -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 paramsToRemove) { + Iterator 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 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; + } + +} diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionDataView.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionDataView.java new file mode 100644 index 0000000000..bfc48ca0ef --- /dev/null +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionDataView.java @@ -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 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 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(); + } + +} diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java index bde202fe30..6ccc82ef56 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java @@ -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(""); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java index f4d63443d4..8cb92e5acb 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorModel.java @@ -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. @@ -22,12 +22,12 @@ import ghidra.app.util.cparser.C.ParseException; import ghidra.app.util.parser.FunctionSignatureParser; import ghidra.program.model.address.Address; import ghidra.program.model.data.*; -import ghidra.program.model.lang.*; +import ghidra.program.model.lang.Register; import ghidra.program.model.listing.*; import ghidra.program.model.listing.Function.FunctionUpdateType; +import ghidra.program.model.listing.VariableUtilities.VariableConflictHandler; import ghidra.program.model.pcode.Varnode; -import ghidra.program.model.symbol.SourceType; -import ghidra.program.model.symbol.SymbolUtilities; +import ghidra.program.model.symbol.*; import ghidra.util.*; import ghidra.util.exception.*; @@ -36,45 +36,33 @@ public class FunctionEditorModel { HTMLUtilities.escapeHTML(" or to commit edits, to abort"); static final String NONE_CHOICE = "-NONE-"; - private String name; + private FunctionData functionData; + private FunctionDataView originalFunctionData; + + private String signatureFieldText; + private ModelChangeListener listener; - private boolean hasVarArgs; - private ParamInfo returnInfo; - private List parameters = new ArrayList<>(); - private int autoParamCount = 0; + + private Function function; + private Program program; + + private List selectedParams = new ArrayList<>(); + + private boolean isInParsingMode; + + private DataTypeManagerService dataTypeManagerService; + private String statusText = ""; private boolean isValid = true; - private boolean signatureTransformed = false; - private boolean isInLine; - private boolean isNoReturn; - private String callingConventionName; - private Function function; - private FunctionManager functionManager; - private String callFixupName; - private int[] selectedFunctionRows = new int[0]; - private Program program; - private boolean allowCustomStorage; - private boolean isInParsingMode; - private String signatureFieldText; - private DataTypeManagerService dataTypeManagerService; - private boolean modelChanged = false; + private boolean isSignatureTransformed = false; + private boolean hasSignificantParameterChanges = false; public FunctionEditorModel(DataTypeManagerService service, Function function) { this.dataTypeManagerService = service; this.function = function; this.program = function.getProgram(); - this.name = function.getName(); - this.functionManager = program.getFunctionManager(); - allowCustomStorage = function.hasCustomVariableStorage(); - hasVarArgs = function.hasVarArgs(); - isInLine = function.isInline(); - isNoReturn = function.hasNoReturn(); - callingConventionName = function.getCallingConventionName(); - callFixupName = function.getCallFixup(); - if (callFixupName == null) { - callFixupName = NONE_CHOICE; - } - initializeParametersAndReturn(); + functionData = new FunctionData(function); + this.originalFunctionData = new FunctionDataView(functionData); validate(); } @@ -82,59 +70,21 @@ public class FunctionEditorModel { this.listener = listener; } + boolean hasChanges() { + return !functionData.equals(originalFunctionData); + } + + boolean hasSignificantParameterChanges() { + return hasSignificantParameterChanges; + } + // Returns the current calling convention or the default calling convention if current unknown - private PrototypeModel getEffectiveCallingConvention() { - PrototypeModel effectiveCallingConvention = - functionManager.getCallingConvention(callingConventionName); - if (effectiveCallingConvention == null) { - effectiveCallingConvention = functionManager.getDefaultCallingConvention(); - } - return effectiveCallingConvention; - } - - 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); - modelChanged = true; - } - - autoParamCount = 0; - Parameter[] params = function.getParameters(); - for (Parameter parameter : params) { - if (parameter.isAutoParameter()) { - ++autoParamCount; - } - parameters.add(new ParamInfo(this, parameter)); - } - - fixupOrdinals(); - } - - private boolean hasModifiedParametersOrReturn() { - if (returnInfo.isModified()) { - return true; - } - if ((function.getParameterCount() - - function.getAutoParameterCount()) != (parameters.size() - autoParamCount)) { - return true; - } - for (ParamInfo info : parameters) { - if (!info.isAutoParameter() && info.isModified()) { - return true; - } - } - return false; - } List getCallingConventionNames() { Collection names = function.getProgram().getFunctionManager().getCallingConventionNames(); List list = new ArrayList<>(names); + String callingConventionName = getCallingConventionName(); if (callingConventionName != null && !names.contains(callingConventionName)) { list.add(callingConventionName); Collections.sort(list); @@ -148,34 +98,6 @@ public class FunctionEditorModel { return program.getCompilerSpec().getPcodeInjectLibrary().getCallFixupNames(); } - void setName(String name) { - if (this.name.equals(name)) { - return; - } - this.name = name.trim(); - notifyDataChanged(); - } - - public void setCallingConventionName(String callingConventionName) { - this.callingConventionName = callingConventionName; - removeExplicitThisParameter(); - updateParameterAndReturnStorage(); - notifyDataChanged(); - } - - String getCallingConventionName() { - return callingConventionName; - } - - void setHasVarArgs(boolean b) { - hasVarArgs = b; - notifyDataChanged(); - } - - String getName() { - return name; - } - void dispose() { listener = new ModelChangeListener() { @Override @@ -191,11 +113,6 @@ public class FunctionEditorModel { } private void notifyDataChanged() { - notifyDataChanged(true); - } - - private void notifyDataChanged(boolean functionDataChanged) { - this.modelChanged |= functionDataChanged; validate(); if (listener != null) { Swing.runLater(() -> listener.dataChanged()); @@ -204,23 +121,27 @@ public class FunctionEditorModel { private void validate() { statusText = ""; - if (signatureTransformed) { - statusText = "Signature transformed due to auto-params and/or forced-indirect storage"; - signatureTransformed = false; // one-shot message + if (isSignatureTransformed) { + statusText = + "Signature transformed due to auto-params and/or forced-indirect storage change"; + isSignatureTransformed = false; // one-shot message } isValid = hasValidName() && hasValidReturnType() && hasValidReturnStorage() && hasValidParams(); + hasSignificantParameterChanges = false; if (isValid) { + hasSignificantParameterChanges = functionData.hasParameterChanges(originalFunctionData); checkUnassignedStorage(); } } private boolean hasValidReturnStorage() { - if (!allowCustomStorage) { + if (!functionData.canCustomizeStorage()) { return true; } + ParamInfo returnInfo = functionData.getReturnInfo(); VariableStorage returnStorage = returnInfo.getStorage(); DataType returnType = returnInfo.getDataType(); @@ -243,23 +164,19 @@ public class FunctionEditorModel { returnDataTypeSize + "-bytes)"; return false; } - else if (storageSize > returnDataTypeSize && storageSize <= 8 && returnDataTypeSize <= 8 && - Undefined.isUndefined(returnInfo.getDataType())) { - // grow undefined type size if needed - returnInfo.setFormalDataType(Undefined.getUndefinedDataType(storageSize)); - } return true; } private void checkUnassignedStorage() { + ParamInfo returnInfo = functionData.getReturnInfo(); VariableStorage returnStorage = returnInfo.getStorage(); DataType returnType = returnInfo.getFormalDataType(); boolean hasUnassignedStorage = returnStorage != null && returnStorage.isUnassignedStorage(); if (!hasUnassignedStorage) { - for (ParamInfo param : parameters) { + for (ParamInfo param : functionData.getParameters()) { if (param.getStorage().isUnassignedStorage()) { hasUnassignedStorage = true; break; @@ -270,9 +187,9 @@ public class FunctionEditorModel { if (hasUnassignedStorage) { statusText = "Warning: Return Storage and/or Parameter Storage is Unassigned"; } - else if (!allowCustomStorage && - Function.UNKNOWN_CALLING_CONVENTION_STRING.equals(callingConventionName)) { - if (!(VoidDataType.isVoidDataType(returnType) && parameters.isEmpty())) { + else if (!functionData.canCustomizeStorage() && Function.UNKNOWN_CALLING_CONVENTION_STRING + .equals(functionData.getCallingConventionName())) { + if (!(VoidDataType.isVoidDataType(returnType) && !functionData.hasParameters())) { statusText = "Warning: No calling convention specified. Ghidra may automatically assign one later."; } @@ -281,21 +198,82 @@ public class FunctionEditorModel { } private boolean hasValidParams() { - for (ParamInfo param : parameters) { + for (ParamInfo param : functionData.getParameters()) { if (!(isValidParamType(param) && isValidParamName(param) && isValidStorage(param))) { return false; } } - return checkForConflictingParameters(); + return hasNonConflictingStorage(); } - private boolean checkForConflictingParameters() { - // VARDO conflicting parameters + private void clearStorageConflicts() { + for (ParamInfo p : functionData.getParameters()) { + p.setHasStorageConflict(false); + } + } + + private void setStorageConflict(int ordinal) { + for (ParamInfo p : functionData.getParameters()) { + if (p.getOrdinal() == ordinal) { + p.setHasStorageConflict(true); + } + } + } + + private boolean hasNonConflictingStorage() { + clearStorageConflicts(); + if (!functionData.canCustomizeStorage()) { + return true; + } + ArrayList params = new ArrayList<>(); + for (ParamInfo paramInfo : functionData.getParameters()) { + params.add(paramInfo.getParameter(SourceType.USER_DEFINED)); + } + for (Parameter p : params) { + if (identifyStorageConflicts(p, params)) { + statusText = "One or more parameter storage conflicts exist"; + return false; + } + } return true; } + /** + * Scan the list of function parameters for those whose storage conflicts with the specified + * parameter. + * @param p parameter whose storage should be examined for conflict with others + * @param params list of function parameters to search + * @return true if storage conflict detected, else false + */ + private boolean identifyStorageConflicts(Parameter p, ArrayList params) { + try { + VariableUtilities.checkVariableConflict(params, p, p.getVariableStorage(), + conflicts -> handleConflicts(conflicts)); + } + catch (VariableSizeException e) { + // This exception occurs when any parameter storage conflict is detected/ + // Mark the tested parameter as being in conflict. + setStorageConflict(p.getOrdinal()); + return true; + } + return false; + } + + /** + * Mark all storage conflicts identified by + * {@link VariableUtilities#checkVariableConflict(List, Variable, VariableStorage, VariableConflictHandler)} + * @param conflicts parameters whose storage conflicts + * @return return false to indicate conflicts have not been resolved and additional checks + * should be disconctinued. + * @see VariableConflictHandler + */ + private boolean handleConflicts(List conflicts) { + conflicts.forEach(var -> setStorageConflict(((Parameter) var).getOrdinal())); + return false; + } + private boolean isValidStorage(ParamInfo param) { - if (!allowCustomStorage) { + if (!functionData.canCustomizeStorage()) { return true; } @@ -321,18 +299,19 @@ public class FunctionEditorModel { paramSize + "-bytes) assigned to parameter " + (param.getOrdinal() + 1); return false; } - else if (paramSize == 0) { - // assume 0-sized structure which we need to allow - } - else if (storageSize > paramSize && storageSize <= 8 && paramSize <= 8 && - Undefined.isUndefined(param.getDataType())) { - // grow undefined type size if needed - param.setFormalDataType(Undefined.getUndefinedDataType(storageSize)); - } +// else if (paramSize == 0) { +// // assume 0-sized structure which we need to allow +// } +// else if (storageSize > paramSize && storageSize <= 8 && paramSize <= 8 && +// Undefined.isUndefined(param.getDataType())) { +// // grow undefined type size if needed +// param.setFormalDataType(Undefined.getUndefinedDataType(storageSize)); +// } return true; } boolean hasValidName() { + String name = functionData.getName(); if (name.length() == 0) { statusText = "Missing function name"; return false; @@ -346,7 +325,7 @@ public class FunctionEditorModel { } private boolean hasValidReturnType() { - DataType returnType = returnInfo.getDataType(); + DataType returnType = functionData.getReturnInfo().getDataType(); if (VoidDataType.isVoidDataType(returnType)) { return true; } @@ -365,7 +344,7 @@ public class FunctionEditorModel { "Invalid name for parameter " + (param.getOrdinal() + 1) + ": " + paramName; return false; } - for (ParamInfo info : parameters) { + for (ParamInfo info : functionData.getParameters()) { if (info != param && info.getName().equals(paramName)) { statusText = "Duplicate parameter name: " + paramName; return false; @@ -393,65 +372,6 @@ public class FunctionEditorModel { return isValid; } - String getFunctionSignatureTextFromModel() { - 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(getParamNameString(param)); - - } - 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(); - } - - String getNameString() { - return name.length() == 0 ? "?" : name; - } - - private String getParamNameString(ParamInfo param) { - return param.getName(); - } - - boolean hasVarArgs() { - return hasVarArgs; - } - - DataType getReturnType() { - return returnInfo.getDataType(); - } - - DataType getFormalReturnType() { - return returnInfo.getFormalDataType(); - } - - public boolean setFormalReturnType(DataType formalReturnType) { - return setParameterFormalDataType(returnInfo, formalReturnType); - } - String getStatusText() { if (isInParsingMode) { return PARSING_MODE_STATUS_TEXT; @@ -459,22 +379,6 @@ public class FunctionEditorModel { return statusText; } - void setIsInLine(boolean isInLine) { - if (isInLine == this.isInLine) { - return; - } - this.isInLine = isInLine; - if (isInLine) { - callFixupName = NONE_CHOICE; - } - notifyDataChanged(); - } - - void setNoReturn(boolean isNoReturn) { - this.isNoReturn = isNoReturn; - notifyDataChanged(); - } - boolean isInlineAllowed() { return !getAffectiveFunction().isExternal(); } @@ -489,204 +393,156 @@ public class FunctionEditorModel { return function.isThunk() ? function.getThunkedFunction(true) : function; } + String getFunctionSignatureTextFromModel() { + return functionData.getFunctionSignatureText(); + } + + String getNameString() { + return functionData.getNameString(); + } + + String getName() { + return functionData.getName(); + } + + void setName(String name) { + if (getName().equals(name)) { + return; + } + functionData.setName(name.trim()); + notifyDataChanged(); + } + + String getCallingConventionName() { + return functionData.getCallingConventionName(); + } + + public void setCallingConventionName(String callingConventionName) { + if (Objects.equals(getCallingConventionName(), callingConventionName)) { + return; + } + functionData.setCallingConventionName(callingConventionName); + notifyDataChanged(); + } + + boolean hasVarArgs() { + return functionData.hasVarArgs(); + } + + void setHasVarArgs(boolean b) { + if (b == hasVarArgs()) { + return; + } + functionData.setVarArgs(b); + notifyDataChanged(); + } + + DataType getReturnType() { + return functionData.getReturnInfo().getDataType(); + } + + DataType getFormalReturnType() { + return functionData.getReturnInfo().getFormalDataType(); + } + + public boolean setFormalReturnType(DataType formalReturnType) { + return setParameterFormalDataType(functionData.getReturnInfo(), formalReturnType); + } + boolean isInLine() { - return isInLine; + return functionData.isInline(); + } + + void setIsInLine(boolean isInLine) { + if (isInLine == functionData.isInline()) { + return; + } + functionData.setInline(isInLine); + if (isInLine && functionData.hasCallFixup()) { + functionData.clearCallFixup(); + } + notifyDataChanged(); } boolean isNoReturn() { - return isNoReturn; + return functionData.hasNoReturn(); } - String getCallFixupName() { - return callFixupName; - } - - void setCallFixupName(String callFixupName) { - if (callFixupName.equals(this.callFixupName)) { + void setNoReturn(boolean hasNoReturn) { + if (hasNoReturn == functionData.hasNoReturn()) { return; } - this.callFixupName = callFixupName; - if (!callFixupName.equals(NONE_CHOICE)) { - isInLine = false; + functionData.setHasNoReturn(hasNoReturn); + notifyDataChanged(); + } + + String getCallFixupChoice() { + String fixupName = functionData.getCallFixupName(); + return fixupName == null ? NONE_CHOICE : fixupName; + } + + void setCallFixupChoice(String callFixupName) { + if (callFixupName.equals(getCallFixupChoice())) { + return; + } + if (NONE_CHOICE.equals(callFixupName)) { + callFixupName = null; + } + functionData.setCallFixupName(callFixupName); + if (isInLine() && functionData.hasCallFixup()) { + functionData.setInline(false); } notifyDataChanged(); } - public void setSelectedParameterRow(int[] selectedRows) { - selectedFunctionRows = selectedRows; + public void setSelectedParameterRows(int[] selectedRows) { + selectedParams.clear(); + List parameters = functionData.getParameters(); + for (int i : selectedRows) { + ParamInfo p; + if (i == 0) { + p = functionData.getReturnInfo(); + } + else { + p = parameters.get(i - 1); + } + selectedParams.add(p); + } + Collections.sort(selectedParams); notifyDataChanged(); } - private void setSelectedRow(int row) { - selectedFunctionRows = new int[] { row }; + private ParamInfo getSelectedParam() { + return selectedParams.iterator().next(); } - private void adjustSelectionForRowRemoved(int row) { - // adjust selectedParamRows - if (selectedFunctionRows.length == 0) { - return; - } - List rows = new ArrayList<>(); - for (int i : selectedFunctionRows) { - if (i < row) { - rows.add(i); - } - if (i > row) { - rows.add(i - 1); - } - } - selectedFunctionRows = new int[rows.size()]; - int index = 0; - for (int i : rows) { - selectedFunctionRows[index++] = i; - } - } - - private void adjustSelectionForRowAdded(int row) { - // adjust selectedParamRows - if (selectedFunctionRows.length == 0) { - return; - } - List rows = new ArrayList<>(); - for (int i : selectedFunctionRows) { - if (i < row) { - rows.add(i); - } - if (i >= row) { - rows.add(i + 1); - } - } - selectedFunctionRows = new int[rows.size()]; - int index = 0; - for (int i : rows) { - selectedFunctionRows[index++] = i; - } + private void setSelectedParam(ParamInfo p) { + selectedParams.clear(); + selectedParams.add(p); } int[] getSelectedParameterRows() { - return selectedFunctionRows; + List list = new ArrayList<>(); + for (ParamInfo p : selectedParams) { + list.add(p.getOrdinal() + 1); + } + Collections.sort(list); + int[] selectedRows = new int[list.size()]; + for (int i = 0; i < selectedRows.length; i++) { + selectedRows[i] = list.get(i); + } + return selectedRows; } void addParameter() { if (listener != null) { listener.tableRowsChanged(); } - ParamInfo param = new ParamInfo(this, null, DataType.DEFAULT, - VariableStorage.UNASSIGNED_STORAGE, parameters.size()); - parameters.add(param); - fixupOrdinals(); - updateParameterAndReturnStorage(); - setSelectedRow(parameters.size()); + ParamInfo p = functionData.addNewParameter(); + setSelectedParam(p); notifyDataChanged(); } - /** - * Switch to custom storage and perform required transformations - */ - private void switchToCustomStorage() { - try { - VariableStorage returnStorage = returnInfo.getStorage(); - if (returnStorage.isForcedIndirect()) { - DataType returnType = returnInfo.getDataType(); - if (returnStorage.isVoidStorage()) { - returnType = VoidDataType.dataType; - } - returnInfo.setFormalDataType(returnType); - returnInfo.setStorage(returnStorage.clone(program)); - signatureTransformed = true; - } - autoParamCount = 0; - int paramCnt = parameters.size(); - for (int i = 0; i < paramCnt; i++) { - ParamInfo paramInfo = parameters.get(i); - DataType dt = paramInfo.getDataType(); - VariableStorage storage = paramInfo.getStorage(); - signatureTransformed |= storage.isAutoStorage(); - paramInfo.setFormalDataType(dt); - paramInfo.setStorage(storage.clone(program)); - } - } - catch (InvalidInputException e) { - throw new AssertException(e); // unexpected - } - } - - private void updateParameterAndReturnStorage() { - if (allowCustomStorage) { - 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(program, dataTypes, true); - - returnInfo.setStorage(paramStorage[0]); - - List 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 { - if (autoParamCount < oldAutoCount) { - if (oldParams.get(autoParamCount) - .getStorage() - .getAutoParameterType() != storage.getAutoParameterType()) { - adjustSelectionForRowRemoved(i); - } - } - else { - adjustSelectionForRowAdded(i); - } - 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); - } - for (int i = oldAutoCount; i > autoParamCount; i--) { - adjustSelectionForRowRemoved(i); - } - fixupOrdinals(); - } - - private void fixupOrdinals() { - for (int i = 0; i < parameters.size(); i++) { - parameters.get(i).setOrdinal(i); - } - } - public void removeParameters() { if (!canRemoveParameters()) { throw new AssertException("Attempted to remove parameters when not allowed."); @@ -694,20 +550,19 @@ public class FunctionEditorModel { if (listener != null) { listener.tableRowsChanged(); } - Arrays.sort(selectedFunctionRows); - for (int i = selectedFunctionRows.length - 1; i >= 0; i--) { - int index = selectedFunctionRows[i]; - parameters.remove(index - 1); + int ordinal = selectedParams.get(0).getOrdinal(); + functionData.removeParameters(selectedParams); + selectedParams.clear(); + ParamInfo selectParam = null; + for (ParamInfo p : functionData.getParameters()) { + selectParam = p; + if (ordinal == p.getOrdinal()) { + break; + } } - if (parameters.isEmpty()) { - selectedFunctionRows = new int[0]; + if (selectParam != null) { + setSelectedParam(selectParam); } - else { - int selectRow = Math.min(selectedFunctionRows[0], parameters.size()); - selectedFunctionRows = new int[] { selectRow }; - } - fixupOrdinals(); - updateParameterAndReturnStorage(); notifyDataChanged(); } @@ -718,12 +573,8 @@ public class FunctionEditorModel { if (listener != null) { listener.tableRowsChanged(); } - int paramIndex = selectedFunctionRows[0] - 1; // first row is return value - ParamInfo param = parameters.remove(paramIndex); - parameters.add(paramIndex - 1, param); - fixupOrdinals(); - setSelectedRow(selectedFunctionRows[0] - 1); // move selection up one row - updateParameterAndReturnStorage(); + ParamInfo p = getSelectedParam(); + functionData.moveParameterUp(p.getOrdinal()); notifyDataChanged(); } @@ -734,25 +585,22 @@ public class FunctionEditorModel { if (listener != null) { listener.tableRowsChanged(); } - int paramIndex = selectedFunctionRows[0] - 1; - ParamInfo param = parameters.remove(paramIndex); - parameters.add(paramIndex + 1, param); - fixupOrdinals(); - setSelectedRow(selectedFunctionRows[0] + 1); - updateParameterAndReturnStorage(); + ParamInfo p = getSelectedParam(); + functionData.moveParameterDown(p.getOrdinal()); notifyDataChanged(); } + // TODO: Exposing this method is inappropriate public List getParameters() { - return parameters; + return functionData.getParameters(); } boolean canRemoveParameters() { - if (selectedFunctionRows.length == 0) { + if (selectedParams.size() == 0) { return false; } - for (int row : selectedFunctionRows) { - if (row <= autoParamCount) { + for (ParamInfo p : selectedParams) { + if (p.isAutoParameter() || p.isReturnParameter()) { return false; } } @@ -760,25 +608,31 @@ public class FunctionEditorModel { } boolean canMoveParameterUp() { - // remember first row (return type) and auto-params cannot be moved. - int minRowToMoveUp = 2 + autoParamCount; - if (parameters.size() > 0 && parameters.get(0).getName().equals("this")) { - minRowToMoveUp++; + if (selectedParams.size() != 1) { + return false; } - return selectedFunctionRows.length == 1 && selectedFunctionRows[0] >= minRowToMoveUp; + ParamInfo p = selectedParams.iterator().next(); + if (p.getOrdinal() <= getAutoParamCount()) { + return false; + } + return true; } boolean canMoveParameterDown() { - if (selectedFunctionRows.length != 1) { + if (selectedParams.size() != 1) { return false; } - // remember first row (return type) and auto-params cannot be moved. - int minRowToMoveDown = 1 + autoParamCount; - if (parameters.size() > 0 && parameters.get(0).getName().equals("this")) { - minRowToMoveDown++; + ParamInfo p = selectedParams.iterator().next(); + if (p.getOrdinal() < getAutoParamCount() || p.getOrdinal() >= (getParamCount() - 1)) { + return false; } - int selectedRow = selectedFunctionRows[0]; - return selectedRow >= minRowToMoveDown && selectedRow < parameters.size(); + if (canUseCustomStorage() && functionData.hasParameters()) { + List parameters = getParameters(); + if ("this".equals(parameters.get(0).getName())) { + return false; + } + } + return true; } void setParameterName(ParamInfo param, String newName) { @@ -801,7 +655,8 @@ public class FunctionEditorModel { } param.setFormalDataType(formalDataType.clone(program.getDataTypeManager())); - if (allowCustomStorage) { + + if (canUseCustomStorage()) { if (isReturn && VoidDataType.isVoidDataType(formalDataType)) { param.setStorage(VariableStorage.VOID_STORAGE); } @@ -818,7 +673,7 @@ public class FunctionEditorModel { } } else { - updateParameterAndReturnStorage(); + functionData.updateParameterAndReturnStorage(); } notifyDataChanged(); return true; @@ -851,7 +706,7 @@ public class FunctionEditorModel { } VariableStorage getReturnStorage() { - return returnInfo.getStorage(); + return functionData.getReturnInfo().getStorage(); } Function getFunction() { @@ -862,167 +717,150 @@ public class FunctionEditorModel { if (storage == null) { storage = VariableStorage.UNASSIGNED_STORAGE; } - returnInfo.setStorage(storage); + if (storage.equals(getReturnStorage())) { + return; + } + functionData.getReturnInfo().setStorage(storage); notifyDataChanged(); } + // TODO: Exposing this method is inappropriate public void setParameterStorage(ParamInfo param, VariableStorage storage) { + if (storage == null) { + storage = VariableStorage.UNASSIGNED_STORAGE; + } + if (storage.equals(param.getStorage())) { + return; + } param.setStorage(storage); notifyDataChanged(); } - 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 void removeExplicitThisParameter() { - if (!allowCustomStorage && - CompilerSpec.CALLING_CONVENTION_thiscall.equals(callingConventionName)) { - int thisIndex = findExplicitThisParameter(); - if (thisIndex >= 0) { - parameters.remove(thisIndex); // remove explicit 'this' parameter - adjustSelectionForRowRemoved(thisIndex); - } - } - } - - 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; - } - - 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(); - adjustSelectionForRowRemoved(index); - if (dt instanceof Pointer ptr) { - return ptr.getDataType(); - } - } - return null; - } - /** * Change custom storage enablement * @param b enablement state */ public void setUseCustomizeStorage(boolean b) { - if (b == allowCustomStorage) { + if (b == canUseCustomStorage()) { return; } - allowCustomStorage = b; - if (!allowCustomStorage) { - removeExplicitThisParameter(); - DataType returnDt = removeExplicitReturnStoragePtrParameter(); - if (returnDt != null) { - returnInfo.setFormalDataType(returnDt); - returnInfo.setStorage(VariableStorage.UNASSIGNED_STORAGE); - } - updateParameterAndReturnStorage(); - } - else { - switchToCustomStorage(); - } + functionData.setUseCustomStorage(b); + isSignatureTransformed = !functionData.getFunctionSignatureText() + .equals(originalFunctionData.getFunctionSignatureText()); notifyDataChanged(); } - public boolean canCustomizeStorage() { - return allowCustomStorage; + public boolean canUseCustomStorage() { + return functionData.canCustomizeStorage(); } boolean apply() { - if (!modelChanged) { - return true; - } + return apply(true); + } + + boolean apply(boolean commitFullParamDetails) { int id = program.startTransaction("Edit Function"); try { - return applyFunctionData(); + if (applyFunctionData(commitFullParamDetails)) { + setModelUnchanged(); + return true; + } + return false; } finally { program.endTransaction(id, true); } } - private boolean applyFunctionData() { + private boolean applyFunctionData(boolean commitFullParamDetails) { try { + String name = functionData.getName(); if (!name.equals(function.getName())) { function.setName(name, SourceType.USER_DEFINED); } - boolean paramsOrReturnModified = hasModifiedParametersOrReturn(); - if (!paramsOrReturnModified) { + boolean isInline = functionData.isInline(); + if (function.isInline() != isInline) { + function.setInline(isInline); + } - // change param names without impacting signature source - for (ParamInfo paramInfo : parameters) { - if (!paramInfo.isAutoParameter() && paramInfo.isNameModified()) { - Parameter param = paramInfo.getOriginalParameter(); + boolean hasNoReturn = functionData.hasNoReturn(); + if (function.hasNoReturn() != hasNoReturn) { + function.setNoReturn(hasNoReturn); + } + + String fixupName = functionData.getCallFixupName(); + if (!SystemUtilities.isEqual(fixupName, function.getCallFixup())) { + function.setCallFixup(fixupName); + } + + boolean hasVarArgs = hasVarArgs(); + if (function.hasVarArgs() != hasVarArgs) { + function.setVarArgs(hasVarArgs); + } + + String callingConventionName = functionData.getCallingConventionName(); + boolean isCallingConventionChanged = + !Objects.equals(callingConventionName, originalFunctionData.callingConventionName); + + boolean useCustomStorage = functionData.canCustomizeStorage(); + + if (!commitFullParamDetails) { + + // Partial commit without return/parameter details - no need for source type change + + if (useCustomStorage != function.hasCustomVariableStorage()) { + function.setCustomVariableStorage(useCustomStorage); + } + + if (isCallingConventionChanged) { + function.setCallingConvention(callingConventionName); + } + + if (!hasSignificantParameterChanges && + functionData.hasParameterNamesChanged(originalFunctionData)) { + + for (ParamInfo paramInfo : functionData.getParameters()) { + Parameter param = function.getParameter(paramInfo.getOrdinal()); if (param != null) { if (param.getSymbol().isDeleted()) { // concurrent removal of param - must do full update - paramsOrReturnModified = true; break; } param.setName(paramInfo.getName(), SourceType.USER_DEFINED); } } } + + return true; } - if (paramsOrReturnModified) { - List params = new ArrayList<>(); - for (ParamInfo paramInfo : parameters) { - if (paramInfo.isAutoParameter()) { - continue; - } - params.add(paramInfo.getParameter(allowCustomStorage)); - } + SourceType sigSource = + hasSignificantParameterChanges ? SourceType.USER_DEFINED : SourceType.ANALYSIS; - // TODO: How should we handle conflicts with locals? - function.updateFunction(callingConventionName, - returnInfo.getParameter(allowCustomStorage), params, - allowCustomStorage ? FunctionUpdateType.CUSTOM_STORAGE - : FunctionUpdateType.DYNAMIC_STORAGE_FORMAL_PARAMS, - true, SourceType.USER_DEFINED); - } - else { - boolean changed = false; - if (allowCustomStorage != function.hasCustomVariableStorage()) { - function.setCustomVariableStorage(allowCustomStorage); - changed = true; - } - if (!function.getCallingConventionName().equals(callingConventionName)) { - try { - function.setCallingConvention(callingConventionName); - } - catch (InvalidInputException e) { - // user had to choose from list, so can't happen - throw new AssertException("Unexpected exception", e); - } - } - if (changed && function.getSignatureSource() == SourceType.DEFAULT && - parameters.size() == 0 && - !Function.UNKNOWN_CALLING_CONVENTION_STRING.equals(callingConventionName)) { - function.setSignatureSource(SourceType.USER_DEFINED); - } + SymbolTable symbolTable = program.getSymbolTable(); + + List params = new ArrayList<>(); + for (ParamInfo paramInfo : functionData.getParameters()) { + if (paramInfo.isAutoParameter()) { + continue; + } + SourceType source = SourceType.USER_DEFINED; + Symbol var = symbolTable.getLocalVariableSymbol(paramInfo.getName(), function); + if (var instanceof Parameter) { + source = var.getSource(); + } + params.add(paramInfo.getParameter(source)); } + + // TODO: How should we handle conflicts with locals? + function.updateFunction(callingConventionName, + functionData.getReturnInfo().getParameter(SourceType.DEFAULT), params, + useCustomStorage ? FunctionUpdateType.CUSTOM_STORAGE + : FunctionUpdateType.DYNAMIC_STORAGE_FORMAL_PARAMS, + true, sigSource); } catch (DuplicateNameException e) { Msg.showError(this, null, "Function Edit Error", e.getMessage()); @@ -1033,32 +871,15 @@ public class FunctionEditorModel { return false; } - if (function.isInline() != isInLine) { - function.setInline(isInLine); - } - - if (function.hasVarArgs() != hasVarArgs) { - function.setVarArgs(hasVarArgs); - } - - if (function.hasNoReturn() != isNoReturn) { - function.setNoReturn(isNoReturn); - } - - String fixupName = callFixupName.equals(NONE_CHOICE) ? null : callFixupName; - if (!SystemUtilities.isEqual(fixupName, function.getCallFixup())) { - function.setCallFixup(fixupName); - } - return true; } private FunctionSignature getFunctionSignature() { - FunctionDefinitionDataType funDt = new FunctionDefinitionDataType(name); - funDt.setReturnType(returnInfo.getFormalDataType()); + FunctionDefinitionDataType funDt = new FunctionDefinitionDataType(getName()); + funDt.setReturnType(getFormalReturnType()); List params = new ArrayList<>(); - for (ParamInfo paramInfo : parameters) { + for (ParamInfo paramInfo : getParameters()) { if (paramInfo.isAutoParameter()) { continue; } @@ -1067,7 +888,7 @@ public class FunctionEditorModel { params.add(new ParameterDefinitionImpl(paramName, paramDt, null)); } funDt.setArguments(params.toArray(new ParameterDefinition[params.size()])); - funDt.setVarArgs(hasVarArgs); + funDt.setVarArgs(hasVarArgs()); return funDt; } @@ -1080,7 +901,11 @@ public class FunctionEditorModel { } int getAutoParamCount() { - return autoParamCount; + return functionData.getAutoParamCount(); + } + + int getParamCount() { + return functionData.getParamCount(); } private boolean isSameSize(DataType dt1, DataType dt2) { @@ -1092,38 +917,40 @@ public class FunctionEditorModel { public void setFunctionData(FunctionDefinitionDataType functionDefinition) { - name = functionDefinition.getName(); + setName(functionDefinition.getName()); setCallingConventionName(functionDefinition.getCallingConventionName()); DataType returnDt = functionDefinition.getReturnType(); - returnInfo.setFormalDataType(returnDt); + setFormalReturnType(returnDt); - List oldParams = parameters; - parameters = new ArrayList<>(); - autoParamCount = 0; - selectedFunctionRows = new int[0]; + List oldParams = new ArrayList<>(getParameters()); + functionData.removeAllParameters(); + + List parameters = functionData.getParameters(); for (ParameterDefinition paramDefinition : functionDefinition.getArguments()) { - parameters.add(new ParamInfo(this, paramDefinition)); + parameters.add(new ParamInfo(functionData, paramDefinition)); } - hasVarArgs = functionDefinition.hasVarArgs(); - fixupOrdinals(); + setHasVarArgs(functionDefinition.hasVarArgs()); - if (allowCustomStorage) { + functionData.fixupOrdinals(); + + if (canUseCustomStorage()) { if (VoidDataType.isVoidDataType(returnDt)) { - returnInfo.setStorage(VariableStorage.VOID_STORAGE); + setReturnStorage(VariableStorage.VOID_STORAGE); } - else if (!isSameSize(returnInfo.getFormalDataType(), - functionDefinition.getReturnType())) { - returnInfo.setStorage(VariableStorage.UNASSIGNED_STORAGE); + else if (!isSameSize(getFormalReturnType(), functionDefinition.getReturnType())) { + setReturnStorage(VariableStorage.UNASSIGNED_STORAGE); } reconcileCustomStorage(oldParams, parameters); } - else { - updateParameterAndReturnStorage(); - } + + selectedParams.clear(); + + functionData.updateParameterAndReturnStorage(); + notifyDataChanged(); } @@ -1178,8 +1005,7 @@ public class FunctionEditorModel { signatureFieldText.equals(getFunctionSignatureTextFromModel()); if (isInParsingMode == signatureTextFieldInSync) { isInParsingMode = !isInParsingMode; - notifyDataChanged(false); -// notifyParsingModeChanged(); + notifyDataChanged(); } } @@ -1187,7 +1013,7 @@ public class FunctionEditorModel { setSignatureFieldText(getFunctionSignatureTextFromModel()); } - boolean hasChanges() { + boolean hasSignatureTextChanges() { return !Objects.equals(getFunctionSignatureTextFromModel(), signatureFieldText); } @@ -1197,9 +1023,9 @@ public class FunctionEditorModel { FunctionDefinitionDataType f = parser.parse(getFunctionSignature(), signatureFieldText); // Preserve calling convention and noreturn flag from current model - f.setNoReturn(isNoReturn); + f.setNoReturn(functionData.hasNoReturn()); try { - f.setCallingConvention(callingConventionName); + f.setCallingConvention(getCallingConventionName()); } catch (InvalidInputException e) { // ignore @@ -1210,17 +1036,21 @@ public class FunctionEditorModel { } int getFunctionNameStartPosition() { - return returnInfo.getFormalDataType().getName().length() + 1; + return getFormalReturnType().getName().length() + 1; } /** - * Sets the change state of the model. Normally, the model sets the modelChanged variable to true - * every time something is changed. This provides a way to for applications to make some initial changes - * but make the dialog think that nothing has changed. - * @param b the new changeState for this model + * Sets the change state of the model to unchanged. Normally, the model sets the modelChanged + * variable to true every time something is changed. This provides a way to for applications + * to make some initial changes but make the dialog think that nothing has changed. */ - public void setModelChanged(boolean b) { - modelChanged = b; + public void setModelUnchanged() { + originalFunctionData = new FunctionDataView(functionData); + resetSignatureTextField(); + validate(); + if (listener != null) { + Swing.runLater(() -> listener.dataChanged()); + } } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionVariableData.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionVariableData.java index b59308611b..bce00eaef6 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionVariableData.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionVariableData.java @@ -36,4 +36,6 @@ interface FunctionVariableData { public String getName(); public DataType getFormalDataType(); + + public boolean hasStorageConflict(); } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParamInfo.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParamInfo.java index 357b74d880..14a73ddc53 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParamInfo.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParamInfo.java @@ -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 { - 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; + } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParameterTableModel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParameterTableModel.java index bf93155dbf..3d284d1010 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParameterTableModel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/ParameterTableModel.java @@ -291,6 +291,11 @@ class ParameterTableModel extends AbstractGTableModel { 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 { public void setName(String name) { // no name for return type } + + @Override + public boolean hasStorageConflict() { + return false; + } } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressEditorDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressEditorDialog.java index ecb59ff7d9..5008f00f06 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressEditorDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/StorageAddressEditorDialog.java @@ -388,6 +388,11 @@ public class StorageAddressEditorDialog extends DialogComponentProvider public DataType getFormalDataType() { return variable.getDataType(); } + + @Override + public boolean hasStorageConflict() { + return false; + } } } diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/FunctionEditorModelTest.java b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/FunctionEditorModelTest.java index cda7621120..18541d0844 100644 --- a/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/FunctionEditorModelTest.java +++ b/Ghidra/Features/Base/src/test/java/ghidra/app/plugin/core/function/editor/FunctionEditorModelTest.java @@ -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()); diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/SpecifyCPrototypeAction.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/SpecifyCPrototypeAction.java index acb079f956..aeffece704 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/SpecifyCPrototypeAction.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/SpecifyCPrototypeAction.java @@ -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()); } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Function.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Function.java index 35b490b1f6..96f5a84c67 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Function.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Function.java @@ -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(); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java index 89845b3c5d..043c0e7d43 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/VariableUtilities.java @@ -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 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 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 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(" ... {");