Merge remote-tracking branch

'origin/GP-3278_dev747368_dwarf_enum_signedness' (Closes #5180)

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/attribs/DWARFAttributeFactory.java

@@ -111,13 +111,13 @@ public class DWARFAttributeFactory {
 				return new DWARFBlobAttribute(reader.readNextByteArray(length));
 			}
 			case DW_FORM_data1:
-				return new DWARFNumericAttribute(8, reader.readNextByte(), true);
+				return new DWARFNumericAttribute(8, reader.readNextByte(), true, true);
 			case DW_FORM_data2:
-				return new DWARFNumericAttribute(16, reader.readNextShort(), true);
+				return new DWARFNumericAttribute(16, reader.readNextShort(), true, true);
 			case DW_FORM_data4:
-				return new DWARFNumericAttribute(32, reader.readNextInt(), true);
+				return new DWARFNumericAttribute(32, reader.readNextInt(), true, true);
 			case DW_FORM_data8:
-				return new DWARFNumericAttribute(64, reader.readNextLong(), true);
+				return new DWARFNumericAttribute(64, reader.readNextLong(), true, true);
 			case DW_FORM_sdata:
 				return new DWARFNumericAttribute(64, reader.readNext(LEB128::signed), true);
 			case DW_FORM_udata:

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/attribs/DWARFNumericAttribute.java

@@ -22,13 +22,15 @@ import ghidra.program.model.scalar.Scalar;
  */
 public class DWARFNumericAttribute extends Scalar implements DWARFAttributeValue {
 
+	private final boolean ambiguous;
+
 	/**
 	 * Creates a new numeric value, using 64 bits and marked as signed
 	 * 
 	 * @param value long 64 bit value
 	 */
 	public DWARFNumericAttribute(long value) {
-		this(64, value, true);
+		this(64, value, true, false);
 	}
 
 	/**
@@ -36,10 +38,41 @@ public class DWARFNumericAttribute extends Scalar implements DWARFAttributeValue
 	 * 
 	 * @param bitLength number of bits, valid values are 1..64, or 0 if value is also 0
 	 * @param value value of the scalar, any bits that are set above bitLength will be ignored
-	 * @param signed true for a signed value, false for an unsigned value. 
+	 * @param signed true for a signed value, false for an unsigned value.
 	 */
 	public DWARFNumericAttribute(int bitLength, long value, boolean signed) {
+		this(bitLength, value, signed, false);
+	}
+
+	/**
+	 * Creates a new numeric value, using the specific bitLength and value.
+	 * 
+	 * @param bitLength number of bits, valid values are 1..64, or 0 if value is also 0
+	 * @param value value of the scalar, any bits that are set above bitLength will be ignored
+	 * @param signed true for a signed value, false for an unsigned value.
+	 * @param ambiguous true for value with ambiguous signedness ({@code signed} parameter should
+	 * not be trusted), false for value where the {@code signed} parameter is known to be correct
+	 */
+	public DWARFNumericAttribute(int bitLength, long value, boolean signed, boolean ambiguous) {
 		super(bitLength, value, signed);
+		this.ambiguous = ambiguous;
+	}
+
+	/**
+	 * {@return boolean flag, if true this value's signedness is up to the user of the value,
+	 * if false the signedness was determined when the value was constructed}
+	 */
+	public boolean isAmbiguousSignedness() {
+		return ambiguous;
+	}
+
+	/**
+	 * {@return the value, forcing the signedness of ambiguous values using the specified hint} 
+	 * @param signednessHint true to default to a signed value, false to default to an 
+	 * unsigned value
+	 */
+	public long getValueWithSignednessHint(boolean signednessHint) {
+		return getValue(ambiguous ? signednessHint : isSigned());
 	}
 
 	@Override

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/dwarf4/next/DWARFDataTypeImporter.java

@@ -28,6 +28,7 @@ import org.apache.commons.lang3.StringUtils;
 
 import ghidra.app.util.DataTypeNamingUtil;
 import ghidra.app.util.bin.format.dwarf4.*;
+import ghidra.app.util.bin.format.dwarf4.attribs.DWARFNumericAttribute;
 import ghidra.app.util.bin.format.dwarf4.encoding.*;
 import ghidra.app.util.bin.format.dwarf4.expression.DWARFExpressionException;
 import ghidra.program.database.DatabaseObject;
@@ -390,6 +391,10 @@ public class DWARFDataTypeImporter {
 
 		DWARFNameInfo dni = prog.getName(diea);
 		int enumSize = (int) diea.getUnsignedLong(DWARFAttribute.DW_AT_byte_size, -1);
+		// in addition to byte_size, enums can have encoding (signed/unsigned) and a basetype, which
+		// itself might have a signed/unsigned encoding.
+		// Which attributes are present varies wildly between versions and vendors, so seems 
+		// best to just rely on the bare minimum.
 
 		if (enumSize == 0) {
 			Msg.warn(this, "Enum " + dni.getNamespacePath() + "[DWARF DIE " + diea.getHexOffset() +
@@ -403,12 +408,11 @@ public class DWARFDataTypeImporter {
 		}
 
 		Enum enumDT = new EnumDataType(dni.getParentCP(), dni.getName(), enumSize, dataTypeManager);
-		populateStubEnum(enumDT, diea);
+		populateStubEnum(enumDT, diea, false);
 
 		// Merge enums with the same name / category path if possible
 		for (DataType prevDT : dwarfDTM.forAllConflicts(dni.asDataTypePath())) {
-			if (prevDT instanceof Enum && ((Enum) prevDT).getLength() == enumDT.getLength()) {
-				Enum prevEnum = (Enum) prevDT;
+			if (prevDT instanceof Enum prevEnum && prevEnum.getLength() == enumDT.getLength()) {
 				if (isCompatEnumValues(enumDT, prevEnum)) {
 					mergeEnumValues(prevEnum, enumDT);
 					return new DWARFDataType(prevEnum, dni, diea.getOffset());
@@ -422,24 +426,29 @@ public class DWARFDataTypeImporter {
 		return new DWARFDataType(result, dni, diea.getOffset());
 	}
 
-	private void populateStubEnum(Enum enumDT, DIEAggregate diea) {
+	private void populateStubEnum(Enum enumDT, DIEAggregate diea, boolean defaultSignedness) {
+		// NOTE: gcc tends to emit values without an explicit signedness.  The caller
+		// can specify a default signedness, but this should probably always be unsigned.
 		for (DebugInfoEntry childEntry : diea.getChildren(DWARFTag.DW_TAG_enumerator)) {
 			DIEAggregate childDIEA = prog.getAggregate(childEntry);
-			String childName = childDIEA.getName();
+			String valueName = childDIEA.getName();
 
-			// TODO: DW_AT_const_value also supports block and string form types?
-			long childValue = childDIEA.getLong(DWARFAttribute.DW_AT_const_value, 0);
+			DWARFNumericAttribute enumValAttr = childDIEA
+					.getAttribute(DWARFAttribute.DW_AT_const_value, DWARFNumericAttribute.class);
+			if (enumValAttr != null) {
+				long enumVal = enumValAttr.getValueWithSignednessHint(defaultSignedness);
 
-			// NOTE: adding the same name=value pair a second time is handled correctly and ignored.
-			// Adding a second name=different_value pair generates an exception
-			try {
-				enumDT.add(childName, childValue);
-			}
-			catch (IllegalArgumentException iae) {
-				Msg.error(this,
-					"Failed to add value " + childName + "=" + childValue + "[" +
-						Long.toHexString(childValue) + "] to enum " + enumDT.getCategoryPath(),
-					iae);
+				// NOTE: adding the same name=value pair a second time is handled correctly and ignored.
+				// Adding a second name=different_value pair generates an exception
+				try {
+					enumDT.add(valueName, enumVal);
+				}
+				catch (IllegalArgumentException iae) {
+					Msg.error(this,
+						"Failed to add value %s=%d[%x] to enum %s".formatted(valueName, enumVal,
+							enumVal, enumDT.getCategoryPath()),
+						iae);
+				}
 			}
 		}
 	}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/scalar/Scalar.java

@@ -93,6 +93,16 @@ public class Scalar {
 		return signed ? getSignedValue() : value;
 	}
 
+	/**
+	 * {@return the value, using the specified signedness.  Equivalent to calling getSignedValue()
+	 * or getUnsignedValue()}
+	 * 
+	 * @param signednessOverride true for a signed value, false for an unsigned value
+	 */
+	public long getValue(boolean signednessOverride) {
+		return signednessOverride ? getSignedValue() : value;
+	}
+
 	/**
 	 * Returns the BigInteger representation of the value.
 	 *