Merge remote-tracking branch 'origin/GP-1-dragonmacher-enum-signedness-fix' into patch

Author: ryanmkurtz

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumDB.java

@@ -112,13 +112,17 @@ private EnumSignedState computeSignedness() {
 		long minValue = valueMap.firstKey();
 		long maxValue = valueMap.lastKey();
 
-		if (minValue < 0) {
-			return SIGNED;
-		}
 		if (maxValue > getMaxPossibleValue(length, true)) {
+			if (minValue < 0) {
+				return INVALID;
+			}
 			return UNSIGNED;
 		}
 
+		if (minValue < 0) {
+			return SIGNED;
+		}
+
 		return NONE;		// we have no negatives and no large unsigned values
 	}
 
@@ -874,6 +878,19 @@ public boolean isSigned() {
 		}
 	}
 
+	@Override
+	public EnumSignedState getSignedState() {
+		lock.acquire();
+		try {
+			checkIsValid();
+			initializeIfNeeded();
+			return signedState;
+		}
+		finally {
+			lock.release();
+		}
+	}
+
 	@Override
 	public int getMinimumPossibleLength() {
 		lock.acquire();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/EnumSignedState.java

@@ -18,13 +18,14 @@
 /**
  * Keeps track of the signed state of an enum datatype. Enum are fundamentally either signed or
  * unsigned, but sometimes you can't tell based on the values they contain. Once a negative value
- * is added, then the enum becomes locked as signed, preventing high unsigned values from being 
- * added. Once a high value unsigned value is added, then it becomes locked as unsigned value. If
- * neither a negative value or high unsigned value has been added, then the enum is not locked as
- * either signed or unsigned.
+ * is added, then the enum becomes locked as signed, preventing high unsigned values (those values
+ * that are too big for signed value of the enum size) from being added. Once a high value unsigned 
+ * value is added, then it becomes locked as unsigned value. If neither a negative value or high 
+ * unsigned value has been added, then the enum is not locked as either signed or unsigned.
  */
 public enum EnumSignedState {
 	SIGNED, 	// Enum contains at least 1 negative value, preventing high unsigned values
 	UNSIGNED,   // Enum contains at least 1 high unsigned value, preventing negative values
-	NONE	    // Enum contains neither a negative or a high unsigned value, so can go either way
+	NONE,	    // Enum contains neither a negative or a high unsigned value, so can go either way
+	INVALID     // Enum contains both negative and high unsigned values; can happen with old types
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/Enum.java

@@ -19,6 +19,7 @@
 import java.util.NoSuchElementException;
 
 import ghidra.docking.settings.Settings;
+import ghidra.program.database.data.EnumSignedState;
 
 public interface Enum extends DataType {
 
@@ -133,6 +134,12 @@ public interface Enum extends DataType {
 	 */
 	public boolean isSigned();
 
+	/**
+	 * Returns the signed state.
+	 * @return the signed state.
+	 */
+	public EnumSignedState getSignedState();
+
 	/**
 	 * Returns the maximum value that this enum can represent based on its size and signedness.
 	 * @return the maximum value that this enum can represent based on its size and signedness.

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/EnumDataType.java

@@ -148,6 +148,11 @@ public void add(String valueName, long value) {
 
 	@Override
 	public void add(String valueName, long value, String comment) {
+		doAdd(valueName, value, comment);
+		signedState = computeSignedness();
+	}
+
+	private void doAdd(String valueName, long value, String comment) {
 		bitGroups = null;
 		checkValue(value);
 		if (nameMap.containsKey(valueName)) {
@@ -161,8 +166,6 @@ public void add(String valueName, long value, String comment) {
 		if (!StringUtils.isBlank(comment)) {
 			commentMap.put(valueName, comment);
 		}
-		signedState = computeSignedness();
-
 	}
 
 	private EnumSignedState computeSignedness() {
@@ -172,13 +175,17 @@ private EnumSignedState computeSignedness() {
 		long minValue = valueMap.firstKey();
 		long maxValue = valueMap.lastKey();
 
-		if (minValue < 0) {
-			return SIGNED;
-		}
 		if (maxValue > getMaxPossibleValue(length, true)) {
+			if (minValue < 0) {
+				return INVALID;
+			}
 			return UNSIGNED;
 		}
 
+		if (minValue < 0) {
+			return SIGNED;
+		}
+
 		return NONE;		// we have no negatives and no large unsigned values
 	}
 
@@ -277,6 +284,11 @@ public boolean isSigned() {
 		return signedState == SIGNED;
 	}
 
+	@Override
+	public EnumSignedState getSignedState() {
+		return signedState;
+	}
+
 	@Override
 	public long getMinPossibleValue() {
 		return getMinPossibleValue(length, signedState != UNSIGNED);
@@ -503,10 +515,10 @@ public void replaceWith(DataType dataType) {
 		commentMap = new HashMap<>();
 		setLength(enumm.getLength());
 		String[] names = enumm.getNames();
+		signedState = enumm.getSignedState();
 		for (String valueName : names) {
-			add(valueName, enumm.getValue(valueName), enumm.getComment(valueName));
+			doAdd(valueName, enumm.getValue(valueName), enumm.getComment(valueName));
 		}
-		computeSignedness();
 	}
 
 	@Override