BitFields - corrections to structure editor actions and some packing

fixes

Ghidra/Features/Base/src/main/help/help/topics/DataTypeEditors/StructureEditor.htm

@@ -169,12 +169,14 @@
     <H3>Unaligned Structures</H3>
 
     <BLOCKQUOTE>
-      <P>When a structure is unaligned, each component immediately follows the one before it. In
+      <P>When a structure is unaligned, with the exception of bitfields, each component immediately 
+      follows the one before it. In
       other words no automatic alignment or padding occurs. Unaligned structures can contain 
-      Undefined bytes. Unaligned structures should be used to position components with known data
+      Unaligned structures should be used to position components with known data
       types at specific offsets within the structure. Therefore the structure editor tries to
       prevent defined components (those other than Undefined bytes) from accidentally being moved
-      to a different offset when performing operations like drag and drop.<BR>
+      to a different offset when performing operations like drag and drop, although undefined bytes
+      may be consumed. <BR>
       </P>
 
       <DIV style="text-align: center; margin-top: 10px;">
@@ -188,9 +190,12 @@
       <P>An aligned structure is defined similar to a structure in a C header file. The data types
       are specified for each of the components, but their offsets will be automatically adjusted to
       the correct alignment based on the data type and position in the structure. A default
-      Undefined byte cannot be added to an aligned structure. The overall size of the structure is
+      "undefined" byte cannot be added to an aligned structure, although an "undefined1" component 
-      determined by the components it contains and whether or not it is packed. When you select the
+      can be and is treated like any other fixed-length datatype.   The overall size of the structure is
-      Align checkbox, the GUI displays buttons to allow an align attribute, <SPAN style=
+      determined by the components it contains and the specified pack value. The alignment and 
+      packing behavior is determined by the effective data organization as defined by each compiler
+      specification (data type archives utilize a default data organization).  When you select the
+      <B>Align</B> checkbox, the GUI displays buttons to allow an align attribute, <SPAN style=
       "font-weight: bold; font-style: italic;">align( minimum )</SPAN>, to be specified. It also
       allows a pack value, <SPAN style="font-style: italic;"><SPAN style=
       "font-weight: bold;">pack</SPAN></SPAN> <SPAN style=
@@ -256,7 +261,7 @@
 
       <UL>
         <LI><B>none</B> - The components within this structure should align themselves in the
-        default way for the compiler. The components are not being packed.<BR>
+        default way for the compiler. The components are not being packed with a reduced alignment.<BR>
         </LI>
 
         <LI>by value - The bottom button with a text field next to it allows you to specify a pack
@@ -264,7 +269,9 @@
         component.<BR>
         </LI>
       </UL>
-
+	  <P><IMG alt="Note:" src="../../shared/note.png"> The placement of bitfields may be influenced
+	  based upon the specified pack value.<BR>
+      </P>
       <P><IMG alt="Note:" src="../../shared/note.png"> The equivalent of having <I>no C #pragma
       pack attribute</I> on the structure or union is to choose <B>none</B>. The equivalent for a C
       code attribute of <I>#pragma pack()</I> without a value is to specify a <B>pack value</B> of
@@ -306,6 +313,42 @@
     
     </BLOCKQUOTE>
     
+    <H2><A name="Structure_Editor_Bitfields"></A>Bitfield Component</H2>
+
+    <P>A structure and union may define bitfield components which attempt to model 
+    bitfield definitions found within C/C++. Unlike other byte-oriented components, 
+    bitfield components have the following characteristics: 
+    <BLOCKQUOTE>
+      	<ul>
+      		<li>A bitfield datatype may not exist anywhere other than within a structure or union.</li>
+      		<li>A bitfield datatype may not be selected via the datatype chooser or tree since it
+      		requires the specification of additional attributes (bit-size, bit-offset, etc.).  The
+      		bit-size is generally appended to the base-datatype for entry and presentation purposes
+      		(e.g., char:1).  NOTE: At the API level there is no public or default constructor for the 
+      		<I>BitFieldDataType</I>.  Specific API methods exist for adding bitfields to structures and unions.</li>
+      		<li>A zero-length bitfield may be defined within a byte but its' precise bit position
+      		is controlled by endianess alone.  A zero-length bitfield has no affect within an unaligned
+      		structure and is intended for use within aligned structures where it may impart alignment
+      		affects based upon compiler conventions.</li> 
+      		<li>Inserting or moving bitfields may cause component shifts based upon allocation unit
+      		byte size and possible placement conflicts.</li>
+      		<li>The start/end byte offsets may be shared with adjacent bitfield components.</li>
+      		<li>Unoccupied bits within a partially occupied byte are not represented by any component
+      		(similar to padding bytes within aligned structures).  Using the Bitfield Viewer provides
+      		a bit-level view of a component.  (click <IMG alt="[magnifier icon]" src="images/magnifier.png"> 
+      		adjacent to bitfield datatype to view within Bitfield Viewer).</li>
+      		<li>A separate Bitfield Editor, for use with unaligned structures only, 
+      		must be used to precisley place a bitfield component.  Adding a bitfield component via the 
+      		structure table view via datatype text entry (e.g., char:1) provides only rough placement. 
+      		The BitField Editor may be displayed using the the Add Bitfield and
+      		Edit Bitfield popup actions on a selected structure component.  The datatype text entry approach
+      		must be used for all unions and aligned strctures.</li>
+      	</ul>
+    </BLOCKQUOTE>
+    
+    <P><IMG alt="Note:" src="../../shared/note.png">The use of bitfield components is not 
+    currently reflected in decompiler results or assembly markup. </P>
+    
 	<H2><A name="Structure_Editor_Flex_Array"></A>Flexible Array Component</H2>
 
     <P>A structure may be defined with a trailing flexible array component which corresponds to
@@ -321,19 +364,12 @@
     structure and will not appear within the structure when applied to Data within memory as it 
     corresponds to the memory location which immediately follows the end of the structure.</P>
     
-    <P><IMG alt="Note:" src="../../shared/note.png">The presence of a flexible array component may not 
+    <P><IMG alt="Note:" src="../../shared/note.png">The use of flexible array components is not 
-    be reflected in decompilation results.  Its primary purpose if to reflect the C source definition
+    currently reflected in decompiler results or listing reference markup. Its primary purpose 
-    of a structure with correct alignment and structure sizing.</P>
+    if to reflect the C/C++ source definition of a structure with correct alignment and structure sizing.</P>
     
-    <P><IMG alt="Note:" src="../../shared/note.png"> The equivalent of having <SPAN style=
+    <P><IMG alt="Note:" src="../../shared/note.png">While C/C++ support flexible arrays anywhere
-      "font-style: italic;">no C #pragma pack attribute</SPAN> on the structure or union is to
+    within a structure, Ghidra only supports the case where it is the last structure component.</P>
-      choose <SPAN style="font-weight: bold;">none</SPAN>. The equivalent for a C code attribute of
-      <SPAN style="font-style: italic;">#pragma pack()</SPAN> without a value is to specify a <SPAN
-      style="font-weight: bold;">pack value</SPAN> of <SPAN style="font-weight: bold;">1</SPAN>.
-      The equivalent of <SPAN style="font-style: italic;"># pragma</SPAN> <SPAN style=
-      "font-style: italic;">pack(4)</SPAN> is to specify a <SPAN style="font-weight: bold;">pack
-      value</SPAN> of 4.<BR>
-      </P>
  
       <DIV style="text-align: center;">
         <IMG alt="" src="images/StructureEditorWithFlexArray.png"><BR>

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

@@ -132,7 +132,8 @@ public class BitFieldEditorPanel extends JPanel {
 		if (adjustedOffset < 0 || adjustedOffset > composite.getLength()) {
 			return;
 		}
-		placementComponent.setAllocationOffset(adjustedOffset);
+		placementComponent.updateAllocation(placementComponent.getAllocationByteSize(),
+			adjustedOffset);
 		updateAllocationOffsetLabel();
 	}
 
@@ -272,8 +273,8 @@ public class BitFieldEditorPanel extends JPanel {
 		public void mousePressed(MouseEvent e) {
 			selectionActive = false;
 			if (e.getButton() == MouseEvent.BUTTON1 && bitOffsetInput.isEnabled()) {
+				bitSizeModel.setValue(1L); // must change size first
 				startBit = setBitFieldOffset(e.getPoint());
-				bitSizeModel.setValue(1L);
 				lastBit = startBit;
 				selectionActive = startBit >= 0;
 			}
@@ -377,10 +378,10 @@ public class BitFieldEditorPanel extends JPanel {
 		if (!BitFieldDataType.isValidBaseDataType(initialBaseDataType)) {
 			initialBaseDataType = IntegerDataType.dataType.clone(composite.getDataTypeManager());
 		}
-		placementComponent.setAllocationOffset(allocationOffset);
 		long allocationSize = useCurrentAllocation ? (Long) allocSizeModel.getValue()
 				: initialBaseDataType.getLength();
-		placementComponent.initAdd((int) allocationSize, 1, bitOffset);
+		placementComponent.updateAllocation((int) allocationSize, allocationOffset);
+		placementComponent.initAdd(1, bitOffset);
 		initControls(null, initialBaseDataType, 1);
 		enableControls(true);
 	}
@@ -398,10 +399,7 @@ public class BitFieldEditorPanel extends JPanel {
 		DataType initialBaseDataType = null;
 		int allocationSize = -1;
 		if (useExistingAllocationSize) {
-			BitFieldAllocation bitFieldAllocation = placementComponent.getBitFieldAllocation();
+			allocationSize = placementComponent.getAllocationByteSize();
-			if (bitFieldAllocation != null) {
-				allocationSize = bitFieldAllocation.getAllocationByteSize();
-			}
 		}
 		if (bitfieldDtc != null) {
 			if (!bitfieldDtc.isBitFieldComponent()) {
@@ -421,9 +419,8 @@ public class BitFieldEditorPanel extends JPanel {
 		if (allocationSize < 1) {
 			allocationSize = 4;
 		}
-		// TODO: adjust offset and allocationSize if needed
+		placementComponent.updateAllocation(allocationSize, allocationOffset);
-		placementComponent.setAllocationOffset(allocationOffset);
+		placementComponent.init(bitfieldDtc);
-		placementComponent.init(allocationSize, bitfieldDtc);
 		BitFieldAllocation bitFieldAllocation = placementComponent.getBitFieldAllocation(); // get updated instance
 		initControls(initialFieldName, initialBaseDataType, bitFieldAllocation.getBitSize());
 		enableControls(bitfieldDtc != null);
@@ -442,11 +439,11 @@ public class BitFieldEditorPanel extends JPanel {
 			fieldNameTextField.setText(initialFieldName);
 
 			// Use current placementComponent to obtain initial values
-			BitFieldAllocation bitFieldAllocation = placementComponent.getBitFieldAllocation();
+			allocSizeModel.setValue((long) placementComponent.getAllocationByteSize());
-			allocSizeModel.setValue((long) bitFieldAllocation.getAllocationByteSize());
+			int allocBits = 8 * placementComponent.getAllocationByteSize();
-			int allocBits = 8 * bitFieldAllocation.getAllocationByteSize();
 			bitSizeModel.setValue((long) initialBitSize);
 			bitOffsetModel.setMaximum((long) allocBits - 1);
+			BitFieldAllocation bitFieldAllocation = placementComponent.getBitFieldAllocation();
 			bitOffsetModel.setValue((long) bitFieldAllocation.getBitOffset());
 			updateBitSizeModel();
 
@@ -596,7 +593,8 @@ public class BitFieldEditorPanel extends JPanel {
 				bitSize = allocBits;
 				bitSizeModel.setValue(Long.valueOf(bitSize));
 			}
-			placementComponent.refresh(allocSize, bitSize, boff);
+			placementComponent.refresh(allocSize, placementComponent.getAllocationOffset(), bitSize,
+				boff);
 		}
 		finally {
 			updating = false;

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

@@ -55,7 +55,8 @@ public class BitFieldPlacementComponent extends JPanel {
 	private final Composite composite;
 	private final boolean bigEndian;
 
-	private int allocationOffset;
+	private int allocationByteOffset;
+	private int allocationByteSize;
 	private BitFieldAllocation bitFieldAllocation;
 
 	private EditMode editMode = EditMode.NONE;
@@ -127,8 +128,7 @@ public class BitFieldPlacementComponent extends JPanel {
 		}
 
 		int extraLineSpace = BYTE_SEPARATOR_THICKNESS - BIT_SEPARATOR_THICKNESS;
-		return (bitFieldAllocation.allocationByteSize * BYTE_WIDTH) + BYTE_SEPARATOR_THICKNESS +
+		return (allocationByteSize * BYTE_WIDTH) + BYTE_SEPARATOR_THICKNESS + extraLineSpace;
-			extraLineSpace;
 	}
 
 	public boolean isBigEndian() {
@@ -142,7 +142,7 @@ public class BitFieldPlacementComponent extends JPanel {
 	int getBitOffset(Point point) {
 		int bitWidthWithLine = BIT_WIDTH + BIT_SEPARATOR_THICKNESS;
 		int cellIndex = (point.x - BYTE_SEPARATOR_THICKNESS) / bitWidthWithLine;
-		return (8 * bitFieldAllocation.allocationByteSize) - cellIndex - 1;
+		return (8 * allocationByteSize) - cellIndex - 1;
 	}
 
 	private void updatePreferredSize() {
@@ -150,15 +150,23 @@ public class BitFieldPlacementComponent extends JPanel {
 		revalidate();
 	}
 
-	void refresh(int allocationByteSize, int bitSize, int bitOffset) {
+	void refresh(int bitSize, int bitOffset) {
-		bitFieldAllocation =
+		bitFieldAllocation = new BitFieldAllocation(bitSize, bitOffset);
-			new BitFieldAllocation(allocationByteSize, bitSize, bitOffset, editComponent);
 		updatePreferredSize();
 		repaint();
 	}
 
-	void setAllocationOffset(int allocationOffset) {
+	void refresh(int byteSize, int byteOffset, int bitSize, int bitOffset) {
-		this.allocationOffset = allocationOffset;
+		this.allocationByteOffset = byteOffset;
+		this.allocationByteSize = byteSize;
+		bitFieldAllocation = new BitFieldAllocation(bitSize, bitOffset);
+		updatePreferredSize();
+		repaint();
+	}
+
+	void updateAllocation(int byteSize, int byteOffset) {
+		this.allocationByteOffset = byteOffset;
+		this.allocationByteSize = byteSize;
 		if (bitFieldAllocation != null) {
 			bitFieldAllocation.refresh();
 			repaint();
@@ -166,23 +174,27 @@ public class BitFieldPlacementComponent extends JPanel {
 	}
 
 	int getAllocationOffset() {
-		return allocationOffset;
+		return allocationByteOffset;
 	}
 
-	void initAdd(int allocationByteSize, int bitSize, int bitOffset) {
+	int getAllocationByteSize() {
+		return allocationByteSize;
+	}
+
+	void initAdd(int bitSize, int bitOffset) {
 		editMode = EditMode.ADD;
 		editOrdinal = -1;
 		editComponent = null;
-		refresh(allocationByteSize, bitSize, bitOffset);
+		refresh(bitSize, bitOffset);
 	}
 
-	void init(int allocationByteSize, DataTypeComponent editDtc) {
+	void init(DataTypeComponent editDtc) {
 
 		if (editDtc == null) {
 			editMode = EditMode.NONE;
 			editOrdinal = -1;
 			this.editComponent = null;
-			refresh(allocationByteSize, 0, 0);
+			refresh(0, 0);
 			return;
 		}
 
@@ -193,10 +205,9 @@ public class BitFieldPlacementComponent extends JPanel {
 		editOrdinal = editDtc.getOrdinal();
 		this.editComponent = editDtc;
 
-		BitFieldPlacement placement = new BitFieldPlacement(editDtc, allocationByteSize);
+		BitFieldPlacement placement = new BitFieldPlacement(editDtc);
-		bitFieldAllocation =
+		bitFieldAllocation = new BitFieldAllocation(placement.rightBit - placement.leftBit + 1,
-			new BitFieldAllocation(allocationByteSize, placement.rightBit - placement.leftBit + 1,
+			(8 * allocationByteSize) - placement.rightBit - 1);
-				(8 * allocationByteSize) - placement.rightBit - 1, editDtc);
 		updatePreferredSize();
 		repaint();
 	}
@@ -205,12 +216,7 @@ public class BitFieldPlacementComponent extends JPanel {
 		if (composite instanceof Union) {
 			return false;
 		}
-		for (BitAttributes attrs : bitFieldAllocation.bitAttributes) {
+		return bitFieldAllocation.hasConflict;
-			if (attrs.hasConflict() && (attrs.isAddBitField() || attrs.isEditField())) {
-				return true;
-			}
-		}
-		return false;
 	}
 
 	/**
@@ -231,7 +237,7 @@ public class BitFieldPlacementComponent extends JPanel {
 		if (editMode != EditMode.NONE) {
 			editMode = EditMode.NONE;
 			editOrdinal = -1;
-			refresh(bitFieldAllocation.allocationByteSize, 0, 0);
+			refresh(0, 0);
 		}
 	}
 
@@ -287,15 +293,15 @@ public class BitFieldPlacementComponent extends JPanel {
 			String name = (fieldName != null && fieldName.length() != 0) ? fieldName : null;
 			DataTypeComponent dtc;
 			if (composite instanceof Union) {
-				dtc = composite.insertBitField(ordinal, bitFieldAllocation.allocationByteSize,
+				dtc = composite.insertBitField(ordinal, allocationByteSize,
 					bitFieldAllocation.bitOffset, baseDataType, bitFieldAllocation.bitSize, name,
 					null);
 			}
 			else {
 				Structure struct = (Structure) composite;
-				dtc = struct.insertBitFieldAt(allocationOffset,
+				dtc = struct.insertBitFieldAt(allocationByteOffset, allocationByteSize,
-					bitFieldAllocation.allocationByteSize, bitFieldAllocation.bitOffset,
+					bitFieldAllocation.bitOffset, baseDataType, bitFieldAllocation.bitSize, name,
-					baseDataType, bitFieldAllocation.bitSize, name, null);
+					null);
 			}
 			if (listener != null) {
 				listener.componentChanged(dtc.getOrdinal());
@@ -358,12 +364,12 @@ public class BitFieldPlacementComponent extends JPanel {
 		y += CELL_HEIGHT + BYTE_SEPARATOR_THICKNESS;
 		g.fillRect(0, y, width, BYTE_SEPARATOR_THICKNESS); // bottom line
 
-		paintByteHeader(g, BYTE_SEPARATOR_THICKNESS, allocationOffset);
+		paintByteHeader(g, BYTE_SEPARATOR_THICKNESS, allocationByteOffset);
 		paintBits((Graphics2D) g, (2 * BYTE_SEPARATOR_THICKNESS) + CELL_HEIGHT);
 	}
 
 	private void paintByteHeader(Graphics g, int y, int baseOffset) {
-		int byteSize = bitFieldAllocation.allocationByteSize;
+		int byteSize = allocationByteSize;
 		int x = BYTE_SEPARATOR_THICKNESS;
 		for (int i = 0; i < byteSize; i++) {
 			// last byte header needs to slightly wider
@@ -384,7 +390,7 @@ public class BitFieldPlacementComponent extends JPanel {
 
 		int offset = byteIndex;
 		if (!bigEndian) {
-			offset = bitFieldAllocation.allocationByteSize - byteIndex - 1;
+			offset = allocationByteSize - byteIndex - 1;
 		}
 		offset += baseOffset;
 
@@ -462,9 +468,9 @@ public class BitFieldPlacementComponent extends JPanel {
 		boolean truncateRight;
 		boolean zeroBitField;
 
-		BitFieldPlacement(DataTypeComponent component, int allocationByteSize) {
+		BitFieldPlacement(DataTypeComponent component) {
 			int startOffset = component.getOffset();
-			int offsetAdjBytes = startOffset - allocationOffset;
+			int offsetAdjBytes = startOffset - allocationByteOffset;
 			if (!bigEndian) {
 				offsetAdjBytes = allocationByteSize - offsetAdjBytes - component.getLength();
 			}
@@ -511,25 +517,20 @@ public class BitFieldPlacementComponent extends JPanel {
 
 	class BitFieldAllocation {
 
-		private final int allocationByteSize;
 		private final int bitSize;
 		private final int bitOffset;
 		private boolean hasConflict;
-		private DataTypeComponent editComponent;
 
 		// bit layout normalized to big-endian layout
 		// left-most allocation msb has array index of 0 
 		private BitAttributes[] bitAttributes;
 
-		BitFieldAllocation(int allocationByteSize, int bitSize, int bitOffset,
+		BitFieldAllocation(int bitSize, int bitOffset) {
-				DataTypeComponent editComponent) {
 			if (allocationByteSize <= 0 || (bitSize + bitOffset) > (8 * allocationByteSize)) {
-				throw new IllegalArgumentException("allocation size too small");
+				throw new AssertException("allocation size too small");
 			}
-			this.allocationByteSize = allocationByteSize;
 			this.bitSize = bitSize;
 			this.bitOffset = bitOffset;
-			this.editComponent = editComponent;
 			refresh();
 		}
 
@@ -576,7 +577,7 @@ public class BitFieldPlacementComponent extends JPanel {
 
 		private void allocateStructureMembers(Structure struct) {
 
-			int allocationEndOffset = allocationOffset + allocationByteSize - 1;
+			int allocationEndOffset = allocationByteOffset + allocationByteSize - 1;
 
 			for (DataTypeComponent component : struct.getDefinedComponents()) {
 				if (component.getOrdinal() == editOrdinal) {
@@ -584,13 +585,13 @@ public class BitFieldPlacementComponent extends JPanel {
 				}
 				int startOffset = component.getOffset();
 				int endOffset = component.getEndOffset();
-				if (endOffset < allocationOffset) {
+				if (endOffset < allocationByteOffset) {
 					continue;
 				}
 				if (startOffset > allocationEndOffset) {
 					continue;
 				}
-				BitFieldPlacement placement = new BitFieldPlacement(component, allocationByteSize);
+				BitFieldPlacement placement = new BitFieldPlacement(component);
 				if (placement.zeroBitField) {
 					allocateZeroBitField(component, placement.rightBit);
 				}
@@ -616,7 +617,7 @@ public class BitFieldPlacementComponent extends JPanel {
 						leftEndType = truncatedLeft ? EndBitType.TRUNCATED_END : EndBitType.END;
 					}
 					if (i == rightBit) {
-						rightEndType = truncatedLeft ? EndBitType.TRUNCATED_END : EndBitType.END;
+						rightEndType = truncatedRight ? EndBitType.TRUNCATED_END : EndBitType.END;
 					}
 				}
 				bitAttributes[i] =
@@ -629,10 +630,6 @@ public class BitFieldPlacementComponent extends JPanel {
 			bitAttributes[bitIndex] = new BitAttributes(dtc, bitAttributes[bitIndex]);
 		}
 
-		public int getAllocationByteSize() {
-			return allocationByteSize;
-		}
-
 		public int getBitOffset() {
 			return bitOffset;
 		}

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

@@ -135,8 +135,8 @@ public class BitFieldViewerPanel extends JPanel {
 			allocationSize = 4;
 		}
 
-		placementComponent.setAllocationOffset(allocationOffset);
+		placementComponent.updateAllocation(allocationSize, allocationOffset);
-		placementComponent.init(allocationSize, bitfieldDtc);
+		placementComponent.init(bitfieldDtc);
 		updateAllocationOffsetLabel();
 	}
 

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

@@ -457,7 +457,8 @@ public abstract class CompEditorModel extends CompositeEditorModel {
 	protected abstract DataTypeComponent insert(int rowIndex, DataType dataType, int length,
 			String name, String comment) throws InvalidDataTypeException;
 
-	protected abstract void insert(int rowIndex, DataType dataType, int length, int numCopies) throws InvalidDataTypeException;
+	protected abstract void insert(int rowIndex, DataType dataType, int length, int numCopies)
+			throws InvalidDataTypeException;
 
 	/**
 	 * Add a DataType component into to an editable structure
@@ -1297,8 +1298,10 @@ public abstract class CompEditorModel extends CompositeEditorModel {
 		DataTypeComponent comp = getComponent(rowIndex);
 		// Set the field name and comment the same as before
 		try {
+			if (comp.getFieldName() == null) {
 				comp.setFieldName(fieldName);
 			}
+		}
 		catch (DuplicateNameException exc) {
 			Msg.showError(this, null, null, null);
 		}

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

@@ -464,20 +464,27 @@ class StructureEditorModel extends CompEditorModel {
 	 *  Moves the components between the start index (inclusive) and the end
 	 *  index (inclusive) to the new index (relative to the initial component set).
 	 *
-	 * @param startRow row index of the starting component to move.
+	 * @param startIndex row index of the starting component to move.
-	 * @param endRow row index of the ending component to move.
+	 * @param endIndex row index of the ending component to move.
 	 * @return true if components are moved.
 	 */
-	private boolean shiftComponentsUp(int startRow, int endRow) {
+	private boolean shiftComponentsUp(int startIndex, int endIndex) {
 		int numComps = getNumComponents();
-		if ((startRow > endRow) || startRow <= 0 || startRow >= numComps || endRow <= 0 ||
+		if ((startIndex > endIndex) || startIndex <= 0 || startIndex >= numComps || endIndex <= 0 ||
-			endRow >= numComps) {
+			endIndex >= numComps) {
 			return false;
 		}
-		DataTypeComponent comp = getComponent(startRow - 1);
+		int len = getLength();
-		deleteComponent(startRow - 1);
+
+		DataTypeComponent comp = deleteComponentAndResidual(startIndex - 1);
+
 		try {
-			insert(endRow, comp.getDataType(), comp.getLength(), comp.getFieldName(),
+			if (!isAligned() && comp.isBitFieldComponent()) {
+				// insert residual undefined bytes before inserting unaligned bitfield
+				int lenChange = len - getLength();
+				insert(endIndex, DataType.DEFAULT, 1, lenChange);
+			}
+			insert(endIndex, comp.getDataType(), comp.getLength(), comp.getFieldName(),
 				comp.getComment());
 		}
 		catch (InvalidDataTypeException e) {
@@ -500,9 +507,16 @@ class StructureEditorModel extends CompEditorModel {
 			endIndex < 0 || endIndex >= numComponents - 1) {
 			return false;
 		}
-		DataTypeComponent comp = getComponent(endIndex + 1);
+		int len = getLength();
-		deleteComponent(endIndex + 1);
+
+		DataTypeComponent comp = deleteComponentAndResidual(endIndex + 1);
+
 		try {
+			if (!isAligned() && comp.isBitFieldComponent()) {
+				// insert residual undefined bytes before inserting unaligned bitfield
+				int lenChange = len - getLength();
+				insert(startIndex, DataType.DEFAULT, 1, lenChange);
+			}
 			insert(startIndex, comp.getDataType(), comp.getLength(), comp.getFieldName(),
 				comp.getComment());
 		}
@@ -512,6 +526,31 @@ class StructureEditorModel extends CompEditorModel {
 		return true;
 	}
 
+	private DataTypeComponent deleteComponentAndResidual(int index) {
+
+		DataTypeComponent comp = getComponent(index);
+		deleteComponent(index);
+
+		if (isAligned() || !comp.isBitFieldComponent() || index >= getNumComponents()) {
+			return comp;
+		}
+
+		// Deleting a bitfield component does not remove consumed space.
+		// This operation should remove any residual undefined components
+
+		int startOffset = comp.getOffset();
+
+		for (int i = index + comp.getLength() - 1; i >= index; --i) {
+			DataTypeComponent dtc = getComponent(i);
+			if (dtc != null && dtc.getDataType() == DataType.DEFAULT &&
+				dtc.getOffset() >= startOffset) {
+				deleteComponent(i);
+			}
+		}
+
+		return comp;
+	}
+
 	/* (non-Javadoc)
 	 * @see ghidra.app.plugin.compositeeditor.EditorModel#moveUp()
 	 */
@@ -589,12 +628,12 @@ class StructureEditorModel extends CompEditorModel {
 		FieldRange range = selection.getFieldRange(0);
 
 		DataTypeComponent comp = getComponent(range.getStart().getIndex().intValue());
-		if (comp == null) {
+		if (comp == null || comp.isFlexibleArrayComponent()) {
-			return false;
-		}
-		if (comp.isFlexibleArrayComponent()) {
 			return true;
 		}
+		if (comp.isBitFieldComponent()) {
+			return false;
+		}
 
 		DataType dt = comp.getDataType();
 		int dtLen = dt.getLength();
@@ -654,6 +693,18 @@ class StructureEditorModel extends CompEditorModel {
 		super.deleteSelectedComponents();
 	}
 
+	private boolean selectionContainsBitField() {
+		int startIx = selection.getFieldRange(0).getStart().getIndex().intValue();
+		int endIx = selection.getFieldRange(0).getEnd().getIndex().intValue();
+		for (int rowIndex = startIx; rowIndex <= endIx; rowIndex++) {
+			DataTypeComponent dtc = getComponent(rowIndex);
+			if (dtc != null && dtc.isBitFieldComponent()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * Returns whether or not the component at the selected index
 	 * is allowed to be duplicated.
@@ -662,31 +713,24 @@ class StructureEditorModel extends CompEditorModel {
 	 */
 	@Override
 	public boolean isDuplicateAllowed() {
-		boolean dupAllowed = false;
+
-		if (this.getNumSelectedComponentRows() != 1) {
+		if (!isSingleRowSelection() || this.getNumSelectedComponentRows() != 1) {
 			return false;
 		}
 
-		int rowIndex = selection.getFieldRange(0).getStart().getIndex().intValue();
-		// Get the range this index is in, if its in one.
-		FieldRange range = getSelectedRangeContaining(rowIndex);
-		boolean notInMultiLineSelection = true;
-		if ((range != null) &&
-			((range.getEnd().getIndex().intValue() - range.getStart().getIndex().intValue()) > 1)) {
-			notInMultiLineSelection = false;
-		}
-
 		// set actions based on number of items selected
-		if (notInMultiLineSelection && (rowIndex <= getNumComponents())) {
+		int rowIndex = getRow();
 		DataTypeComponent comp = getComponent(rowIndex);
 		DataType dt = comp.getDataType();
 		if (viewComposite.isInternallyAligned()) {
-				dupAllowed = true;
+			return true;
 		}
-			else {
 		if (dt.equals(DataType.DEFAULT)) {
 			return true; // Insert an undefined and push everything down.
 		}
+		if (comp.isBitFieldComponent()) {
+			return false; // unable to place unaligned bitfield in a reasonable fashion
+		}
 		// Can always duplicate at the end.
 		if (isAtEnd(rowIndex) || onlyUndefinedsUntilEnd(rowIndex + 1)) {
 			return true;
@@ -701,11 +745,9 @@ class StructureEditorModel extends CompEditorModel {
 		}
 		int undefSize = getNumUndefinedBytesAt(rowIndex + 1);
 		if (dtSize <= undefSize) {
-					dupAllowed = true;
+			return true;
 		}
-			}
+		return false;
-		}
-		return dupAllowed;
 	}
 
 	/**
@@ -985,7 +1027,16 @@ class StructureEditorModel extends CompEditorModel {
 					dtc.getComment());
 			}
 			else {
-				dtc = ((Structure) viewComposite).insert(rowIndex, dataType, length, name, comment);
+				if (isAligned() || !(dataType instanceof BitFieldDataType)) {
+					dtc = ((Structure) viewComposite).insert(rowIndex, dataType, length, name,
+						comment);
+				}
+				else {
+					BitFieldDataType bitfield = (BitFieldDataType) dataType;
+					dtc = ((Structure) viewComposite).insertBitField(rowIndex, length,
+						bitfield.getBitOffset(), bitfield.getBaseDataType(),
+						bitfield.getDeclaredBitSize(), name, comment);
+				}
 				if (rowIndex <= row) {
 					row++;
 				}
@@ -1314,21 +1365,64 @@ class StructureEditorModel extends CompEditorModel {
 		int length = 0;
 		final StructureDataType structureDataType =
 			new StructureDataType(originalCategoryPath, uniqueName, length, originalDTM);
+
+		if (isAligned()) {
+			structureDataType.setPackingValue(getPackingValue());
+		}
+
 // Get data type components to make into structure.
+		DataTypeComponent firstDtc = null;
+		DataTypeComponent lastDtc = null;
 		for (int rowIndex = minRow; rowIndex < maxRow; rowIndex++) {
 			DataTypeComponent component = getComponent(rowIndex);
+			if (rowIndex == minRow) {
+				firstDtc = component;
+			}
+			if (component == null) {
+				lastDtc = component;
+				continue;
+			}
+
+			DataType dt = component.getDataType();
 			int compLength = component.getLength();
+
 			length += compLength;
-			structureDataType.add(component.getDataType(), compLength, component.getFieldName(),
+
+			if (!isAligned() && component.isBitFieldComponent()) {
+				BitFieldDataType bitfield = (BitFieldDataType) dt;
+				structureDataType.insertBitFieldAt(component.getOffset() - firstDtc.getOffset(),
+					compLength, bitfield.getBitOffset(), bitfield.getBaseDataType(),
+					bitfield.getDeclaredBitSize(), component.getFieldName(),
 					component.getComment());
 			}
+			else {
+				structureDataType.add(dt, compLength, component.getFieldName(),
+					component.getComment());
+			}
+
+			lastDtc = component;
+		}
 		DataType addedDataType = createDataTypeInOriginalDTM(structureDataType);
 		if (viewComposite.isInternallyAligned()) {
 			deleteSelectedComponents();
 			insert(minRow, addedDataType, addedDataType.getLength());
 		}
 		else {
+			int adjustmentBytes = 0;
+			if (firstDtc != null && firstDtc.isBitFieldComponent() && minRow > 0) {
+				DataTypeComponent dtc = getComponent(minRow - 1);
+				if (dtc.getEndOffset() == firstDtc.getOffset()) {
+					++adjustmentBytes;
+				}
+			}
+			if (lastDtc != null && lastDtc.isBitFieldComponent() && maxRow < getNumComponents()) {
+				DataTypeComponent dtc = getComponent(maxRow);
+				if (dtc.getOffset() == lastDtc.getEndOffset()) {
+					++adjustmentBytes;
+				}
+			}
 			clearSelectedComponents();
+			insertMultiple(minRow, DataType.DEFAULT, 1, adjustmentBytes);
 			replace(minRow, addedDataType, addedDataType.getLength());
 		}
 	}

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

@@ -224,7 +224,12 @@ class UnionEditorModel extends CompEditorModel {
 	 */
 	@Override
 	public boolean isArrayAllowed() {
-		return isSingleRowSelection();
+		if (!isSingleRowSelection()) {
+			return false;
+		}
+		FieldRange range = selection.getFieldRange(0);
+		DataTypeComponent comp = getComponent(range.getStart().getIndex().intValue());
+		return (comp == null || !comp.isBitFieldComponent());
 	}
 
 	/**

Ghidra/Features/Base/src/test/java/ghidra/program/model/data/MSVCStructureImplBitFieldTest.java

@@ -33,6 +33,40 @@ public class MSVCStructureImplBitFieldTest extends AbstractCompositeImplBitField
 		}
 	}
 
+	@Test
+	public void testStructureBitFieldsA1() {
+		Structure struct = getStructure("A1");
+		//@formatter:off
+		CompositeTestUtils.assertExpectedComposite(this, "/A1\n" + 
+			"Aligned\n" + 
+			"Structure A1 {\n" + 
+			"   0   char[5]   5   a   \"\"\n" + 
+			"   8   int:3(0)   1   b   \"\"\n" + 
+			"   8   int:8(3)   2   c   \"\"\n" + 
+			"   9   int:8(3)   2   d   \"\"\n" + 
+			"   10   int:6(3)   2   e   \"\"\n" + 
+			"}\n" + 
+			"Size = 12   Actual Alignment = 4", struct);
+		//@formatter:on
+	}
+
+	@Test
+	public void testStructureBitFieldsA2() {
+		Structure struct = getStructure("A2");
+		//@formatter:off
+		CompositeTestUtils.assertExpectedComposite(this, "/A2\n" + 
+			"Aligned\n" + 
+			"Structure A2 {\n" + 
+			"   0   oddStruct   5   a   \"\"\n" + 
+			"   8   int:3(0)   1   b   \"\"\n" + 
+			"   8   int:8(3)   2   c   \"\"\n" + 
+			"   9   int:8(3)   2   d   \"\"\n" + 
+			"   10   int:6(3)   2   e   \"\"\n" + 
+			"}\n" + 
+			"Size = 12   Actual Alignment = 4", struct);
+		//@formatter:on
+	}
+
 	@Test
 	public void testStructureBitFieldsB1() {
 		Structure struct = getStructure("B1");

Ghidra/Features/Base/src/test/java/ghidra/program/model/data/StructureImplBigEndianBitFieldTest.java

@@ -35,6 +35,40 @@ public class StructureImplBigEndianBitFieldTest extends AbstractCompositeImplBit
 		}
 	}
 
+	@Test
+	public void testStructureBitFieldsA1() {
+		Structure struct = getStructure("A1");
+		//@formatter:off
+		CompositeTestUtils.assertExpectedComposite(this, "/A1\n" + 
+			"Aligned\n" + 
+			"Structure A1 {\n" + 
+			"   0   char[5]   5   a   \"\"\n" + 
+			"   5   int:3(5)   1   b   \"\"\n" + 
+			"   5   int:8(5)   2   c   \"\"\n" + 
+			"   6   int:8(5)   2   d   \"\"\n" + 
+			"   8   int:6(2)   1   e   \"\"\n" + 
+			"}\n" + 
+			"Size = 12   Actual Alignment = 4", struct);
+		//@formatter:on
+	}
+
+	@Test
+	public void testStructureBitFieldsA2() {
+		Structure struct = getStructure("A2");
+		//@formatter:off
+		CompositeTestUtils.assertExpectedComposite(this, "/A2\n" + 
+			"Aligned\n" + 
+			"Structure A2 {\n" + 
+			"   0   oddStruct   5   a   \"\"\n" + 
+			"   5   int:3(5)   1   b   \"\"\n" + 
+			"   5   int:8(5)   2   c   \"\"\n" + 
+			"   6   int:8(5)   2   d   \"\"\n" + 
+			"   8   int:6(2)   1   e   \"\"\n" + 
+			"}\n" + 
+			"Size = 12   Actual Alignment = 4", struct);
+		//@formatter:on
+	}
+
 	@Test
 	public void testStructureBitFieldsB1() {
 		Structure struct = getStructure("B1");

Ghidra/Features/Base/src/test/java/ghidra/program/model/data/StructureImplLittleEndianBitFieldTest.java

@@ -73,6 +73,40 @@ public class StructureImplLittleEndianBitFieldTest extends AbstractCompositeImpl
 		}
 	}
 
+	@Test
+	public void testStructureBitFieldsA1() {
+		Structure struct = getStructure("A1");
+		//@formatter:off
+		CompositeTestUtils.assertExpectedComposite(this, "/A1\n" + 
+			"Aligned\n" + 
+			"Structure A1 {\n" + 
+			"   0   char[5]   5   a   \"\"\n" + 
+			"   5   int:3(0)   1   b   \"\"\n" + 
+			"   5   int:8(3)   2   c   \"\"\n" + 
+			"   6   int:8(3)   2   d   \"\"\n" + 
+			"   8   int:6(0)   1   e   \"\"\n" + 
+			"}\n" + 
+			"Size = 12   Actual Alignment = 4", struct);
+		//@formatter:on
+	}
+
+	@Test
+	public void testStructureBitFieldsA2() {
+		Structure struct = getStructure("A2");
+		//@formatter:off
+		CompositeTestUtils.assertExpectedComposite(this, "/A2\n" + 
+			"Aligned\n" + 
+			"Structure A2 {\n" + 
+			"   0   oddStruct   5   a   \"\"\n" + 
+			"   5   int:3(0)   1   b   \"\"\n" + 
+			"   5   int:8(3)   2   c   \"\"\n" + 
+			"   6   int:8(3)   2   d   \"\"\n" + 
+			"   8   int:6(0)   1   e   \"\"\n" + 
+			"}\n" + 
+			"Size = 12   Actual Alignment = 4", struct);
+		//@formatter:on
+	}
+
 	@Test
 	public void testStructureBitFieldsB1() {
 		Structure struct = getStructure("B1");

Ghidra/Features/Base/src/test/resources/ghidra/app/util/cparser/bitfields.c

@@ -16,6 +16,10 @@
 #include <stdio.h>
 #include "bitfields.h"
 
+struct A1 A1 = { { 5, 0xa, 5, 0xa, 5}, 3, 0xAA, 0x55, 0x3f };
+
+struct A2 A2 = { { 5, 0x050a050a }, 3, 0xAA, 0x55, 0x3f };
+
 
 struct B1 B1 = { 5, 0x2A, -1, 0xA };
 

Ghidra/Features/Base/src/test/resources/ghidra/app/util/cparser/bitfields.h

@@ -15,6 +15,31 @@
  */
 // Verify bitfield grouping and alignment without zero-length bitfields
 
+#pragma pack(1)
+
+struct oddStruct {
+        char x;
+        int  y;
+};
+
+#pragma pack()
+
+struct A1 {
+        char a[5]; // test for alignment overlap (gcc)
+        int b:3;
+        int c:8;
+        int d:8;
+        int e:6;
+};
+
+struct A2 {
+        oddStruct a; // test for check alignment overlap (gcc)
+        int b:3;
+        int c:8;
+        int d:8;
+        int e:6;
+};
+
 struct B1 {
 	char a;
 	unsigned short b:6; // gcc groups with previous non-bitfield
@@ -449,5 +474,3 @@ union U1p2 {
 
 #pragma pack()
 
-
-

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

@@ -481,26 +481,7 @@ class StructureDB extends CompositeDB implements Structure {
 		if (isInternallyAligned()) {
 			return;
 		}
-		int shiftAmount = dtc.getLength();
+		int shiftAmount = dtc.isBitFieldComponent() ? 0 : dtc.getLength();
-		if (dtc.isBitFieldComponent()) {
-			// Must handle potential overlap with adjacent components
-			// NOTE: existing bitfields will not overlap by more than one byte
-			int minOffset = dtc.getOffset();
-			int maxOffset = dtc.getEndOffset();
-			if (index > 0) {
-				DataTypeComponentDB previousDtc = components.get(index - 1);
-				if (previousDtc.getEndOffset() == dtc.getOffset()) {
-					++minOffset;
-				}
-			}
-			if (minOffset <= maxOffset && index < components.size()) {
-				DataTypeComponentDB nextDtc = components.get(index);
-				if (nextDtc.getOffset() == dtc.getOffset()) {
-					--maxOffset;
-				}
-			}
-			shiftAmount = maxOffset - minOffset + 1;
-		}
 		shiftOffsets(index, -1, -shiftAmount);
 	}
 

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

@@ -274,28 +274,10 @@ public class AlignedStructurePacker {
 			updateComponent(lastComponent, ordinal, groupOffset, 1, minimumAlignment);
 		}
 
-		private boolean canPack(DataType dataType) {
-			if (dataType instanceof TypeDef) {
-				dataType = ((TypeDef) dataType).getBaseDataType();
-			}
-			if (dataType instanceof BitFieldDataType) {
-				return true;
-			}
-			if (dataType instanceof AbstractIntegerDataType) {
-				return true;
-			}
-			if (dataType instanceof Enum) {
-				return true;
-			}
-			return false;
-		}
-
 		private boolean packComponent(InternalDataTypeComponent dataTypeComponent) {
 
-			if (lastComponent == null || dataTypeComponent.isZeroBitFieldComponent() ||
+			if (lastComponent == null || dataTypeComponent.isZeroBitFieldComponent()) {
-				!canPack(lastComponent.getDataType()) ||
+				return false;
-				!canPack(dataTypeComponent.getDataType())) {
-				return false; // can't pack incompatible types - start new group
 			}
 
 			if (dataTypeComponent.isBitFieldComponent()) {

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

@@ -262,8 +262,8 @@ public interface Composite extends DataType {
 
 	/**
 	 * Deletes the component at the given ordinal position.
-	 * <BR>Note: For an aligned structure the delete will have no effect if the
+	 * <BR>Note: Removal of bitfields from an unaligned structure will 
-	 * ordinal position is a component that provides alignment padding.
+	 * not shift other components with vacated bytes reverting to undefined.
 	 * @param ordinal the ordinal of the component to be deleted.
 	 * @throws ArrayIndexOutOfBoundsException if component ordinal is out of bounds
 	 */
@@ -271,9 +271,7 @@ public interface Composite extends DataType {
 
 	/**
 	 * Deletes the components at the given ordinal positions.
-	 * <BR>Note 1: For an aligned structure the delete will have no effect if the
+	 * <BR>Note: Removal of bitfields from an unaligned structure will 
-	 * ordinal position is a component that provides alignment padding.
-	 * <BR>Note 2: Removal of bitfields from an unaligned structure will 
 	 * not shift other components with vacated bytes reverting to undefined.
 	 * @param ordinals the ordinals of the component to be deleted.
 	 * @throws ArrayIndexOutOfBoundsException if any specified component ordinal is out of bounds

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

@@ -193,26 +193,7 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 		if (isInternallyAligned()) {
 			return;
 		}
-		int shiftAmount = dtc.getLength();
+		int shiftAmount = dtc.isBitFieldComponent() ? 0 : dtc.getLength();
-		if (dtc.isBitFieldComponent()) {
-			// Must handle potential overlap with adjacent components
-			// NOTE: existing bitfields will not overlap by more than one byte
-			int minOffset = dtc.getOffset();
-			int maxOffset = dtc.getEndOffset();
-			if (index > 0) {
-				DataTypeComponentImpl previousDtc = components.get(index - 1);
-				if (previousDtc.getEndOffset() == dtc.getOffset()) {
-					++minOffset;
-				}
-			}
-			if (minOffset <= maxOffset && index < components.size()) {
-				DataTypeComponentImpl nextDtc = components.get(index);
-				if (nextDtc.getOffset() == dtc.getOffset()) {
-					--maxOffset;
-				}
-			}
-			shiftAmount = maxOffset - minOffset + 1;
-		}
 		shiftOffsets(index, -1, -shiftAmount);
 	}
 

Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/DataTypeEditorsScreenShots.java

@@ -295,7 +295,8 @@ public class DataTypeEditorsScreenShots extends GhidraScreenShotGenerator {
 		waitForBusyTool(tool);
 	}
 
-	private void createAlignedDetailedStructure(long address, boolean includeFlexArray) {
+	private void createAlignedDetailedStructure(long address,
+			boolean includeBitFieldsAndFlexArray) {
 
 		goToListing(address);
 
@@ -303,15 +304,25 @@ public class DataTypeEditorsScreenShots extends GhidraScreenShotGenerator {
 		struct.add(new ByteDataType(), "myByteElement", "alignment 1");
 		struct.add(new ByteDataType(), "", "This is my undefined element");
 		struct.add(new WordDataType(), "myWordElement", "alignment 2");
+		if (includeBitFieldsAndFlexArray) {
+			try {
+				struct.addBitField(ByteDataType.dataType, 1, "myBitField1", "alignment 1");
+				struct.addBitField(ByteDataType.dataType, 2, "myBitField2", "alignment 1");
+				struct.addBitField(ByteDataType.dataType, 3, "myBitField3", "alignment 1");
+			}
+			catch (InvalidDataTypeException e) {
+				failWithException("Unexpected Error", e);
+			}
+		}
 		struct.add(new ByteDataType(), "myByteElement2", "alignment 1");
 		struct.add(new DWordDataType(), "myDWordElement", "alignment 4");
-		if (includeFlexArray) {
+		if (includeBitFieldsAndFlexArray) {
 			struct.setFlexibleArrayComponent(CharDataType.dataType, "flex",
 				"unsized flexible array");
 		}
 		struct.clearComponent(1);
-		struct.setDescription(
+		struct.setDescription("Members internally aligned " +
-			"Members internally aligned " + (includeFlexArray ? "with a flexible char array"
+			(includeBitFieldsAndFlexArray ? "with bitfields and a flexible char array"
 					: "according to their alignment size") +
 			". ");
 		struct.setInternallyAligned(true);