GP-2355 Improved Enum handling of negative values. Previously, the GUI only supported unsigned values even thought the API supported signed values.

This commit is contained in:
ghidragon 2023-04-07 15:39:03 -04:00
parent acb07dd535
commit a2a787b404
9 changed files with 970 additions and 111 deletions

View file

@ -15,6 +15,8 @@
*/
package ghidra.program.database.data;
import static ghidra.program.database.data.EnumSignedState.*;
import java.io.IOException;
import java.math.BigInteger;
import java.util.*;
@ -37,7 +39,6 @@ import ghidra.util.UniversalID;
* Database implementation for the enumerated data type.
*/
class EnumDB extends DataTypeDB implements Enum {
private static final SettingsDefinition[] ENUM_SETTINGS_DEFINITIONS =
new SettingsDefinition[] { MutabilitySettingsDefinition.DEF };
@ -45,9 +46,10 @@ class EnumDB extends DataTypeDB implements Enum {
private EnumValueDBAdapter valueAdapter;
private Map<String, Long> nameMap; // name to value
private TreeMap<Long, List<String>> valueMap; // value to names
private SortedMap<Long, List<String>> valueMap; // value to names
private Map<String, String> commentMap; // name to comment
private List<BitGroup> bitGroups;
private EnumSignedState signedState = null;
EnumDB(DataTypeManagerDB dataMgr, DBObjectCache<DataTypeDB> cache, EnumDBAdapter adapter,
EnumValueDBAdapter valueAdapter, DBRecord record) {
@ -97,6 +99,27 @@ class EnumDB extends DataTypeDB implements Enum {
String comment = rec.getString(EnumValueDBAdapter.ENUMVAL_COMMENT_COL);
addToCache(valueName, value, comment);
}
signedState = computeSignedness();
}
private EnumSignedState computeSignedness() {
int length = record.getByteValue(EnumDBAdapter.ENUM_SIZE_COL);
if (valueMap.isEmpty()) {
return NONE;
}
long minValue = valueMap.firstKey();
long maxValue = valueMap.lastKey();
if (minValue < 0) {
return SIGNED;
}
if (maxValue > getMaxPossibleValue(length, true)) {
return UNSIGNED;
}
return NONE; // we have no negatives and no large unsigned values
}
private void addToCache(String valueName, long value, String comment) {
@ -258,8 +281,8 @@ class EnumDB extends DataTypeDB implements Enum {
lock.acquire();
try {
checkDeleted();
checkValue(value);
initializeIfNeeded();
checkValue(value);
if (nameMap.containsKey(valueName)) {
throw new IllegalArgumentException(valueName + " already exists in this enum");
}
@ -272,6 +295,7 @@ class EnumDB extends DataTypeDB implements Enum {
valueAdapter.createRecord(key, valueName, value, comment);
adapter.updateRecord(record, true);
addToCache(valueName, value, comment);
signedState = computeSignedness();
dataMgr.dataTypeChanged(this, false);
}
catch (IOException e) {
@ -283,17 +307,17 @@ class EnumDB extends DataTypeDB implements Enum {
}
private void checkValue(long value) {
int length = getLength();
int length = record.getByteValue(EnumDBAdapter.ENUM_SIZE_COL);
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));
long min = getMinPossibleValue();
long max = getMaxPossibleValue();
if (value < min || value > max) {
throw new IllegalArgumentException(
"Attempted to add a value outside the range for this enum: (" + min + ", " + max +
"): " + value);
}
}
@ -317,6 +341,7 @@ class EnumDB extends DataTypeDB implements Enum {
}
}
adapter.updateRecord(record, true);
signedState = computeSignedness();
dataMgr.dataTypeChanged(this, false);
}
catch (IOException e) {
@ -366,7 +391,7 @@ class EnumDB extends DataTypeDB implements Enum {
adapter.updateRecord(record, true);
addToCache(valueName, value, comment);
}
signedState = computeSignedness();
if (oldLength != newLength) {
notifySizeChanged(false);
}
@ -615,6 +640,56 @@ class EnumDB extends DataTypeDB implements Enum {
}
}
@Override
public long getMinPossibleValue() {
lock.acquire();
try {
checkIsValid();
int length = record.getByteValue(EnumDBAdapter.ENUM_SIZE_COL);
return getMinPossibleValue(length, signedState != UNSIGNED);
}
finally {
lock.release();
}
}
@Override
public long getMaxPossibleValue() {
lock.acquire();
try {
checkIsValid();
int length = record.getByteValue(EnumDBAdapter.ENUM_SIZE_COL);
return getMaxPossibleValue(length, signedState == SIGNED);
}
finally {
lock.release();
}
}
private long getMaxPossibleValue(int bytes, boolean allowNegativeValues) {
if (bytes == 8) {
return Long.MAX_VALUE;
}
int bits = bytes * 8;
if (allowNegativeValues) {
bits -= 1; // take away 1 bit for the sign
}
// the largest value that can be held in n bits in 2^n -1
return (1L << bits) - 1;
}
private long getMinPossibleValue(int bytes, boolean allowNegativeValues) {
if (!allowNegativeValues) {
return 0;
}
int bits = bytes * 8;
// smallest value (largest negative) that can be stored in n bits is when the sign bit
// is on (and sign extended), and all less significant bits are 0
return -1L << (bits - 1);
}
@Override
protected boolean refresh() {
try {
@ -754,4 +829,71 @@ class EnumDB extends DataTypeDB implements Enum {
lock.release();
}
}
@Override
public boolean contains(String name) {
lock.acquire();
try {
checkIsValid();
initializeIfNeeded();
return nameMap.containsKey(name);
}
finally {
lock.release();
}
}
@Override
public boolean contains(long value) {
lock.acquire();
try {
checkIsValid();
initializeIfNeeded();
return valueMap.containsKey(value);
}
finally {
lock.release();
}
}
@Override
public boolean isSigned() {
lock.acquire();
try {
checkIsValid();
initializeIfNeeded();
return signedState == SIGNED;
}
finally {
lock.release();
}
}
@Override
public int getMinimumPossibleLength() {
lock.acquire();
try {
if (valueMap.isEmpty()) {
return 1;
}
long minValue = valueMap.firstKey();
long maxValue = valueMap.lastKey();
boolean hasNegativeValues = minValue < 0;
// check the min and max values in this enum to see if they fit in 1 byte enum, then
// 2 byte enum, then 4 byte enum. If the min min and max values fit, then all other values
// will fit as well
for (int size = 1; size < 8; size *= 2) {
long minPossible = getMinPossibleValue(size, hasNegativeValues);
long maxPossible = getMaxPossibleValue(size, hasNegativeValues);
if (minValue >= minPossible && maxValue <= maxPossible) {
return size;
}
}
return 8;
}
finally {
lock.release();
}
}
}

View file

@ -0,0 +1,30 @@
/* ###
* IP: GHIDRA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package ghidra.program.database.data;
/**
* 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.
*/
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
}

View file

@ -107,4 +107,50 @@ public interface Enum extends DataType {
* @return formatted integer string
*/
public String getRepresentation(BigInteger bigInt, Settings settings, int bitLength);
/**
* Returns true if this enum has an entry with the given name.
* @param name the name to check for an entry
* @return true if this enum has an entry with the given name
*/
public boolean contains(String name);
/**
* Returns true if this enum has an entry with the given value.
* @param value the value to check for an entry
* @return true if this enum has an entry with the given value
*/
public boolean contains(long value);
/**
* Returns true if the enum contains at least one negative value. Internally, enums have
* three states, signed, unsigned, and none (can't tell from the values). If any of
* the values are negative, the enum is considered signed. If any of the values are large
* unsigned values (upper bit set), then it is considered unsigned. This method will return
* true if the enum is signed, and false if it is either unsigned or none (meaning that it
* doesn't matter for the values that are contained in the enum.
* @return true if the enum contains at least one negative value
*/
public boolean isSigned();
/**
* 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.
*/
public long getMaxPossibleValue();
/**
* 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.
*/
public long getMinPossibleValue();
/**
* Returns the smallest length (size in bytes) this enum can be and still represent all of
* it's current values. Note that that this will only return powers of 2 (1,2,4, or 8)
* @return the smallest length (size in bytes) this enum can be and still represent all of
* it's current values
*/
public int getMinimumPossibleLength();
}

View file

@ -15,6 +15,8 @@
*/
package ghidra.program.model.data;
import static ghidra.program.database.data.EnumSignedState.*;
import java.math.BigInteger;
import java.util.*;
@ -23,6 +25,7 @@ import org.apache.commons.lang3.StringUtils;
import ghidra.docking.settings.Settings;
import ghidra.docking.settings.SettingsDefinition;
import ghidra.program.database.data.DataTypeUtilities;
import ghidra.program.database.data.EnumSignedState;
import ghidra.program.model.mem.MemBuffer;
import ghidra.program.model.mem.MemoryAccessException;
import ghidra.program.model.scalar.Scalar;
@ -34,11 +37,12 @@ public class EnumDataType extends GenericDataType implements Enum {
new SettingsDefinition[] { MutabilitySettingsDefinition.DEF };
private Map<String, Long> nameMap; // name to value
private TreeMap<Long, List<String>> valueMap; // value to names
private Map<String, String> commentMap; // name to comment
private SortedMap<Long, List<String>> valueMap; // value to names
private int length;
private String description;
private List<BitGroup> bitGroups;
private EnumSignedState signedState = NONE;
public EnumDataType(String name, int length) {
this(CategoryPath.ROOT, name, length, null);
@ -157,9 +161,27 @@ public class EnumDataType extends GenericDataType implements Enum {
if (!StringUtils.isBlank(comment)) {
commentMap.put(valueName, comment);
}
signedState = computeSignedness();
}
private EnumSignedState computeSignedness() {
if (valueMap.isEmpty()) {
return NONE;
}
long minValue = valueMap.firstKey();
long maxValue = valueMap.lastKey();
if (minValue < 0) {
return SIGNED;
}
if (maxValue > getMaxPossibleValue(length, true)) {
return UNSIGNED;
}
return NONE; // we have no negatives and no large unsigned values
}
@Override
public void remove(String valueName) {
bitGroups = null;
@ -182,6 +204,7 @@ public class EnumDataType extends GenericDataType implements Enum {
}
commentMap.remove(valueName);
signedState = computeSignedness();
}
@Override
@ -220,50 +243,90 @@ public class EnumDataType extends GenericDataType implements Enum {
if (newLength == length) {
return;
}
if (newLength < 1 || newLength > 8) {
throw new IllegalArgumentException("Enum length must be between 1 and 8 inclusive");
}
checkValues(newLength);
int minLength = getMinimumPossibleLength();
if (newLength < minLength || newLength > 8) {
throw new IllegalArgumentException(
"Enum length must be between " + minLength + "and 8 inclusive");
}
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 (value > newMaxValue) {
throw new IllegalArgumentException("Setting the length of this Enum to a size " +
"that cannot contain the current value for \"" + valueName + "\" of 0x" +
Long.toHexString(value) + "\nOld length: " + length + "; new length: " +
newLength);
}
}
}
private void checkValue(long value) {
if (length == 8) {
return; // all long values permitted
}
long max = getMaxEnumValue(length);
if (value > max) {
long min = getMinPossibleValue();
long max = getMaxPossibleValue();
if (value < min || value > max) {
throw new IllegalArgumentException(
getName() + " enum value 0x" + Long.toHexString(value) +
" is outside the range of 0x0 to 0x" + Long.toHexString(max));
"Attempted to add a value outside the range for this enum: (" + min + ", " + max +
"): " + value);
}
}
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
public boolean isSigned() {
return signedState == SIGNED;
}
@Override
public long getMinPossibleValue() {
return getMinPossibleValue(length, signedState != UNSIGNED);
}
@Override
public long getMaxPossibleValue() {
return getMaxPossibleValue(length, signedState == SIGNED);
}
@Override
public int getMinimumPossibleLength() {
if (valueMap.isEmpty()) {
return 1;
}
long minValue = valueMap.firstKey();
long maxValue = valueMap.lastKey();
boolean hasNegativeValues = minValue < 0;
// check the min and max values in this enum to see if they fit in 1 byte enum, then
// 2 byte enum, then 4 byte enum. If the min min and max values fit, then all other values
// will fit as well
for (int size = 1; size < 8; size *= 2) {
long minPossible = getMinPossibleValue(size, hasNegativeValues);
long maxPossible = getMaxPossibleValue(size, hasNegativeValues);
if (minValue >= minPossible && maxValue <= maxPossible) {
return size;
}
}
return 8;
}
private long getMaxPossibleValue(int bytes, boolean allowNegativeValues) {
if (bytes == 8) {
return Long.MAX_VALUE;
}
int bits = bytes * 8;
if (allowNegativeValues) {
bits -= 1; // take away 1 bit for the sign
}
// the largest value that can be held in n bits in 2^n -1
return (1L << bits) - 1;
}
private long getMinPossibleValue(int bytes, boolean allowNegativeValues) {
if (!allowNegativeValues) {
return 0;
}
int bits = bytes * 8;
// smallest value (largest negative) that can be stored in n bits is when the sign bit
// is on (and sign extended), and all less significant bits are 0
return -1L << (bits - 1);
}
@Override
@ -438,10 +501,30 @@ public class EnumDataType extends GenericDataType implements Enum {
for (String valueName : names) {
add(valueName, enumm.getValue(valueName), enumm.getComment(valueName));
}
computeSignedness();
}
@Override
public String getDefaultLabelPrefix() {
return name;
}
@Override
public boolean contains(String entryName) {
return nameMap.containsKey(entryName);
}
@Override
public boolean contains(long value) {
return valueMap.containsKey(value);
}
/**
* Sets this enum to it smallest (power of 2) size that it can be and still represent all its
* current values.
*/
public void pack() {
setLength(getMinimumPossibleLength());
}
}

View file

@ -15,6 +15,8 @@
*/
package ghidra.program.model.data;
import static org.junit.Assert.*;
import org.junit.*;
import ghidra.program.model.address.Address;
@ -52,6 +54,244 @@ public class EnumDataTypeTest {
BigEndianDataConverter.INSTANCE.getBytes(Integer.MIN_VALUE), true);
Assert.assertEquals("bob", enumDt.getRepresentation(memBuffer, null, 0));
}
@Test
public void testCanAddHighUnsignedValue() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", 0xff);
assertEquals(0xff, enumDt.getValue("a"));
}
@Test
public void testCanAddNegativeValue() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", -1);
assertEquals(-1, enumDt.getValue("a"));
}
@Test
public void testCantAddNegativeAndHighUnsignedValue() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", -1);
try {
enumDt.add("b", 0xff);
fail("Expected Illegal ArgumentException");
}
catch (IllegalArgumentException e) {
// expected
}
}
@Test
public void testCantAddHighUnsignedAndNegativeValue() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", 0xff);
try {
enumDt.add("b", -1);
fail("Expected Illegal ArgumentException");
}
catch (IllegalArgumentException e) {
// expected
}
}
@Test
public void testCanAddNegativeAfterAddingThenRemovingHighUnsignedValue() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", 0xff);
enumDt.remove("a");
enumDt.add("a", -1);
assertEquals(-1, enumDt.getValue("a"));
}
@Test
public void testGetMinPossibleWidthUnsigned() {
EnumDataType enumDt = new EnumDataType("Test", 4);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("a", 0);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("b", 0xff);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("c", 0x100);
assertEquals(2, enumDt.getMinimumPossibleLength());
enumDt.add("d", 0xffff);
assertEquals(2, enumDt.getMinimumPossibleLength());
enumDt.add("e", 0x1ffff);
assertEquals(4, enumDt.getMinimumPossibleLength());
enumDt.add("f", 0xffffffffL);
assertEquals(4, enumDt.getMinimumPossibleLength());
}
@Test
public void testGetMinPossibleLengthFor8ByteEnum() {
EnumDataType enumDt = new EnumDataType("Test", 8);
enumDt.add("a", -1);
assertEquals(1, enumDt.getMinimumPossibleLength());
}
@Test
public void test8ByteEnums() {
EnumDataType enumDt = new EnumDataType("Test", 8);
enumDt.add("a", 0);
assertEquals(Long.MIN_VALUE, enumDt.getMinPossibleValue());
assertEquals(Long.MAX_VALUE, enumDt.getMaxPossibleValue());
assertFalse(enumDt.isSigned());
enumDt.add("b", -1);
assertTrue(enumDt.isSigned());
}
@Test
public void testGetMinPossibleWidthSigned() {
EnumDataType enumDt = new EnumDataType("Test", 4);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("a", 0);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("b", -0x1);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("c", 0xff);
assertEquals(2, enumDt.getMinimumPossibleLength());
enumDt.add("d", 0xffff);
assertEquals(4, enumDt.getMinimumPossibleLength());
enumDt.add("e", 0x1ffff);
assertEquals(4, enumDt.getMinimumPossibleLength());
}
@Test
public void testGetMinPossibleWidthSignedWithBigNegatives() {
EnumDataType enumDt = new EnumDataType("Test", 4);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("a", 0);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("b", -0x1);
assertEquals(1, enumDt.getMinimumPossibleLength());
enumDt.add("c", -0xff);
assertEquals(2, enumDt.getMinimumPossibleLength());
enumDt.add("d", -0xffff);
assertEquals(4, enumDt.getMinimumPossibleLength());
enumDt.add("e", -0x1ffff);
assertEquals(4, enumDt.getMinimumPossibleLength());
}
@Test
public void testMinMaxPossibleValuesNoSignednessSize1() {
EnumDataType enumDt = new EnumDataType("Test", 1);
assertEquals(-0x80, enumDt.getMinPossibleValue());
assertEquals(0xff, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesNoSignednessSize2() {
EnumDataType enumDt = new EnumDataType("Test", 2);
assertEquals(-0x8000, enumDt.getMinPossibleValue());
assertEquals(0xffff, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesNoSignednessSize4() {
EnumDataType enumDt = new EnumDataType("Test", 4);
assertEquals(-0x80000000L, enumDt.getMinPossibleValue());
assertEquals(0xffffffffL, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesNoSignednessSize8() {
EnumDataType enumDt = new EnumDataType("Test", 8);
assertEquals(Long.MIN_VALUE, enumDt.getMinPossibleValue());
assertEquals(Long.MAX_VALUE, enumDt.getMaxPossibleValue());
}
public void testMinMaxPossibleValuesSignedSize1() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", -1); // this makes it an signed enum
assertEquals(-0x80, enumDt.getMinPossibleValue());
assertEquals(0x7f, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesSignedSize2() {
EnumDataType enumDt = new EnumDataType("Test", 2);
enumDt.add("a", -1); // this makes it an signed enum
assertEquals(-0x8000, enumDt.getMinPossibleValue());
assertEquals(0x7fff, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesSignedSize4() {
EnumDataType enumDt = new EnumDataType("Test", 4);
enumDt.add("a", -1); // this makes it an signed enum
assertEquals(-0x80000000L, enumDt.getMinPossibleValue());
assertEquals(0x7fffffffL, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesSignedSize8() {
EnumDataType enumDt = new EnumDataType("Test", 8);
enumDt.add("a", -1); // this makes it an signed enum
assertEquals(Long.MIN_VALUE, enumDt.getMinPossibleValue());
assertEquals(Long.MAX_VALUE, enumDt.getMaxPossibleValue());
}
public void testMinMaxPossibleValuesUnsignedSize1() {
EnumDataType enumDt = new EnumDataType("Test", 1);
enumDt.add("a", 0xff); // this makes it an unsigned enum
assertEquals(0, enumDt.getMinPossibleValue());
assertEquals(0xff, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesUnsignedSize2() {
EnumDataType enumDt = new EnumDataType("Test", 2);
enumDt.add("a", 0xffff); // this makes it an signed enum
assertEquals(0, enumDt.getMinPossibleValue());
assertEquals(0xffff, enumDt.getMaxPossibleValue());
}
@Test
public void testMinMaxPossibleValuesUnsignedSize4() {
EnumDataType enumDt = new EnumDataType("Test", 4);
enumDt.add("a", 0xffffffffL); // this makes it an signed enum
assertEquals(0, enumDt.getMinPossibleValue());
assertEquals(0xffffffffL, enumDt.getMaxPossibleValue());
}
@Test
public void testContainsName() {
EnumDataType enumDt = new EnumDataType("Test", 4);
enumDt.add("a", 0x1);
enumDt.add("b", -1);
assertTrue(enumDt.contains("a"));
assertTrue(enumDt.contains("b"));
assertFalse(enumDt.contains("c"));
}
@Test
public void testContainsValue() {
EnumDataType enumDt = new EnumDataType("Test", 4);
enumDt.add("a", 0x1);
enumDt.add("b", -1);
assertTrue(enumDt.contains(-1));
assertTrue(enumDt.contains(1));
assertFalse(enumDt.contains(3));
}
}