diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DataAdapterFromSettings.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DataAdapterFromSettings.java index 3f6bf417d8..0fde046a1e 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DataAdapterFromSettings.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DataAdapterFromSettings.java @@ -47,6 +47,11 @@ public interface DataAdapterFromSettings extends Data { return hasMutability(MutabilitySettingsDefinition.CONSTANT); } + @Override + default boolean isWritable() { + return hasMutability(MutabilitySettingsDefinition.WRITABLE); + } + @Override default boolean isVolatile() { return hasMutability(MutabilitySettingsDefinition.VOLATILE); diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileCallback.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileCallback.java index fa841b195e..1276d3daf7 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileCallback.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileCallback.java @@ -30,7 +30,8 @@ import ghidra.program.model.data.*; import ghidra.program.model.lang.*; import ghidra.program.model.lang.ConstantPool.Record; import ghidra.program.model.listing.*; -import ghidra.program.model.mem.*; +import ghidra.program.model.mem.MemoryAccessException; +import ghidra.program.model.mem.MemoryBufferImpl; import ghidra.program.model.pcode.*; import ghidra.program.model.symbol.*; import ghidra.util.Msg; @@ -840,9 +841,8 @@ public class DecompileCallback { else { highSymbol = new HighCodeSymbol(0, SymbolUtilities.getDynamicName(program, data.getAddress()), data, dtmanage); - SymbolEntry entry = highSymbol.getFirstWholeMap(); - if (data.getDataType() == DataType.DEFAULT && !entry.isReadOnly() && - !entry.isVolatile()) { + if (data.getDataType() == DataType.DEFAULT && + highSymbol.getMutability() == MutabilitySettingsDefinition.NORMAL) { return false; } } @@ -875,53 +875,6 @@ public class DecompileCallback { encodeResult(encoder, labelSymbol, namespc); } - /** - * Check address is read only. This only checks whether the block containing - * the address is read-only. It does not, and should not, check if there is - * a data object that has been set to constant - * - * @param addr - address to check - * - * @return true if the block is read_only, and there are no write - * references. - */ - private boolean isReadOnlyNoData(Address addr) { - boolean readonly = false; - MemoryBlock block = program.getMemory().getBlock(addr); - if (block != null) { - readonly = !block.isWrite(); - // if the block says read-only, check the refs to the variable - // if the block says read-only, check the refs to the variable - if (readonly) { - ReferenceIterator refIter = program.getReferenceManager().getReferencesTo(addr); - int count = 0; -// boolean foundRead = false; - while (refIter.hasNext() && count < 100) { - Reference ref = refIter.next(); - if (ref.getReferenceType().isWrite()) { - readonly = false; - break; - } - if (ref.getReferenceType().isRead()) { -// foundRead = true; - } - count++; - } - // TODO: Don't do override if no read reference found - // - // if we only have indirect refs to it, don't assume readonly! - //if (!foundRead && readonly && count > 1) { - // readonly = false; - //} - // they must be reading it multiple times for some reason - // if (readonly && count > 1) { - // readonly = false; - // } - } - } - return readonly; - } - /** * This function deals with the vagaries of the getMappedSymbols * interface when the queried address is in the body of a function. @@ -967,16 +920,15 @@ public class DecompileCallback { if (range.contains(addr)) { Address first = range.getMinAddress(); Address last = range.getMaxAddress(); - boolean readonly = true; // Treat function body as readonly encodeHole(encoder, first.getAddressSpace(), first.getUnsignedOffset(), - last.getUnsignedOffset(), readonly, false); + last.getUnsignedOffset(), MutabilitySettingsDefinition.CONSTANT); return; } } // There is probably some sort of error, just return a block // containing the single queried address encodeHole(encoder, addr.getAddressSpace(), addr.getUnsignedOffset(), - addr.getUnsignedOffset(), true, false); + addr.getUnsignedOffset(), MutabilitySettingsDefinition.CONSTANT); } private int getExtraPopOverride(Function func, Address addr) { @@ -1013,10 +965,14 @@ public class DecompileCallback { } private void encodeHole(Encoder encoder, AddressSpace spc, long first, long last, - boolean readonly, boolean isVolatile) throws IOException { + int mutability) throws IOException { encoder.openElement(ELEM_HOLE); - encoder.writeBool(ATTRIB_READONLY, readonly); - encoder.writeBool(ATTRIB_VOLATILE, isVolatile); + if (mutability == MutabilitySettingsDefinition.CONSTANT) { + encoder.writeBool(ATTRIB_READONLY, true); + } + else if (mutability == MutabilitySettingsDefinition.VOLATILE) { + encoder.writeBool(ATTRIB_VOLATILE, true); + } encoder.writeSpace(ATTRIB_SPACE, spc); encoder.writeUnsignedInteger(ATTRIB_FIRST, first); encoder.writeUnsignedInteger(ATTRIB_LAST, last); @@ -1038,10 +994,9 @@ public class DecompileCallback { * @throws IOException for errors in the underlying stream */ private void encodeHole(Encoder encoder, Address addr) throws IOException { - boolean readonly = isReadOnlyNoData(addr); - boolean isvolatile = isVolatileNoData(addr); + int mutability = MappedEntry.getMutabilityOfAddress(addr, program); encodeHole(encoder, addr.getAddressSpace(), addr.getUnsignedOffset(), - addr.getUnsignedOffset(), readonly, isvolatile); + addr.getUnsignedOffset(), mutability); } private void encodeExternalRef(Encoder encoder, Address addr, ExternalReference ref) @@ -1135,20 +1090,6 @@ public class DecompileCallback { return null; } - /** - * Check whether the address is volatile. Do not check the data object. - * - * @param addr is address to check for volatility - * @return true if the address is volatile - */ - private boolean isVolatileNoData(Address addr) { - if (program.getLanguage().isVolatile(addr)) { - return true; - } - MemoryBlock block = program.getMemory().getBlock(addr); - return (block != null && block.isVolatile()); - } - private Function getFunctionContaining(Address addr) { if (cachedFunction != null && cachedFunction.getBody().contains(addr)) { return cachedFunction; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/util/PseudoData.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/util/PseudoData.java index 6ee5a8174b..7474c1a59d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/util/PseudoData.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/util/PseudoData.java @@ -544,6 +544,11 @@ public class PseudoData extends PseudoCodeUnit implements Data { return false; } + @Override + public boolean isWritable() { + return false; + } + @Override public boolean isVolatile() { return false; diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java index b9b285b40e..e4fa0f0dda 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/DataDB.java @@ -130,7 +130,7 @@ class DataDB extends CodeUnitDB implements Data { private void computeLength() { // NOTE: Data intentionally does not use aligned-length - length = dataType.getLength(); + length = dataType.getLength(); // undefined will never change their size if (dataType instanceof Undefined) { @@ -367,6 +367,11 @@ class DataDB extends CodeUnitDB implements Data { return hasMutability(MutabilitySettingsDefinition.CONSTANT); } + @Override + public boolean isWritable() { + return hasMutability(MutabilitySettingsDefinition.WRITABLE); + } + @Override public boolean isVolatile() { return hasMutability(MutabilitySettingsDefinition.VOLATILE); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/MutabilitySettingsDefinition.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/MutabilitySettingsDefinition.java index c7cf3a30bd..e9ca3c9429 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/MutabilitySettingsDefinition.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/MutabilitySettingsDefinition.java @@ -27,9 +27,10 @@ public class MutabilitySettingsDefinition implements EnumSettingsDefinition { public static final int NORMAL = 0; public static final int VOLATILE = 1; public static final int CONSTANT = 2; + public static final int WRITABLE = 3; //NOTE: if these strings change, the XML needs to changed also... - private static final String[] choices = { "normal", "volatile", "constant" }; + private static final String[] choices = { "normal", "volatile", "constant", "writable" }; public static final String MUTABILITY = "mutability"; public static final MutabilitySettingsDefinition DEF = new MutabilitySettingsDefinition(); @@ -51,7 +52,7 @@ public class MutabilitySettingsDefinition implements EnumSettingsDefinition { return NORMAL; } int mode = (int) value.longValue(); - if ((mode < 0) || (mode > CONSTANT)) { + if ((mode < 0) || (mode > WRITABLE)) { mode = NORMAL; } return mode; @@ -69,7 +70,7 @@ public class MutabilitySettingsDefinition implements EnumSettingsDefinition { @Override public void setChoice(Settings settings, int value) { - if (value < 0 || value > CONSTANT) { + if (value < 0 || value > WRITABLE) { settings.clearSetting(MUTABILITY); } else { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Data.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Data.java index 9ca8ffef6e..44e6faf9d7 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Data.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/Data.java @@ -54,14 +54,26 @@ public interface Data extends CodeUnit, Settings { public boolean hasStringValue(); /** - * @return true if data is constant. - * If true, isConstant will always be false + * Determine if this data has explicitly been marked as constant. + * NOTE: This is based upon explicit {@link Data} and {@link DataType} mutability settings + * and does not reflect independent memory block or processor specification settings. + * @return true if data is constant, else false. */ public boolean isConstant(); /** - * @return true if data is volatile. - * If true, isVolatile will always be false + * Determine if this data has explicitly been marked as writable. + * NOTE: This is based upon explicit {@link Data} and {@link DataType} mutability settings + * and does not reflect independent memory block or processor specification settings. + * @return true if data is writable, else false. + */ + public boolean isWritable(); + + /** + * Determine if this data has explicitly been marked as volatile. + * NOTE: This is based upon explicit {@link Data} and {@link DataType} mutability settings + * and does not reflect independent memory block or processor specification settings. + * @return true if data is volatile, else false. */ public boolean isVolatile(); @@ -285,4 +297,5 @@ public interface Data extends CodeUnit, Settings { * @return the prefix */ public String getDefaultLabelPrefix(DataTypeDisplayOptions options); + } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/DataStub.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/DataStub.java index 28ed59133f..2bcff01401 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/DataStub.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/DataStub.java @@ -392,6 +392,11 @@ public class DataStub implements Data { throw new UnsupportedOperationException(); } + @Override + public boolean isWritable() { + throw new UnsupportedOperationException(); + } + @Override public boolean isVolatile() { throw new UnsupportedOperationException(); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/DynamicEntry.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/DynamicEntry.java index d8dbad18fc..2930170018 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/DynamicEntry.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/DynamicEntry.java @@ -22,6 +22,7 @@ import java.io.IOException; import ghidra.program.model.address.Address; import ghidra.program.model.address.AddressSpace; +import ghidra.program.model.data.MutabilitySettingsDefinition; import ghidra.program.model.listing.Program; import ghidra.program.model.listing.VariableStorage; import ghidra.util.exception.AssertException; @@ -115,13 +116,8 @@ public class DynamicEntry extends SymbolEntry { } @Override - public boolean isReadOnly() { - return false; - } - - @Override - public boolean isVolatile() { - return false; + public int getMutability() { + return MutabilitySettingsDefinition.NORMAL; } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighSymbol.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighSymbol.java index 904ffaf698..8bd432d808 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighSymbol.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighSymbol.java @@ -22,6 +22,7 @@ import java.io.IOException; import ghidra.program.model.address.Address; import ghidra.program.model.data.DataType; +import ghidra.program.model.data.MutabilitySettingsDefinition; import ghidra.program.model.lang.DynamicVariableStorage; import ghidra.program.model.listing.*; import ghidra.program.model.symbol.Namespace; @@ -314,10 +315,14 @@ public class HighSymbol { } /** - * @return true if the symbol's value is considered read-only (by the decompiler) + * Return one of + * - MutabilitySettingsDefinition.NORMAL + * - MutabilitySettingsDefinition.VOLATILE + * - MutabilitySettingsDefinition.CONSTANT + * @return the mutability setting */ - public boolean isReadOnly() { - return entryList[0].isReadOnly(); + public int getMutability() { + return entryList[0].getMutability(); } /** @@ -384,9 +389,11 @@ public class HighSymbol { encoder.writeString(ATTRIB_NAME, name); encoder.writeBool(ATTRIB_TYPELOCK, typelock); encoder.writeBool(ATTRIB_NAMELOCK, namelock); - encoder.writeBool(ATTRIB_READONLY, isReadOnly()); - boolean isVolatile = entryList[0].isVolatile(); - if (isVolatile) { + int mutability = getMutability(); + if (mutability == MutabilitySettingsDefinition.CONSTANT) { + encoder.writeBool(ATTRIB_READONLY, true); + } + else if (mutability == MutabilitySettingsDefinition.VOLATILE) { encoder.writeBool(ATTRIB_VOLATILE, true); } if (isIsolated()) { diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedDataEntry.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedDataEntry.java index 0088576b37..db77011a25 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedDataEntry.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedDataEntry.java @@ -15,6 +15,7 @@ */ package ghidra.program.model.pcode; +import ghidra.program.model.data.MutabilitySettingsDefinition; import ghidra.program.model.listing.Data; import ghidra.program.model.listing.VariableStorage; @@ -57,18 +58,16 @@ public class MappedDataEntry extends MappedEntry { } @Override - public boolean isReadOnly() { - if (data.isConstant()) { - return true; - } - return super.isReadOnly(); - } - - @Override - public boolean isVolatile() { + public int getMutability() { if (data.isVolatile()) { - return true; + return MutabilitySettingsDefinition.VOLATILE; } - return super.isVolatile(); + if (data.isConstant()) { + return MutabilitySettingsDefinition.CONSTANT; + } + if (data.isWritable()) { + return MutabilitySettingsDefinition.NORMAL; + } + return super.getMutability(); } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedEntry.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedEntry.java index b701cbbe54..248679c9c2 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedEntry.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/MappedEntry.java @@ -19,6 +19,7 @@ import java.io.IOException; import ghidra.program.model.address.Address; import ghidra.program.model.data.AbstractFloatDataType; +import ghidra.program.model.data.MutabilitySettingsDefinition; import ghidra.program.model.listing.Program; import ghidra.program.model.listing.VariableStorage; import ghidra.program.model.mem.MemoryBlock; @@ -87,59 +88,44 @@ public class MappedEntry extends SymbolEntry { } @Override - public boolean isReadOnly() { + public int getMutability() { Address addr = storage.getMinAddress(); + return getMutabilityOfAddress(addr, symbol.getProgram()); + } + + /** + * Get the underlying mutability setting of an Address based on the Program + * configuration and the MemoryBlock. Ignore any overrides of Data at the address. + * @param addr is the Address + * @param program is the Program containing the Address + * @return the mutability + */ + public static int getMutabilityOfAddress(Address addr, Program program) { if (addr == null) { - return false; + return MutabilitySettingsDefinition.NORMAL; + } + if (program.getLanguage().isVolatile(addr)) { + return MutabilitySettingsDefinition.VOLATILE; } - boolean readonly = false; - Program program = symbol.getProgram(); MemoryBlock block = program.getMemory().getBlock(addr); if (block != null) { - readonly = !block.isWrite(); + if (block.isVolatile()) { + return MutabilitySettingsDefinition.VOLATILE; + } // if the block says read-only, check the refs to the variable - // if the block says read-only, check the refs to the variable - if (readonly) { + if (!block.isWrite()) { ReferenceIterator refIter = program.getReferenceManager().getReferencesTo(addr); int count = 0; -// boolean foundRead = false; while (refIter.hasNext() && count < 100) { Reference ref = refIter.next(); if (ref.getReferenceType().isWrite()) { - readonly = false; - break; - } - if (ref.getReferenceType().isRead()) { -// foundRead = true; + return MutabilitySettingsDefinition.NORMAL; } count++; } - // TODO: Don't do override if no read reference found - // - // if we only have indirect refs to it, don't assume readonly! - //if (!foundRead && readonly && count > 1) { - // readonly = false; - //} - // they must be reading it multiple times for some reason - // if (readonly && count > 1) { - // readonly = false; - // } + return MutabilitySettingsDefinition.CONSTANT; } } - return readonly; - } - - @Override - public boolean isVolatile() { - Address addr = storage.getMinAddress(); - if (addr == null) { - return false; - } - Program program = symbol.getProgram(); - if (program.getLanguage().isVolatile(addr)) { - return true; - } - MemoryBlock block = program.getMemory().getBlock(addr); - return (block != null && block.isVolatile()); + return MutabilitySettingsDefinition.NORMAL; } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/SymbolEntry.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/SymbolEntry.java index dfa050263d..277b760212 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/SymbolEntry.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/SymbolEntry.java @@ -67,14 +67,13 @@ public abstract class SymbolEntry { public abstract int getSize(); /** - * @return true if the mapped storage is read-only + * Return one of + * - MutabilitySettingsDefinition.NORMAL + * - MutabilitySettingsDefinition.VOLATILE + * - MutabilitySettingsDefinition.CONSTANT + * @return the mutability setting */ - public abstract boolean isReadOnly(); - - /** - * @return true if the mapped storage is volatile - */ - public abstract boolean isVolatile(); + public abstract int getMutability(); /** * The storage used to hold this Symbol may be used for other purposes at different points in diff --git a/GhidraDocs/GhidraClass/Beginner/Introduction_to_Ghidra_Student_Guide.html b/GhidraDocs/GhidraClass/Beginner/Introduction_to_Ghidra_Student_Guide.html index eb5f25790e..cabad05724 100644 --- a/GhidraDocs/GhidraClass/Beginner/Introduction_to_Ghidra_Student_Guide.html +++ b/GhidraDocs/GhidraClass/Beginner/Introduction_to_Ghidra_Student_Guide.html @@ -1119,7 +1119,7 @@ Before you can do anything else, you must first create a project. Projects are u
  • Decompiler output may change due to program changes
  • Selections and navigation made in Listing and Decompiler track one another
    • @@ -1214,7 +1214,7 @@ Before you can do anything else, you must first create a project. Projects are u
  • Change function signatures to cause global improvements
  • Make function signature changes to handle specialized types of functions
    • -
  • Apply read-only memory and volatile memory settings correctly
    • +
  • Apply writable memory, read-only memory and volatile memory settings correctly