BitFields - added a few tests and improved javadoc

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompEditorPanel.java

@@ -210,6 +210,7 @@ public class CompEditorPanel extends CompositeEditorPanel {
 				DataTypeComponent dtc = attrs.getDataTypeComponent(false);
 				if (dtc != null) {
 					model.setSelection(new int[] { dtc.getOrdinal() });
+					table.scrollToSelectedRow();
 				}
 				else {
 					model.setSelection(new FieldSelection());

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/AbstractStructureEditorTest.java

@@ -35,6 +35,7 @@ public abstract class AbstractStructureEditorTest extends AbstractEditorTest {
 	DuplicateAction duplicateAction;
 	DuplicateMultipleAction duplicateMultipleAction;
 	EditComponentAction editComponentAction;
+	EditBitFieldAction editBitFieldAction;
 	EditFieldAction editFieldAction;
 	MoveDownAction moveDownAction;
 	MoveUpAction moveUpAction;
@@ -99,6 +100,7 @@ public abstract class AbstractStructureEditorTest extends AbstractEditorTest {
 		duplicateAction = null;
 		duplicateMultipleAction = null;
 		editComponentAction = null;
+		editBitFieldAction = null;
 		editFieldAction = null;
 		moveDownAction = null;
 		moveUpAction = null;
@@ -142,6 +144,9 @@ public abstract class AbstractStructureEditorTest extends AbstractEditorTest {
 			else if (action instanceof EditComponentAction) {
 				editComponentAction = (EditComponentAction) action;
 			}
+			else if (action instanceof EditBitFieldAction) {
+				editBitFieldAction = (EditBitFieldAction) action;
+			}
 			else if (action instanceof EditFieldAction) {
 				editFieldAction = (EditFieldAction) action;
 			}

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedActions3Test.java

@@ -133,6 +133,37 @@ public class StructureEditorUnlockedActions3Test
 		assertNotEditingField();
 	}
 
+	@Test
+	public void testEditFieldSetBitfieldDataType() throws Exception {
+		init(complexStructure, pgmTestCat);
+
+		DataTypeComponent dtc = model.getComponent(3);
+		assertNotNull(dtc);
+		assertTrue(!dtc.isBitFieldComponent());
+
+		setSelection(new int[] { 3 });
+		assertTrue(!model.isEditingField());
+		invoke(editFieldAction);
+		JTable table = getTable();
+		Container component = (Container) table.getEditorComponent();
+		assertTrue(model.isEditingField());
+		assertEquals(3, model.getRow());
+		assertEquals(model.getDataTypeColumn(), model.getColumn());
+
+		JTextField textField = findComponent(component, JTextField.class);
+		triggerText(textField, "char:2\n");
+
+		waitForSwing();
+
+		assertTrue(!model.isEditingField());
+		assertEquals(3, model.getRow());
+		assertNotEditingField();
+
+		dtc = model.getComponent(3);
+		assertNotNull(dtc);
+		assertTrue(dtc.isBitFieldComponent());
+	}
+
 	@Test
 	public void testFavoritesFixedOnBlankLine() {
 		init(emptyStructure, pgmTestCat);

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorUnlockedEnablementTest.java

@@ -52,6 +52,7 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		runSwing(() -> {
 			installProvider(new StructureEditorProvider(plugin, structDt, false));
 			model = provider.getModel();
+			structureModel = (StructureEditorModel) model;
 //				model.setLocked(false);
 		});
 //		assertTrue(!model.isLocked());
@@ -83,8 +84,8 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		for (CompositeEditorTableAction action : actions) {
 			if ((action instanceof FavoritesAction) || (action instanceof CycleGroupAction) ||
 				(action instanceof EditFieldAction) || (action instanceof InsertUndefinedAction) ||
-				(action instanceof PointerAction) || (action instanceof HexNumbersAction) ||
+				(action instanceof AddBitFieldAction) || (action instanceof PointerAction) ||
-				(action instanceof ApplyAction)) {
+				(action instanceof HexNumbersAction) || (action instanceof ApplyAction)) {
 				checkEnablement(action, true);
 			}
 			else {
@@ -117,7 +118,8 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		for (CompositeEditorTableAction action : actions) {
 			if ((action instanceof FavoritesAction) || (action instanceof CycleGroupAction) ||
 				(action instanceof EditFieldAction) || (action instanceof InsertUndefinedAction) ||
-				(action instanceof PointerAction) || (action instanceof HexNumbersAction)) {
+				(action instanceof AddBitFieldAction) || (action instanceof PointerAction) ||
+				(action instanceof HexNumbersAction)) {
 				checkEnablement(action, true);
 			}
 			else {
@@ -144,7 +146,8 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		for (CompositeEditorTableAction action : actions) {
 			if ((action instanceof EditFieldAction) ||
 				(action instanceof ShowComponentPathAction) ||
-				(action instanceof InsertUndefinedAction) || (action instanceof MoveDownAction) ||
+				(action instanceof InsertUndefinedAction) ||
+				(action instanceof AddBitFieldAction) || (action instanceof MoveDownAction) ||
 				(action instanceof ClearAction) || (action instanceof DuplicateAction) ||
 				(action instanceof DuplicateMultipleAction) || (action instanceof DeleteAction) ||
 				(action instanceof ArrayAction) || (action instanceof PointerAction) ||
@@ -183,7 +186,8 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		for (CompositeEditorTableAction action : actions) {
 			if ((action instanceof EditFieldAction) ||
 				(action instanceof ShowComponentPathAction) ||
-				(action instanceof InsertUndefinedAction) || (action instanceof MoveDownAction) ||
+				(action instanceof InsertUndefinedAction) ||
+				(action instanceof AddBitFieldAction) || (action instanceof MoveDownAction) ||
 				(action instanceof MoveUpAction) || (action instanceof ClearAction) ||
 				(action instanceof DeleteAction) || (action instanceof ArrayAction) ||
 				(action instanceof PointerAction) || (action instanceof HexNumbersAction) ||
@@ -221,7 +225,8 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		for (CompositeEditorTableAction action : actions) {
 			if ((action instanceof EditFieldAction) ||
 				(action instanceof ShowComponentPathAction) ||
-				(action instanceof InsertUndefinedAction) || (action instanceof MoveUpAction) ||
+				(action instanceof InsertUndefinedAction) ||
+				(action instanceof AddBitFieldAction) || (action instanceof MoveUpAction) ||
 				(action instanceof ClearAction) || (action instanceof DuplicateAction) ||
 				(action instanceof DuplicateMultipleAction) || (action instanceof DeleteAction) ||
 				(action instanceof ArrayAction) || (action instanceof PointerAction) ||
@@ -264,7 +269,8 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 		for (CompositeEditorTableAction action : actions) {
 			if ((action instanceof FavoritesAction) || (action instanceof CycleGroupAction) ||
 				(action instanceof EditFieldAction) || (action instanceof InsertUndefinedAction) ||
-				(action instanceof PointerAction) || (action instanceof HexNumbersAction)) {
+				(action instanceof AddBitFieldAction) || (action instanceof PointerAction) ||
+				(action instanceof HexNumbersAction)) {
 				checkEnablement(action, true);
 			}
 			else {
@@ -274,36 +280,72 @@ public class StructureEditorUnlockedEnablementTest extends AbstractStructureEdit
 	}
 
 	@Test
-	public void testEditComponentEnablement() {
+	public void testEditComponentEnablement()
+			throws ArrayIndexOutOfBoundsException, InvalidDataTypeException {
 		init(complexStructure, pgmBbCat);
 
+		((Structure) structureModel.viewComposite).insertBitField(2, 1, 4, CharDataType.dataType, 2,
+			"bf1", null);
+
+		setSelection(new int[] { 2 });
+		assertEquals("char:2", getDataType(2).getDisplayName());
+		assertTrue(!editComponentAction.isEnabled());
+
+		setSelection(new int[] { 3 });
+		assertEquals("word", getDataType(3).getDisplayName());
+		assertTrue(!editComponentAction.isEnabled());
+
+		setSelection(new int[] { 5 });
+		assertEquals("simpleUnion", getDataType(5).getDisplayName());
+		assertTrue(editComponentAction.isEnabled());
+
+		setSelection(new int[] { 7 });
+		assertEquals("simpleStructure *", getDataType(7).getDisplayName());
+		assertTrue(editComponentAction.isEnabled());
+
+		setSelection(new int[] { 15 });
+		assertEquals("byte[7]", getDataType(15).getDisplayName());
+		assertTrue(!editComponentAction.isEnabled());
+
+		setSelection(new int[] { 20 });
+		assertEquals("simpleStructureTypedef", getDataType(20).getDisplayName());
+		assertTrue(editComponentAction.isEnabled());
+
+		setSelection(new int[] { 22 });
+		assertEquals("simpleStructure", getDataType(22).getDisplayName());
+		assertTrue(editComponentAction.isEnabled());
+
+		setSelection(new int[] { 24 });
+		assertEquals(24, model.getNumComponents());
+		assertTrue(!editComponentAction.isEnabled());
+	}
+
+	@Test
+	public void testEditBitfieldEnablement()
+			throws ArrayIndexOutOfBoundsException, InvalidDataTypeException {
+		init(complexStructure, pgmBbCat);
+
+		((Structure) structureModel.viewComposite).insertBitField(2, 1, 4, CharDataType.dataType, 2,
+			"bf1", null);
+
+		setSelection(new int[] { 2 });
+		assertEquals("char:2", getDataType(2).getDisplayName());
+		assertTrue(editBitFieldAction.isEnabled());
+
+		setSelection(new int[] { 3 });
+		assertEquals("word", getDataType(3).getDisplayName());
+		assertTrue(!editBitFieldAction.isEnabled());
+
+		structureModel.setAligned(true);
+
+		// Edit Bitfield action not enabled for Aligned mode
+		setSelection(new int[] { 1 });
+		assertEquals("char:2", getDataType(1).getDisplayName());
+		assertTrue(!editBitFieldAction.isEnabled());
+
 		setSelection(new int[] { 2 });
 		assertEquals("word", getDataType(2).getDisplayName());
-		assertTrue(!editComponentAction.isEnabled());
+		assertTrue(!editBitFieldAction.isEnabled());
-
-		setSelection(new int[] { 4 });
-		assertEquals("simpleUnion", getDataType(4).getDisplayName());
-		assertTrue(editComponentAction.isEnabled());
-
-		setSelection(new int[] { 6 });
-		assertEquals("simpleStructure *", getDataType(6).getDisplayName());
-		assertTrue(editComponentAction.isEnabled());
-
-		setSelection(new int[] { 14 });
-		assertEquals("byte[7]", getDataType(14).getDisplayName());
-		assertTrue(!editComponentAction.isEnabled());
-
-		setSelection(new int[] { 19 });
-		assertEquals("simpleStructureTypedef", getDataType(19).getDisplayName());
-		assertTrue(editComponentAction.isEnabled());
-
-		setSelection(new int[] { 21 });
-		assertEquals("simpleStructure", getDataType(21).getDisplayName());
-		assertTrue(editComponentAction.isEnabled());
-
-		setSelection(new int[] { 23 });
-		assertEquals(23, model.getNumComponents());
-		assertTrue(!editComponentAction.isEnabled());
 	}
 
 	@Test

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

@@ -69,7 +69,7 @@ abstract class CompositeDB extends DataTypeDB implements Composite {
 	/**
 	 * Get the preferred length for a new component.  For Unions and internally aligned
 	 * structures the preferred component length for a fixed-length dataType will be the 
-	 * length of that dataType.  Otherwise the length returned will be no larger then the
+	 * length of that dataType.  Otherwise the length returned will be no larger than the
 	 * specified length.
 	 * @param dataType new component datatype
 	 * @param length constrained length or -1 to force use of dataType size.  Dynamic types

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

@@ -25,13 +25,15 @@ import ghidra.util.exception.AssertException;
 
 /**
  * <code>BitFieldDataType</code> provides a means of defining a minimally sized bit-field
- * for use within data structures.  The length (i.e., storage size) of this datatype
+ * for use within data structures.  The length (i.e., storage size) of this bitfield datatype is
- * less than or equal to the base datatype size and will always be the smallest possible size
+ * the minimum number of bytes required to contain the bitfield at its specified offset.
- * to contain the bitfield and offset within the least significant byte containing the 
+ * The effective bit-size of a bitfield will be limited by the size of the base
- * lsb of the bitfield.
+ * datatype whose size may be controlled by its' associated datatype manager and data organization
+ * (e.g., {@link IntegerDataType}). 
  * <p>
- * NOTE: This datatype implementation is intended for internal use only.  Creating bitfields
+ * NOTE: Instantiation of this datatype implementation is intended for internal use only.  
- * must be accomplished directly via a Structure or Union instance API method.
+ * Creating and manipulating bitfields should be accomplished directly via Structure or Union 
+ * bitfield methods.
  */
 public class BitFieldDataType extends AbstractDataType {
 

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

@@ -62,7 +62,7 @@ public abstract class CompositeDataTypeImpl extends GenericDataType implements C
 	/**
 	 * Get the preferred length for a new component.  For Unions and internally aligned
 	 * structures the preferred component length for a fixed-length dataType will be the 
-	 * length of that dataType.  Otherwise the length returned will be no larger then the
+	 * length of that dataType.  Otherwise the length returned will be no larger than the
 	 * specified length.
 	 * @param dataType new component datatype
 	 * @param length constrained length or -1 to force use of dataType size.  Dynamic types

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

@@ -347,13 +347,30 @@ public interface Structure extends Composite {
 	public void pack(int maxAlignment) throws InvalidInputException;
 
 	/**
-	 * <code>BitOffsetComparator</code> provides ability to compare an Integer bit offset
+	 * <code>BitOffsetComparator</code> provides ability to compare an normalized bit offset
-	 * with a DataTypeComponent object.  The offset will be consider equal (0) if
+	 * (see {@link #getNormalizedBitfieldOffset(int, int, int, int, boolean)} with a
-	 * the component contains the offset.  Bit offsets for this comparator number the 
+	 * {@link DataTypeComponent} object.  The offset will be considered equal (0) if the component 
-	 * msb of the first byte in the structure as 0 and the lsb of the last byte as
+	 * contains the offset.  A normalized component bit numbering is used to establish the footprint
-	 * (8 * structLength) - 1.  Due to the storage order of bitfields within little-endian
+	 * of each component with an ordinal-based ordering (assumes specific LE/BE allocation rules).  
-	 * organization a normalized bit-offset must be used which corresponds to the msb getting
+	 * Bit offsets for this comparator number the first allocated bit of the structure as 0 and the
-	 * filled first (in reality little-endian fills the lsb first). 
+	 * last allocated bit of the structure as (8 * structLength) - 1.  For big-endian bitfields
+	 * the msb of the bitfield will be assigned the lower bit-number (assumes msb-allocated-first), 
+	 * while little-endian will perform similar numbering assuming byte-swap and bit-reversal of the 
+	 * storage unit (assumes lsb-allocated-first).  Both cases result in a normalized view where 
+	 * normalized bit-0 is allocated first.
+	 * 
+	 * Example:
+	 *    
+	 * Big-Endian (normalized view):
+	 *    | . . . . . . . 7 | 8 9 . . . . . . |
+	 *    |<--------------------------------->| storage-size (2-bytes)
+	 *                        |<--------------| bit-offset (6, lsb position within storage unit)
+	 *                    |<--->|               bit-size (3)
+	 *                        
+	 * Little-Endian (normalized view, w/ storage byte-swap and bit-reversal):
+	 *    | . . . . . . 6 7 | 8 . . . . . . . |
+	 *    |------------>|                       bit-offset (6, lsb position within storage unit)
+	 *                  |<--->|                 bit-size (3)
 	 */
 	public static class BitOffsetComparator implements Comparator<Object> {
 
@@ -393,8 +410,12 @@ public interface Structure extends Composite {
 		}
 
 		/**
-		 * Compute the normalize bit offset of a bitfield relative to the start of a structure where 
+		 * Compute the normalized bit offset of a bitfield relative to the start of a structure.
-		 * the lsb of a bit-field is assigned an offset smaller than the msb.
+		 * 
+		 * NOTE: This implementation currently relies only on endianess to dictate bit allocation
+		 * ordering.  If future support is added for alternate bitfield packing, this implementation will
+		 * require modification.
+		 * 
 		 * @param byteOffset byte offset within structure of storage unit
 		 * @param storageSize storage unit size (i.e., component length)
 		 * @param effectiveBitSize size of bitfield in bits

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

@@ -16,7 +16,11 @@
 package ghidra.program.model.data;
 
 /**
- * The Union marker interface
+ * The union interface.
+ * <p>
+ * NOTE: The use of bitfields within all unions assumes a default packing where bit allocation 
+ * always starts with byte-0 of the union.  Bit allocation order is dictated by data organization
+ * endianess (byte-0 msb allocated first for big-endian, while byte-0 lsb allocated first for little-endian).
  */
 public interface Union extends Composite {