Merge remote-tracking branch 'origin/GP-4505_ghidra1_MutabilitySettings'

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);

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;

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;

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);

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 {

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);
+
 }

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();

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;
 	}
 
 }

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()) {

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();
 	}
 }

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;
 	}
 }

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

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
   <li>Decompiler output may change due to program changes</li>
   <ul>
     <li>Changes to data or functions called by the current function</li>
-    <li>Changes to data mutability (read-only/constant or volatile)</li>
+    <li>Changes to data mutability (writable, read-only/constant or volatile)</li>
   </ul>
 <li>Selections and navigation made in Listing and Decompiler track one another</li>
 
@@ -1214,7 +1214,7 @@ Before you can do anything else, you must first create a project. Projects are u
   <br>
   <li>Change function signatures to cause global improvements</li>
   <li>Make function signature changes to handle specialized types of functions</li>
-  <li>Apply read-only memory and volatile memory settings correctly</li>
+  <li>Apply writable memory, read-only memory and volatile memory settings correctly</li>
 </ul>
 <div role="note">
   <br>