GP-2689 - add length check to EnumDataType

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

@@ -21,9 +21,10 @@ import java.util.Set;
 /**
  * Class used to organize long values into sets of values with overlapping bits.
  * For example, if you had values 1,2,3, 8, 12, you could partition them into two bit groups.
- * The values 1,2,3, would be in one bit group because they all either use the "1" or "2" bit
+ * The values 1,2,3, would be in one bit group because they all use the "1" or "2" bit.
- * (If there was on "3", then 1 and 2 could be in separate groups). Also the values "8" and "12"
+ * (If there was no "3" enum value, then the "1" bit and the "2" bit would be in separate groups
- * are in the same group since they share the "8" bit.
+ * since there are no enum values that share any bits.) Also, the values "8" and "12" are in the same
+ * group since they share the "8" bit.
  */
 public class BitGroup {
 	private Set<Long> values = new HashSet<>();

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

@@ -160,29 +160,6 @@ public class EnumDataType extends GenericDataType implements Enum {
 
 	}
 
-	private void checkValue(long value) {
-		if (length == 8) {
-			return; // all long values permitted
-		}
-		// compute maximum enum value as a positive value: (2^length)-1
-		long max = (1L << (getLength() * 8)) - 1;
-		if (value > max) {
-			throw new IllegalArgumentException(
-				getName() + " enum value 0x" + Long.toHexString(value) +
-					" is outside the range of 0x0 to 0x" + Long.toHexString(max));
-
-		}
-	}
-
-	private boolean isTooBig(int testLength, long value) {
-		if (length == 8) {
-			return false; // all long values permitted
-		}
-		// compute maximum enum value as a positive value: (2^length)-1
-		long max = (1L << (testLength * 8)) - 1;
-		return value > max;
-	}
-
 	@Override
 	public void remove(String valueName) {
 		bitGroups = null;
@@ -239,17 +216,54 @@ public class EnumDataType extends GenericDataType implements Enum {
 		return length;
 	}
 
-	public void setLength(int length) {
+	public void setLength(int newLength) {
+		if (newLength == length) {
+			return;
+		}
+		if (length < 1 || length > 8) {
+			throw new IllegalArgumentException("Enum length must be between 1 and 8 inclusive");
+		}
+
+		checkValues(newLength);
+		this.length = newLength;
+	}
+
+	private void checkValues(int newLength) {
+		if (newLength == 8) {
+			return; // all long values permitted
+		}
+
+		long newMaxValue = getMaxEnumValue(newLength);
 		String[] names = getNames();
 		for (String valueName : names) {
 			long value = getValue(valueName);
-			if (isTooBig(length, value)) {
+			if (value > newMaxValue) {
 				throw new IllegalArgumentException("Setting the length of this Enum to a size " +
-					"that cannot contain the current value for \"" + valueName + "\" of " +
+					"that cannot contain the current value for \"" + valueName + "\" of 0x" +
-					Long.toHexString(value));
+					Long.toHexString(value) + "\nOld length: " + length + "; new length: " +
+					newLength);
 			}
 		}
-		this.length = length;
+	}
+
+	private void checkValue(long value) {
+		if (length == 8) {
+			return; // all long values permitted
+		}
+
+		long max = getMaxEnumValue(length);
+		if (value > max) {
+			throw new IllegalArgumentException(
+				getName() + " enum value 0x" + Long.toHexString(value) +
+					" is outside the range of 0x0 to 0x" + Long.toHexString(max));
+
+		}
+	}
+
+	private long getMaxEnumValue(int bytes) {
+		int bits = bytes * 8;     // number of bits used for the given size
+		long power2 = 1L << bits; // 2^length is the number of values that 'bytes' can represent
+		return power2 - 1;		  // max value is always 1 less than 2^length (0-based)
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/test.slow/java/ghidra/program/model/data/EnumTest.java

@@ -239,4 +239,33 @@ public class EnumTest extends AbstractGTest {
 		}
 	}
 
+	@Test
+	public void testSetLength() {
+		EnumDataType enumm = new EnumDataType("Color", 1);
+		enumm.setLength(2);
+		assertEquals(2, enumm.getLength());
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testSetLength_TooBig() {
+		EnumDataType enumm = new EnumDataType("Color", 1);
+		enumm.setLength(10);
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testSetLength_TooSmall() {
+		EnumDataType enumm = new EnumDataType("Color", 1);
+		enumm.setLength(0);
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testSetLength_TooSmallForCurrentValues() {
+		int bytes = 8;
+		EnumDataType enumm = new EnumDataType("Color", bytes);
+		int currentBits = 8 * 8 - 1;
+		long maxValue = (1L << currentBits) - 1;
+		enumm.add("My Name", maxValue);
+
+		enumm.setLength(7);
+	}
 }