diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java index 79f186bd1f..8530215c53 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/CompositeDB.java @@ -213,12 +213,11 @@ abstract class CompositeDB extends DataTypeDB implements CompositeInternal { try { checkDeleted(); record.setString(CompositeDBAdapter.COMPOSITE_COMMENT_COL, desc); - try { - compositeAdapter.updateRecord(record, true); - } - catch (IOException e) { - dataMgr.dbError(e); - } + compositeAdapter.updateRecord(record, true); + dataMgr.dataTypeChanged(this, false); + } + catch (IOException e) { + dataMgr.dbError(e); } finally { lock.release(); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java index de9c7902d5..6268716c45 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeManagerDB.java @@ -1372,25 +1372,23 @@ abstract public class DataTypeManagerDB implements DataTypeManager { SourceArchive sourceArchive) throws DataTypeDependencyException { try { - if (existingDataType instanceof StructureDB) { - if (!(dataType instanceof StructureInternal)) { + if (existingDataType instanceof StructureDB existingStruct) { + if (!(dataType instanceof StructureInternal replacementStruct)) { return false; } - StructureDB existingStruct = (StructureDB) existingDataType; - existingStruct.doReplaceWith((StructureInternal) dataType, true); + existingStruct.doReplaceWith(replacementStruct, true); } - else if (existingDataType instanceof UnionDB) { - if (!(dataType instanceof UnionInternal)) { + else if (existingDataType instanceof UnionDB existingUnion) { + if (!(dataType instanceof UnionInternal replacementUnion)) { return false; } - UnionDB existingUnion = (UnionDB) existingDataType; - existingUnion.doReplaceWith((UnionInternal) dataType, true); + existingUnion.doReplaceWith(replacementUnion, true); } - else if (existingDataType instanceof FunctionDefinitionDB) { - if (!(dataType instanceof FunctionDefinition)) { + else if (existingDataType instanceof FunctionDefinitionDB existingFuncDef) { + if (!(dataType instanceof FunctionDefinition replacementFuncDef)) { return false; } - existingDataType.replaceWith(dataType); + existingFuncDef.doReplaceWith(replacementFuncDef, true); } else if (existingDataType instanceof EnumDB) { if (!(dataType instanceof Enum)) { @@ -3015,7 +3013,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager { category.dataTypeAdded(structDB); structDB.doReplaceWith(struct, false); - structDB.setDescription(struct.getDescription()); // doReplaceWith may have updated the last change time so set it back to what we want. structDB.setLastChangeTime(struct.getLastChangeTime()); @@ -3079,7 +3076,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager { category.dataTypeAdded(unionDB); unionDB.doReplaceWith(union, false); - unionDB.setDescription(union.getDescription()); // doReplaceWith updated the last change time so set it back to what we want. unionDB.setLastChangeTime(union.getLastChangeTime()); @@ -3326,8 +3322,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager { // Make sure category knows about function definition before args/return resolved cat.dataTypeAdded(funDefDb); - funDefDb.setArguments(funDef.getArguments()); - funDefDb.setReturnType(funDef.getReturnType()); + funDefDb.doReplaceWith(funDef, false); // setArguments updated the last change time so set it back to what we want. funDefDb.setLastChangeTime(funDef.getLastChangeTime()); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java index 94af56ce3f..10f8fd403e 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/FunctionDefinitionDB.java @@ -16,7 +16,8 @@ package ghidra.program.database.data; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; import db.DBRecord; import db.Field; @@ -153,10 +154,10 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } @Override - public ParameterDefinition[] getArguments() { + public ParameterDefinitionDB[] getArguments() { lock.acquire(); try { - ParameterDefinition[] vars = new ParameterDefinition[parameters.size()]; + ParameterDefinitionDB[] vars = new ParameterDefinitionDB[parameters.size()]; return parameters.toArray(vars); } finally { @@ -183,41 +184,77 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { @Override public void replaceWith(DataType dataType) { - if (!(dataType instanceof FunctionDefinition)) { + if (!(dataType instanceof FunctionDefinition functionDefinition)) { throw new IllegalArgumentException(); } - doReplaceWith((FunctionDefinition) dataType); - } - - private void doReplaceWith(FunctionDefinition functionDefinition) { - lock.acquire(); + boolean isResolveCacheOwner = dataMgr.activateResolveCache(); try { checkDeleted(); - setArguments(functionDefinition.getArguments()); - try { - setReturnType(functionDefinition.getReturnType()); - } - catch (IllegalArgumentException e) { - setReturnType(DEFAULT); - } - setVarArgs(functionDefinition.hasVarArgs()); - setNoReturn(functionDefinition.hasNoReturn()); - try { - setCallingConvention(functionDefinition.getCallingConventionName(), false); - } - catch (InvalidInputException e) { - // will not happen - } + doReplaceWith(functionDefinition, true); } catch (IOException e) { dataMgr.dbError(e); } finally { + if (isResolveCacheOwner) { + dataMgr.flushResolveQueue(true); + } lock.release(); } } + void doReplaceWith(FunctionDefinition functionDefinition, boolean notify) throws IOException { + doSetArguments(functionDefinition.getArguments(), true, false); + try { + doSetReturnType(functionDefinition.getReturnType(), true, false); + } + catch (IllegalArgumentException e) { + setReturnType(DEFAULT); + } + doSetVarArgs(functionDefinition.hasVarArgs(), false); + doSetNoReturn(functionDefinition.hasNoReturn(), false); + try { + doSetCallingConvention(functionDefinition.getCallingConventionName(), false, false); + } + catch (InvalidInputException e) { + // will not happen + } + + record.setString(FunctionDefinitionDBAdapter.FUNCTION_DEF_COMMENT_COL, + functionDefinition.getComment()); + funDefAdapter.updateRecord(record, false); + + if (notify) { + dataMgr.dataTypeChanged(this, false); + } + + if (pointerPostResolveRequired) { + dataMgr.queuePostResolve(this, functionDefinition); + } + } + + @Override + protected void postPointerResolve(DataType definitionDt, DataTypeConflictHandler handler) { + FunctionDefinition funcDef = (FunctionDefinition) definitionDt; + ParameterDefinition[] definedArguments = funcDef.getArguments(); + ParameterDefinitionDB[] myArguments = getArguments(); + if (definedArguments.length != myArguments.length) { + throw new IllegalArgumentException("mismatched definition datatype"); + } + for (int i = 0; i < definedArguments.length; i++) { + ParameterDefinition arg = definedArguments[i]; + DataType dt = arg.getDataType(); + if (dt instanceof Pointer) { + myArguments[i].doSetDataType(dt, false); + } + } + DataType dt = funcDef.getReturnType(); + if (dt instanceof Pointer) { + doSetReturnType(dt, false, false); + } + } + @Override public String getComment() { lock.acquire(); @@ -274,29 +311,20 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { return getPrototypeString(); } + private DataType doCheckedResolve(DataType dt) { + if (dt instanceof Pointer) { + pointerPostResolveRequired = true; + return resolve(((Pointer) dt).newPointer(DataType.DEFAULT)); + } + return resolve(dt); + } + @Override public void setArguments(ParameterDefinition[] args) { lock.acquire(); try { checkDeleted(); - Iterator it = parameters.iterator(); - while (it.hasNext()) { - ParameterDefinitionDB param = it.next(); - param.getDataType().removeParent(this); - paramAdapter.removeRecord(param.getKey()); - } - parameters.clear(); - for (int i = 0; i < args.length; i++) { - DataType type = - ParameterDefinitionImpl.validateDataType(args[i].getDataType(), dataMgr, false); - DataType resolvedDt = resolve(type); - paramAdapter.createRecord(dataMgr.getID(resolvedDt), key, i, args[i].getName(), - args[i].getComment(), args[i].getLength()); - resolvedDt.addParent(this); - } - loadParameters(); - funDefAdapter.updateRecord(record, true); // update last change time - dataMgr.dataTypeChanged(this, false); + doSetArguments(args, false, true); } catch (IOException e) { dataMgr.dbError(e); @@ -306,29 +334,60 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } + private void doSetArguments(ParameterDefinition[] args, boolean checkedResolveReqd, + boolean notify) throws IOException { + + for (ParameterDefinitionDB param : parameters) { + param.getDataType().removeParent(this); + paramAdapter.removeRecord(param.getKey()); + } + parameters.clear(); + for (int i = 0; i < args.length; i++) { + DataType type = + ParameterDefinitionImpl.validateDataType(args[i].getDataType(), dataMgr, false); + DataType resolvedDt = checkedResolveReqd ? doCheckedResolve(type) : resolve(type); + paramAdapter.createRecord(dataMgr.getID(resolvedDt), key, i, args[i].getName(), + args[i].getComment(), args[i].getLength()); + resolvedDt.addParent(this); + } + loadParameters(); + funDefAdapter.updateRecord(record, true); // update last change time + if (notify) { + dataMgr.dataTypeChanged(this, false); + } + } + @Override public void setReturnType(DataType type) { type = ParameterDefinitionImpl.validateDataType(type, dataMgr, true); lock.acquire(); try { checkDeleted(); + doSetReturnType(type, false, true); + } + finally { + lock.release(); + } + } + + private void doSetReturnType(DataType type, boolean checkedResolveReqd, boolean notify) { + try { getReturnType().removeParent(this); if (type == null) { type = DataType.DEFAULT; } - DataType resolvedDt = resolve(type); + DataType resolvedDt = checkedResolveReqd ? doCheckedResolve(type) : resolve(type); record.setLongValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_RETURN_ID_COL, dataMgr.getID(resolvedDt)); funDefAdapter.updateRecord(record, true); resolvedDt.addParent(this); - dataMgr.dataTypeChanged(this, false); + if (notify) { + dataMgr.dataTypeChanged(this, false); + } } catch (IOException e) { dataMgr.dbError(e); } - finally { - lock.release(); - } } @Override @@ -558,59 +617,79 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { lock.acquire(); try { checkDeleted(); - byte flags = record.getByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL); - if (hasVarArgs) { - flags |= FunctionDefinitionDBAdapter.FUNCTION_DEF_VARARG_FLAG; - } - else { - flags &= ~FunctionDefinitionDBAdapter.FUNCTION_DEF_VARARG_FLAG; - } - record.setByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL, flags); - try { - funDefAdapter.updateRecord(record, true); - dataMgr.dataTypeChanged(this, false); - } - catch (IOException e) { - dataMgr.dbError(e); - } + doSetVarArgs(hasVarArgs, true); + } + catch (IOException e) { + dataMgr.dbError(e); } finally { lock.release(); } } + private void doSetVarArgs(boolean hasVarArgs, boolean notify) throws IOException { + + if (hasVarArgs == hasVarArgs()) { + return; + } + + byte flags = record.getByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL); + if (hasVarArgs) { + flags |= FunctionDefinitionDBAdapter.FUNCTION_DEF_VARARG_FLAG; + } + else { + flags &= ~FunctionDefinitionDBAdapter.FUNCTION_DEF_VARARG_FLAG; + } + record.setByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL, flags); + funDefAdapter.updateRecord(record, true); + + if (notify) { + dataMgr.dataTypeChanged(this, false); + } + } + @Override public void setNoReturn(boolean hasNoReturn) { lock.acquire(); try { checkDeleted(); - byte flags = record.getByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL); - if (hasNoReturn) { - flags |= FunctionDefinitionDBAdapter.FUNCTION_DEF_NORETURN_FLAG; - } - else { - flags &= ~FunctionDefinitionDBAdapter.FUNCTION_DEF_NORETURN_FLAG; - } - record.setByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL, flags); - try { - funDefAdapter.updateRecord(record, true); - dataMgr.dataTypeChanged(this, false); - } - catch (IOException e) { - dataMgr.dbError(e); - } + doSetNoReturn(hasNoReturn, true); + } + catch (IOException e) { + dataMgr.dbError(e); } finally { lock.release(); } } + private void doSetNoReturn(boolean hasNoReturn, boolean notify) throws IOException { + + if (hasNoReturn == hasNoReturn()) { + return; + } + + byte flags = record.getByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL); + if (hasNoReturn) { + flags |= FunctionDefinitionDBAdapter.FUNCTION_DEF_NORETURN_FLAG; + } + else { + flags &= ~FunctionDefinitionDBAdapter.FUNCTION_DEF_NORETURN_FLAG; + } + record.setByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_FLAGS_COL, flags); + funDefAdapter.updateRecord(record, true); + + if (notify) { + dataMgr.dataTypeChanged(this, false); + } + } + @Override public void setGenericCallingConvention(GenericCallingConvention genericCallingConvention) { lock.acquire(); try { checkDeleted(); - setCallingConvention(genericCallingConvention.name(), false); + doSetCallingConvention(genericCallingConvention.name(), false, true); } catch (IOException e) { dataMgr.dbError(e); @@ -628,7 +707,7 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { lock.acquire(); try { checkDeleted(); - setCallingConvention(conventionName, true); + doSetCallingConvention(conventionName, true, true); } catch (IOException e) { dataMgr.dbError(e); @@ -638,12 +717,14 @@ class FunctionDefinitionDB extends DataTypeDB implements FunctionDefinition { } } - private void setCallingConvention(String conventionName, boolean restrictive) + private void doSetCallingConvention(String conventionName, boolean restrictive, boolean notify) throws InvalidInputException, IOException { byte id = dataMgr.getCallingConventionID(conventionName, restrictive); record.setByteValue(FunctionDefinitionDBAdapter.FUNCTION_DEF_CALLCONV_COL, id); funDefAdapter.updateRecord(record, true); - dataMgr.dataTypeChanged(this, false); + if (notify) { + dataMgr.dataTypeChanged(this, false); + } } @Override diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java index d3f034dd9f..aef4260a94 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/ParameterDefinitionDB.java @@ -61,19 +61,32 @@ final class ParameterDefinitionDB implements ParameterDefinition { @Override public void setDataType(DataType type) { - type = ParameterDefinitionImpl.validateDataType(type, dataMgr, false); + // TODO: This is not a DatabaseObject so it lacks ability to refresh properly + // and parameter objects are not singletons. It also lacks locking. + // There is risk of using a stale or deleted parameter object which could lead + // to corruption of a function definition and datatype parent tracking. - getDataType().removeParent(parent); + doSetDataType(type, true); + } - type = dataMgr.resolve(type, null); - type.addParent(parent); - - record.setLongValue(FunctionParameterAdapter.PARAMETER_DT_ID_COL, - dataMgr.getResolvedID(type)); - record.setIntValue(FunctionParameterAdapter.PARAMETER_DT_LENGTH_COL, type.getLength()); + void doSetDataType(DataType type, boolean notify) { try { + + type = ParameterDefinitionImpl.validateDataType(type, dataMgr, false); + + getDataType().removeParent(parent); + + type = dataMgr.resolve(type, null); + type.addParent(parent); + + record.setLongValue(FunctionParameterAdapter.PARAMETER_DT_ID_COL, + dataMgr.getResolvedID(type)); + record.setIntValue(FunctionParameterAdapter.PARAMETER_DT_LENGTH_COL, type.getLength()); adapter.updateRecord(record); - dataMgr.dataTypeChanged(parent, false); + + if (notify) { + dataMgr.dataTypeChanged(parent, false); + } } catch (IOException e) { dataMgr.dbError(e); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java index 8c1a76c7f0..7423a0d814 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java @@ -1645,6 +1645,7 @@ class StructureDB extends CompositeDB implements StructureInternal { record.setIntValue(CompositeDBAdapter.COMPOSITE_NUM_COMPONENTS_COL, numComponents); record.setIntValue(CompositeDBAdapter.COMPOSITE_LENGTH_COL, structLength); record.setIntValue(CompositeDBAdapter.COMPOSITE_ALIGNMENT_COL, structAlignment); + record.setString(CompositeDBAdapter.COMPOSITE_COMMENT_COL, struct.getDescription()); compositeAdapter.updateRecord(record, true); // updates timestamp if (notify) { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java index 662a275253..e5f258c394 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/UnionDB.java @@ -344,6 +344,9 @@ class UnionDB extends CompositeDB implements UnionInternal { doAdd(resolvedDts[i], dtc.getLength(), dtc.getFieldName(), dtc.getComment(), false); } + record.setString(CompositeDBAdapter.COMPOSITE_COMMENT_COL, union.getDescription()); + compositeAdapter.updateRecord(record, false); + repack(false, false); // updates timestamp if (notify) {