Merge remote-tracking branch

'origin/GP-5037_ghidra1_PDBCompositePadding--SQUASHED' (Closes #1030)
This commit is contained in:
ghidra1 2024-11-13 16:05:02 -05:00
commit d5f4d3b9bc
3 changed files with 336 additions and 74 deletions

View File

@ -4,9 +4,9 @@
* 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.
@ -84,7 +84,7 @@ public class BitFieldGroupCompositeMember extends CompositeMember {
if (list.isEmpty()) {
return 0;
}
return list.get(0).getLength();
return list.get(0).getLength(); // length is base-datatype-length
}
@Override
@ -136,6 +136,7 @@ public class BitFieldGroupCompositeMember extends CompositeMember {
if (!member.isSingleBitFieldMember()) {
throw new IllegalArgumentException("expected single bit-field member");
}
// Check verifies that bitfield base-datatype-length is the same as the group
if (!list.isEmpty() &&
(member.getOffset() != getOffset() || member.getLength() != getLength())) {
throw new IllegalArgumentException(

View File

@ -4,9 +4,9 @@
* 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.
@ -16,6 +16,7 @@
package ghidra.app.util.bin.format.pdb;
import java.util.*;
import java.util.Map.Entry;
import java.util.function.Consumer;
import ghidra.program.model.data.*;
@ -39,6 +40,8 @@ public class DefaultCompositeMember extends CompositeMember {
private static int MAX_CONSTRUCTION_DEPTH = 20;
private static final String PADDING_COMPONENT_NAME = "_padding_";
private DataTypeManager dataTypeManager;
private Consumer<String> errorConsumer;
@ -58,8 +61,10 @@ public class DefaultCompositeMember extends CompositeMember {
private BitFieldGroupCompositeMember bitFieldGroup;
// Structure container data
private Map<Integer, CompositeMember> structureMemberOffsetMap;
private TreeMap<Integer, CompositeMember> structureMemberOffsetMap;
private RangeMap structureMemberRangeMap;
private int largestPrimitiveSize;
private boolean hasPadding = false;
// Union container data
private List<CompositeMember> unionMemberList;
@ -135,6 +140,8 @@ public class DefaultCompositeMember extends CompositeMember {
dataTypeManager = member.dataTypeManager;
structureMemberOffsetMap = member.structureMemberOffsetMap;
structureMemberRangeMap = member.structureMemberRangeMap;
// allow padding size to use pointer-size and smaller by default
largestPrimitiveSize = member.dataTypeManager.getDataOrganization().getPointerSize();
unionMemberList = member.unionMemberList;
}
@ -148,7 +155,7 @@ public class DefaultCompositeMember extends CompositeMember {
*/
private DefaultCompositeMember(int componentOffset, DataType baseDataType, int bitSize,
int bitOffsetWithinBaseType) throws InvalidDataTypeException {
memberName = "padding";
memberName = PADDING_COMPONENT_NAME;
memberDataType = new PdbBitField(baseDataType, bitSize, bitOffsetWithinBaseType);
memberIsZeroLengthArray = false;
memberOffset = componentOffset;
@ -314,6 +321,8 @@ public class DefaultCompositeMember extends CompositeMember {
*/
private void alignComposite(int preferredSize) {
Composite composite = (Composite) memberDataType;
// don't attempt to align empty composite - don't complain
if (isStructureContainer()) {
if (structureMemberOffsetMap.isEmpty()) {
@ -324,7 +333,6 @@ public class DefaultCompositeMember extends CompositeMember {
return;
}
Composite composite = (Composite) memberDataType;
Composite copy = (Composite) composite.copy(dataTypeManager);
int pack = 0;
@ -333,20 +341,34 @@ public class DefaultCompositeMember extends CompositeMember {
boolean alignOK = isGoodAlignment(copy, preferredSize);
if (alignOK) {
composite.setToDefaultPacking();
if (hasPadding) {
removeUnnecessaryPadding(composite);
}
}
else {
copy.setToMachineAligned();
alignOK = isGoodAlignment(copy, preferredSize);
if (alignOK) {
composite.setToDefaultPacking();
composite.setToMachineAligned();
}
else {
pack = 1;
copy.setExplicitPackingValue(pack);
if (preferredSize > 0 && copy.getLength() != preferredSize) {
copy.setToMachineAligned(); // will only impact structure length
alignOK = isGoodAlignment(copy, preferredSize);
if (alignOK) {
composite.setExplicitPackingValue(pack);
composite.setToDefaultPacking();
composite.setToMachineAligned();
if (hasPadding) {
removeUnnecessaryPadding(composite);
}
}
else {
copy.setToDefaultAligned(); // restore default alignment
}
}
if (!alignOK) {
removeAllPadding(composite); // includes bit-field padding
if (!hasPadding) {
pack = 1;
copy.setExplicitPackingValue(pack);
alignOK = isGoodAlignment(copy, preferredSize);
if (alignOK) {
composite.setExplicitPackingValue(pack);
}
}
}
}
@ -357,6 +379,64 @@ public class DefaultCompositeMember extends CompositeMember {
}
}
private void removeUnnecessaryPadding(Composite packedComposite) {
if (!packedComposite.isPackingEnabled()) {
throw new IllegalArgumentException("composite must have packing enabled");
}
if (!(packedComposite instanceof Structure struct)) {
return;
}
int preferredLength = packedComposite.getLength();
DataTypeComponent[] definedComponents = struct.getDefinedComponents();
int lastIndex = definedComponents.length - 1;
for (int i = 0; i < definedComponents.length; i++) {
DataTypeComponent dtc = definedComponents[i];
if (!isPaddingComponent(dtc, true)) {
continue; // leave bitfield padding intact
}
int nextComponentOffset = -1;
if (i < lastIndex) {
nextComponentOffset = definedComponents[i + 1].getOffset();
}
int ordinal = dtc.getOrdinal();
// experiment with padding removal and restore if removal impacts structure
struct.delete(ordinal);
if (struct.getLength() != preferredLength || (nextComponentOffset > 0 &&
nextComponentOffset != definedComponents[i + 1].getOffset())) {
// restore padding component
struct.insert(ordinal, dtc.getDataType(), -1, PADDING_COMPONENT_NAME, null);
}
}
}
private void removeAllPadding(Composite composite) {
if (!(composite instanceof Structure struct)) {
return;
}
boolean doDelete = composite.isPackingEnabled();
DataTypeComponent[] definedComponents = struct.getDefinedComponents();
for (int i = definedComponents.length - 1; i >= 0; i--) {
DataTypeComponent dtc = definedComponents[i];
if (isPaddingComponent(dtc, false)) {
if (doDelete) {
struct.delete(dtc.getOrdinal());
}
else {
struct.clearComponent(dtc.getOrdinal());
}
}
}
}
private boolean isPaddingComponent(DataTypeComponent dtc, boolean skipBitFields) {
if (skipBitFields && dtc.isBitFieldComponent()) {
return false;
}
return PADDING_COMPONENT_NAME.equals(dtc.getFieldName());
}
private boolean isGoodAlignment(Composite testComposite, int preferredSize) {
boolean alignOK = true;
if (preferredSize > 0 && testComposite.getNumComponents() != 0) {
@ -410,6 +490,7 @@ public class DefaultCompositeMember extends CompositeMember {
@Override
int getLength() {
if (memberDataType instanceof BitFieldDataType) {
// FIXME: This assumption interferes with pack(1) case for bitfields
BitFieldDataType bitfield = (BitFieldDataType) memberDataType;
return bitfield.getBaseTypeSize();
}
@ -424,6 +505,9 @@ public class DefaultCompositeMember extends CompositeMember {
memberType = MemberType.STRUCTURE;
structureMemberOffsetMap = new TreeMap<>();
structureMemberRangeMap = new RangeMap(-1);
// allow padding size to use pointer-size and smaller by default
largestPrimitiveSize = memberDataType.getDataOrganization().getPointerSize();
hasPadding = false;
unionMemberList = null;
}
else {
@ -610,7 +694,6 @@ public class DefaultCompositeMember extends CompositeMember {
}
DefaultCompositeMember memberCopy = new DefaultCompositeMember(this);
memberCopy.memberOffset = 0;
CategoryPath tempCategoryPath = parent.getDataType().getCategoryPath();
String tempName = allocateTemporaryContainerName("struct");
@ -618,57 +701,18 @@ public class DefaultCompositeMember extends CompositeMember {
Structure nestedStructure =
new StructureDataType(tempCategoryPath, tempName, 0, dataTypeManager);
String oldName = memberName;
DataType oldDataType = memberDataType;
DefaultCompositeMember deferredBitFieldMember = null;
if (oldDataType instanceof PdbBitField) {
PdbBitField bitfieldDt = (PdbBitField) oldDataType;
try {
int bitOffset = bitfieldDt.getBitOffsetWithinBase();
DefaultCompositeMember padding = getPaddingBitField(null, memberCopy);
if (padding != null) {
deferredBitFieldMember = memberCopy;
memberCopy = padding;
bitfieldDt = (PdbBitField) memberCopy.memberDataType;
bitOffset = bitfieldDt.getBitOffsetWithinBase();
}
else if (bitOffset < 0) {
// TODO: assumes little-endian, add support for big-endian
bitOffset = 0;
}
insertMinimalStructureBitfield(nestedStructure, 0, memberCopy.memberName,
bitfieldDt, memberCopy.getMemberComment());
}
catch (InvalidDataTypeException e) {
Msg.error(this, "PDB failed to add bitfield: " + e.getMessage());
return false;
}
}
else {
nestedStructure.insertAtOffset(0, oldDataType, oldDataType.getLength(), oldName,
memberCopy.getMemberComment());
}
memberName = tempName;
memberOffset = 0;
memberDataType = nestedStructure;
memberIsZeroLengthArray = false;
memberDataTypeName = null; // signifies a container
initializeContainer();
structureMemberRangeMap.paintRange(0, memberCopy.getLength() - 1, 0);
structureMemberOffsetMap.put(0, memberCopy);
memberCopy.setParent(this);
if (parent != null) {
parent.memberChanged(oldName, this);
parent.memberChanged(memberCopy.memberName, this);
}
if (deferredBitFieldMember != null) {
return addStructureMember(deferredBitFieldMember);
}
return true;
return addStructureMember(memberCopy);
}
private String getMemberComment() {
@ -688,6 +732,85 @@ public class DefaultCompositeMember extends CompositeMember {
return buf.toString();
}
private int getMinimumPackedStructureLength() {
if (!isStructureContainer()) {
throw new IllegalStateException();
}
Entry<Integer, CompositeMember> lastEntry = structureMemberOffsetMap.lastEntry();
if (lastEntry == null) {
return 0;
}
int lastOffset = lastEntry.getKey();
CompositeMember lastMember = lastEntry.getValue();
return lastOffset + lastMember.getLength();
}
/**
* Insert minimal padding into structure prior to the addition of a component such that packing
* will allow component to be placed at intended offset.
* @param nextComponentOffset
* @param dt
*/
private void insertMinimalStructurePadding(int nextComponentOffset, DataType dt) {
if (!isStructureContainer()) {
throw new IllegalStateException();
}
int structLen = getMinimumPackedStructureLength();
if (nextComponentOffset <= structLen) {
return;
}
if (dt instanceof AbstractIntegerDataType) {
largestPrimitiveSize = Math.max(largestPrimitiveSize, dt.getLength());
}
Structure struct = (Structure) memberDataType;
int fillSpace = nextComponentOffset - structLen;
while (fillSpace > 0) {
int alignedOffset = DataOrganizationImpl.getAlignedOffset(dt.getAlignment(), structLen);
if (alignedOffset == nextComponentOffset) {
return;
}
DataType paddingDt = getPaddingDataType(nextComponentOffset, structLen);
if (paddingDt == null) {
return;
}
int paddingOffset =
DataOrganizationImpl.getAlignedOffset(paddingDt.getAlignment(), structLen);
struct.insertAtOffset(paddingOffset, paddingDt, -1, PADDING_COMPONENT_NAME, null);
hasPadding = true;
structLen = struct.getLength();
fillSpace = nextComponentOffset - structLen;
}
}
private DataType getPaddingDataType(int nextComponentOffset, int structLen) {
if (largestPrimitiveSize <= 1) {
return new CharDataType(dataTypeManager);
}
for (int paddingSize = largestPrimitiveSize; paddingSize > 1; --paddingSize) {
DataType paddingDt =
AbstractIntegerDataType.getSignedDataType(paddingSize, dataTypeManager);
int alignedOffset =
DataOrganizationImpl.getAlignedOffset(paddingDt.getAlignment(), structLen);
if ((alignedOffset + paddingSize) <= nextComponentOffset) {
return paddingDt;
}
}
return new CharDataType(dataTypeManager);
}
/**
* Insert a structure bitfield without creating additional undefined padding
* components (i.e., keep to minimal storage size).
@ -751,9 +874,6 @@ public class DefaultCompositeMember extends CompositeMember {
Composite composite = (Composite) memberDataType;
DataTypeComponent component = composite.getComponent(composite.getNumComponents() - 1);
// if (component.getOffset() != newMember.getOffset()) {
// return false; // unexpected
// }
DataType dataType = component.getDataType();
if (!(dataType instanceof BitFieldDataType) && !(dataType == DataType.DEFAULT)) {
@ -843,10 +963,13 @@ public class DefaultCompositeMember extends CompositeMember {
// TODO: assumes little-endian, add support for big-endian
bitOffset = 0;
}
insertMinimalStructurePadding(member.memberOffset,
bitfieldDt.getBaseDataType());
insertMinimalStructureBitfield((Structure) memberDataType, member.memberOffset,
member.getName(), bitfieldDt, member.getMemberComment());
}
else {
insertMinimalStructurePadding(member.memberOffset, member.memberDataType);
((Structure) memberDataType).insertAtOffset(member.memberOffset,
member.memberDataType, member.getLength(), member.memberName,
member.getMemberComment());
@ -955,13 +1078,29 @@ public class DefaultCompositeMember extends CompositeMember {
return true;
}
int unionMemberCount = unionMemberList.size();
CompositeMember lastUnionMember =
unionMemberCount == 0 ? null : unionMemberList.get(unionMemberCount - 1);
// NOTE: It is assumed that offset will always be ascending and not reach back to union
// members before the last one
CompositeMember lastUnionMember = unionMemberList.get(unionMemberList.size() - 1);
if (lastUnionMember == null) {
member.parent = this;
if (!member.transformIntoStructureContainer()) {
return false;
}
((Union) memberDataType).add(member.memberDataType, member.memberName, null);
unionMemberList.add(member);
if (parent != null) {
parent.sizeChanged(this);
}
return true;
}
if (lastUnionMember.isStructureContainer() &&
member.memberOffset >= lastUnionMember.getOffset()) {
DefaultCompositeMember struct = (DefaultCompositeMember) lastUnionMember;
if (struct.isRelatedBitField(member.memberOffset - lastUnionMember.getOffset(),
member)) {

View File

@ -4,9 +4,9 @@
* 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.
@ -103,6 +103,62 @@ public class CompositeMemberTest extends AbstractGhidraHeadlessIntegrationTest
Msg.info(this, message + " (Test: " + getName() + ")");
}
@Test
public void testUnionWithStructureOfBitFieldsWithPadding() throws Exception {
/**
<datatype name="TestUnion" kind="Union" length="0x8" >
<member name="field1:0x1:0xb" datatype="int" offset="0x4" kind="Member" length="0x1" />
<member name="field2:0x4:0xc" datatype="int" offset="0x4" kind="Member" length="0x4" />
</datatype>
union TestUnion {
struct {
int :32;
int :11;
int field1:1;
int field2:4;
};
short fieldSHORT;
};
*/
UnionDataType union = new UnionDataType(CategoryPath.ROOT, "testUnion", dataMgr);
//@formatter:off
List<MyPdbMember> members =
CollectionUtils.asList(
new MyPdbMember("field1:0x1:0xb", "int", 4),
new MyPdbMember("field2:0x4:0xc", "int", 4),
new MyPdbMember("fieldSHORT", "short", 0)
);
//@formatter:on
assertTrue(DefaultCompositeMember.applyDataTypeMembers(union, false, 8, members, this,
TaskMonitor.DUMMY));
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this,
"/testUnion\n" +
"pack()\n" +
"Union testUnion {\n" +
" 0 testUnion_s_0 8 _s_0 \"\"\n" +
" 0 short 2 fieldSHORT \"\"\n" +
"}\n" +
"Length: 8 Alignment: 4\n" +
"/testUnion/testUnion_s_0\n" +
"pack()\n" +
"Structure testUnion_s_0 {\n" +
" 0 int 4 _padding_ \"\"\n" +
" 4 int:11(0) 2 _padding_ \"\"\n" +
" 5 int:1(3) 1 field1 \"\"\n" +
" 5 int:4(4) 1 field2 \"\"\n" +
"}\n" +
"Length: 8 Alignment: 4", union, true);
//@formatter:on
}
@Test
public void testSimpleStructureWithBitFields() throws Exception {
@ -137,6 +193,72 @@ public class CompositeMemberTest extends AbstractGhidraHeadlessIntegrationTest
//@formatter:on
}
@Test
public void testSimplePack1StructureWithBitFields() throws Exception {
StructureDataType struct = new StructureDataType("struct", 0, dataMgr);
//@formatter:off
List<MyPdbMember> members =
CollectionUtils.asList(
new MyPdbMember("a", "uchar", 0),
new MyPdbMember("b:0x4:0x0", "uchar", 1),
new MyPdbMember("c:0x4:0x4", "uchar", 1),
new MyPdbMember("d:0x4:0x0", "uchar", 2),
new MyPdbMember("e:0x4:0x0", "uint", 3),
new MyPdbMember("f", "ushort", 7));
//@formatter:on
assertTrue(DefaultCompositeMember.applyDataTypeMembers(struct, false, 9, members, this,
TaskMonitor.DUMMY));
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this,
"/struct\n" +
"pack(1)\n" +
"Structure struct {\n" +
" 0 uchar 1 a \"\"\n" +
" 1 uchar:4(0) 1 b \"\"\n" +
" 1 uchar:4(4) 1 c \"\"\n" +
" 2 uchar:4(0) 1 d \"\"\n" +
" 3 uint:4(0) 1 e \"\"\n" +
" 7 ushort 2 f \"\"\n" +
"}\n" +
"Length: 9 Alignment: 1", struct, true);
//@formatter:on
}
@Test
public void testSimpleNoPackStructureWithBitFields() throws Exception {
StructureDataType struct = new StructureDataType("struct", 0, dataMgr);
//@formatter:off
List<MyPdbMember> members =
CollectionUtils.asList(
new MyPdbMember("a", "uchar", 0),
new MyPdbMember("b:0x4:0x0", "uchar", 1),
new MyPdbMember("e:0x4:0x0", "uint", 3),
new MyPdbMember("f", "ushort", 7));
//@formatter:on
assertTrue(DefaultCompositeMember.applyDataTypeMembers(struct, false, 9, members, this,
TaskMonitor.DUMMY));
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this,
"/struct\n" +
"pack(disabled)\n" +
"Structure struct {\n" +
" 0 uchar 1 a \"\"\n" +
" 1 uchar:4(0) 1 b \"\"\n" +
" 3 uint:4(0) 1 e \"\"\n" +
" 7 ushort 2 f \"\"\n" +
"}\n" +
"Length: 9 Alignment: 1", struct, true);
//@formatter:on
}
@Test
public void testSimpleStructureWithFillerBitFields() throws Exception {
@ -162,10 +284,10 @@ public class CompositeMemberTest extends AbstractGhidraHeadlessIntegrationTest
"pack()\n" +
"Structure struct {\n" +
" 0 uchar 1 a \"\"\n" +
" 1 uchar:2(0) 1 padding \"\"\n" +
" 1 uchar:2(0) 1 _padding_ \"\"\n" +
" 1 uchar:2(2) 1 b \"\"\n" +
" 1 uchar:1(4) 1 c1 \"\"\n" +
" 1 uchar:1(5) 1 padding \"\"\n" +
" 1 uchar:1(5) 1 _padding_ \"\"\n" +
" 1 uchar:1(6) 1 c2 \"\"\n" +
" 2 uchar:7(0) 1 d \"\"\n" +
" 4 uint:4(0) 1 e \"\"\n" +
@ -238,20 +360,20 @@ public class CompositeMemberTest extends AbstractGhidraHeadlessIntegrationTest
"pack()\n" +
"Structure struct_u_0_s_0 {\n" +
" 0 char:1(0) 1 a0 \"\"\n" +
" 0 char:1(1) 1 padding \"\"\n" +
" 0 char:1(1) 1 _padding_ \"\"\n" +
" 0 char:1(2) 1 a2 \"\"\n" +
" 0 char:1(3) 1 padding \"\"\n" +
" 0 char:1(3) 1 _padding_ \"\"\n" +
" 0 char:1(4) 1 a4 \"\"\n" +
"}\n" +
"Length: 1 Alignment: 1\n" +
"/struct/struct_u_0/struct_u_0_s_1\n" +
"pack()\n" +
"Structure struct_u_0_s_1 {\n" +
" 0 char:1(0) 1 padding \"\"\n" +
" 0 char:1(0) 1 _padding_ \"\"\n" +
" 0 char:1(1) 1 a1 \"\"\n" +
" 0 char:1(2) 1 padding \"\"\n" +
" 0 char:1(2) 1 _padding_ \"\"\n" +
" 0 char:1(3) 1 a3 \"\"\n" +
" 0 char:1(4) 1 padding \"\"\n" +
" 0 char:1(4) 1 _padding_ \"\"\n" +
" 0 char:1(5) 1 a5 \"\"\n" +
"}\n" +
"Length: 1 Alignment: 1", struct, true);