GP-5076 change PDB class naming for bases and internal category and

significant change to tests

Ghidra/Features/Base/src/main/java/ghidra/app/util/ClassUtils.java

@@ -0,0 +1,35 @@
+/* ###
+ * 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.app.util;
+
+import ghidra.program.model.data.PointerDataType;
+
+public class ClassUtils {
+
+	// Prototype values for now.  Need to come to agreement on what these should be
+	public static final String VBPTR = "{vbptr}";
+	public static final String VFPTR = "{vfptr}";
+
+	/**
+	 * Type used for {@link #VBPTR} and {@link #VFPTR} fields in a class
+	 */
+	public static final PointerDataType VXPTR_TYPE = new PointerDataType();
+
+	private ClassUtils() {
+		// no instances
+	}
+
+}

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/CompositeTypeApplier.java

@@ -362,16 +362,15 @@ public class CompositeTypeApplier extends AbstractComplexTypeApplier {
 			applicator.checkCancelled();
 			RecordNumber recordNumber = vftPtr.getPointerTypeRecordNumber();
 			DataType dataType = applicator.getDataType(recordNumber);
-			if (dataType == null) {
-				throw new PdbException("Type not processed for record: " + recordNumber);
+			if (!(dataType instanceof Pointer ptrType)) {
+				throw new PdbException("vftptr not pointer type for record: " + recordNumber);
 			}
 			int offset = vftPtr.getOffset();
 			String vftPtrMemberName = vftPtr.getName();
 			DefaultPdbUniversalMember member =
 				new DefaultPdbUniversalMember(vftPtrMemberName, dataType, offset);
 			myMembers.add(member);
-			myClassType.addVirtualFunctionTablePointer(member.getName(),
-				member.getDataType().getDataType(), member.getOffset());
+			myClassType.addVirtualFunctionTablePointer(ptrType, offset);
 		}
 	}
 

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/CppCompositeType.java

@@ -20,6 +20,7 @@ import java.util.Map.Entry;
 
 import org.apache.commons.lang3.StringUtils;
 
+import ghidra.app.util.ClassUtils;
 import ghidra.app.util.SymbolPath;
 import ghidra.app.util.bin.format.pdb.*;
 import ghidra.app.util.bin.format.pdb2.pdbreader.PdbException;
@@ -37,6 +38,13 @@ import ghidra.util.task.TaskMonitor;
  */
 public class CppCompositeType {
 
+	private static final String SELF_BASE_COMMENT = "Self Base";
+	private static final String BASE_COMMENT = "Base";
+	private static final String VIRTUAL_BASE_COMMENT = "Virtual Base";
+	private static final String VIRTUAL_BASE_SPECULATIVE_COMMENT =
+		"Virtual Base - Speculative Placement";
+	//private static final String INDIRECT_VIRTUAL_BASE_CLASS_COMMENT = "Indirect Virtual Base Class";
+
 	// Order matters for both base classes and members for class layout.  Members get offsets,
 	//  which helps for those, but layout algorithms usually utilize order.
 	private List<SyntacticBaseClass> syntacticBaseClasses;
@@ -44,6 +52,7 @@ public class CppCompositeType {
 	private List<AbstractMember> myMembers;
 	private List<Member> layoutMembers;
 	private List<Member> layoutVftPtrMembers;
+	private Map<Integer, Pointer> vftPtrTypeByOffset;
 	private boolean isFinal;
 	private ClassKey classKey;
 	private String className; // String for now.
@@ -53,20 +62,19 @@ public class CppCompositeType {
 	private SymbolPath symbolPath;
 	private Composite composite;
 	private CategoryPath categoryPath;
-
-	private ObjectOrientedClassLayout classLayout = null;
+	private CategoryPath internalsCategoryPath;
 
 	private List<ClassPdbMember> memberData;
 
-	private boolean hasDirect;
+	private boolean hasSelfBase;
 
-	static String createDirectClassName(Composite composite) {
-		return composite.getName() + "_direct";
+	static String createSelfBaseClassName(Composite composite) {
+		return composite.getName();
 	}
 
-	static CategoryPath createDirectCategoryPath(CppCompositeType cppType) {
-		return cppType.getBaseCategoryName(
-			CppCompositeType.createDirectClassName(cppType.getComposite()));
+	static CategoryPath createSelfBaseCategoryPath(CppCompositeType cppType) {
+		return cppType.getSelfBaseCategoryName(
+			CppCompositeType.createSelfBaseClassName(cppType.getComposite()));
 	}
 
 	/*
@@ -92,6 +100,7 @@ public class CppCompositeType {
 
 		memberData = new ArrayList<>();
 		layoutVftPtrMembers = new ArrayList<>();
+		vftPtrTypeByOffset = new HashMap<>();
 
 		isFinal = false;
 		classKey = ClassKey.UNKNOWN;
@@ -100,6 +109,7 @@ public class CppCompositeType {
 		this.composite = composite;
 		placeholderVirtualBaseTables = new HashMap<>();
 		categoryPath = new CategoryPath(composite.getCategoryPath(), composite.getName());
+		internalsCategoryPath = new CategoryPath(categoryPath, "!internal");
 		this.mangledName = mangledName;
 	}
 
@@ -214,6 +224,10 @@ public class CppCompositeType {
 		return categoryPath;
 	}
 
+	public CategoryPath getInternalsCategoryPath() {
+		return internalsCategoryPath;
+	}
+
 	public void setFinal(boolean isFinal) {
 		this.isFinal = isFinal;
 	}
@@ -275,9 +289,17 @@ public class CppCompositeType {
 		return layoutMembers.size();
 	}
 
-	public void addVirtualFunctionTablePointer(String name, DataType dataType, int offset) {
-		Member newMember = new Member(name, dataType, false, ClassFieldAttributes.UNKNOWN, offset);
+	public void addVirtualFunctionTablePointer(Pointer ptrType, int offset) {
+		// Not using ptrType argument for member... not generic enough
+		Member newMember =
+			new Member(ClassUtils.VFPTR, ClassUtils.VXPTR_TYPE, false, ClassFieldAttributes.UNKNOWN,
+				offset);
 		layoutVftPtrMembers.add(newMember);
+		vftPtrTypeByOffset.put(offset, ptrType);
+	}
+
+	public Pointer getVftPtrType(int offset) {
+		return vftPtrTypeByOffset.get(offset);
 	}
 
 	private void insertVirtualFunctionTablePointers(List<ClassPdbMember> pdbMembers) {
@@ -596,33 +618,6 @@ public class CppCompositeType {
 		return builder.toString();
 	}
 
-	public ObjectOrientedClassLayout getLayout(ObjectOrientedClassLayout layoutOptions) {
-		if (classLayout == null) {
-			classLayout = determineClassLayout(layoutOptions);
-		}
-		return classLayout;
-	}
-
-	private ObjectOrientedClassLayout determineClassLayout(
-			ObjectOrientedClassLayout layoutOptions) {
-		ObjectOrientedClassLayout initialLayoutDetermination;
-		if (layoutOptions == ObjectOrientedClassLayout.MEMBERS_ONLY) {
-			return ObjectOrientedClassLayout.MEMBERS_ONLY;
-		}
-		else if (getNumLayoutBaseClasses() == 0) {
-			initialLayoutDetermination = ObjectOrientedClassLayout.BASIC_SIMPLE_COMPLEX;
-		}
-		else if (getNumLayoutVirtualBaseClasses() == 0) {
-			initialLayoutDetermination = ObjectOrientedClassLayout.SIMPLE_COMPLEX;
-		}
-		else {
-			initialLayoutDetermination = ObjectOrientedClassLayout.COMPLEX;
-		}
-		ObjectOrientedClassLayout classLayoutOption = layoutOptions;
-		return classLayoutOption.compareTo(initialLayoutDetermination) >= 0 ? classLayoutOption
-				: initialLayoutDetermination;
-	}
-
 	boolean isZeroSize() {
 		return memberData.size() == 0;
 	}
@@ -659,76 +654,70 @@ public class CppCompositeType {
 	public void createVbtBasedLayout(ObjectOrientedClassLayout layoutOptions, VxtManager vxtManager,
 			TaskMonitor monitor) throws PdbException, CancelledException {
 		CategoryPath cn;
-		hasDirect = false;
-		switch (getLayout(layoutOptions)) {
+		hasSelfBase = false;
+		switch (layoutOptions) {
 			case MEMBERS_ONLY:
 				addLayoutPdbMembers(memberData, layoutMembers);
 				break;
-			case BASIC_SIMPLE_COMPLEX:
-				addLayoutPdbMembers(memberData, layoutMembers);
-				insertVirtualFunctionTablePointers(memberData);
-				break;
-			// TODO: evaluate... not really getting difference I thought we could get... so far
-			//  BASIC and SIMPLE seem to yield the same results.  I might be doing something wrong.
-			case SIMPLE_COMPLEX:
-			case COMPLEX:
-				cn = createDirectCategoryPath(this);
-				Composite directDataType = new StructureDataType(cn.getParent(), cn.getName(), 0,
+			case CLASS_HIERARCHY:
+				cn = createSelfBaseCategoryPath(this);
+				Composite selfBaseType = new StructureDataType(cn.getParent(), cn.getName(), 0,
 					composite.getDataTypeManager());
+				selfBaseType.setDescription("Base of " + cn.getName());
 
-				List<ClassPdbMember> directClassPdbMembers = getDirectBaseClassMembers(monitor);
+				List<ClassPdbMember> selfBasePdbMembers = getSelfBaseClassMembers(monitor);
 				List<VirtualLayoutBaseClass> myVirtualLayoutBases = preprocessVirtualBases(monitor);
 
 				// TODO: consider moving down below next line.
 				boolean allVbtFound =
 					reconcileVirtualBaseTables(composite.getDataTypeManager(), vxtManager);
 
-				addLayoutPdbMembers(directClassPdbMembers, layoutMembers);
-				insertVirtualFunctionTablePointers(directClassPdbMembers);
+				addLayoutPdbMembers(selfBasePdbMembers, layoutMembers);
+				insertVirtualFunctionTablePointers(selfBasePdbMembers);
 
-				if (!DefaultCompositeMember.applyDataTypeMembers(directDataType, false, false, 0,
-					directClassPdbMembers, msg -> Msg.warn(this, msg), monitor)) {
-					clearComponents(directDataType);
+				if (!DefaultCompositeMember.applyDataTypeMembers(selfBaseType, false, false, 0,
+					selfBasePdbMembers, msg -> Msg.warn(this, msg), monitor)) {
+					clearComponents(selfBaseType);
 				}
-				int directClassLength = getCompositeLength(directDataType);
+				int selfBaseLength = getCompositeLength(selfBaseType);
 
-				if (directClassLength == 0) {
-					// Not using the direct type (only used it to get the directClassLength), so
+				if (selfBaseLength == 0) {
+					// Not using the direct type (only used it to get the selfBaseLength), so
 					//  remove it and add the members to the main type instead.
-					directDataType.getDataTypeManager().remove(directDataType, monitor);
+					selfBaseType.getDataTypeManager().remove(selfBaseType, monitor);
 				}
 				else {
 					// this does not deal with the case where more members from memberData get
 					// added below and must still fit in "size."
-					if (directClassLength > size) {
+					if (selfBaseLength > size) {
 						// Redo it with the size of the overall structure/class
-						directDataType.getDataTypeManager().remove(directDataType, monitor);
-						directDataType = new StructureDataType(cn.getParent(), cn.getName(), 0,
+						selfBaseType.getDataTypeManager().remove(selfBaseType, monitor);
+						selfBaseType = new StructureDataType(cn.getParent(), cn.getName(), 0,
 							composite.getDataTypeManager());
-						if (!DefaultCompositeMember.applyDataTypeMembers(directDataType, false,
-							false, size, directClassPdbMembers, msg -> Msg.warn(this, msg),
+						if (!DefaultCompositeMember.applyDataTypeMembers(selfBaseType, false,
+							false, size, selfBasePdbMembers, msg -> Msg.warn(this, msg),
 							monitor)) {
-							clearComponents(directDataType);
+							clearComponents(selfBaseType);
 						}
-						directClassLength = getCompositeLength(directDataType);
+						selfBaseLength = getCompositeLength(selfBaseType);
 					}
-					if (getLayout(layoutOptions) == ObjectOrientedClassLayout.SIMPLE_COMPLEX) {
+					if (getNumLayoutVirtualBaseClasses() == 0) {
 						// Not using the dummy/direct type (only used it to get the
-						//  directClassLength), so remove it and add the members to the main
+						//  selfBaseLength), so remove it and add the members to the main
 						//  type instead.
-						directDataType.getDataTypeManager().remove(directDataType, monitor);
-						memberData.addAll(directClassPdbMembers);
+						selfBaseType.getDataTypeManager().remove(selfBaseType, monitor);
+						memberData.addAll(selfBasePdbMembers);
 						//addLayoutPdbMembers(memberData, layoutMembers, monitor);
 					}
 					else {
 						ClassPdbMember directClassPdbMember =
-							new ClassPdbMember("", directDataType, false, 0, null);
+							new ClassPdbMember("", selfBaseType, false, 0, SELF_BASE_COMMENT);
 						memberData.add(directClassPdbMember);
-						hasDirect = true;
+						hasSelfBase = true;
 					}
 				}
 
-				addVirtualBases(directClassLength, memberData, myVirtualLayoutBases, allVbtFound,
+				addVirtualBases(selfBaseLength, memberData, myVirtualLayoutBases, allVbtFound,
 					monitor);
 
 				break;
@@ -744,9 +733,9 @@ public class CppCompositeType {
 
 	//----------------------------------------------------------------------------------------------
 	//----------------------------------------------------------------------------------------------
-	private List<ClassPdbMember> getDirectBaseClassMembers(TaskMonitor monitor)
+	private List<ClassPdbMember> getSelfBaseClassMembers(TaskMonitor monitor)
 			throws CancelledException {
-		List<ClassPdbMember> myDirectClassPdbMembers = new ArrayList<>();
+		List<ClassPdbMember> mySelfClassPdbMembers = new ArrayList<>();
 		TreeMap<Integer, Member> orderedBaseMembers = new TreeMap<>();
 		for (LayoutBaseClass base : getLayoutBaseClasses()) {
 			monitor.checkCancelled();
@@ -755,18 +744,16 @@ public class CppCompositeType {
 				if (!baseComposite.isZeroSize()) {
 					Composite baseDataType = base.getDirectDataType();
 					int offset = ((DirectLayoutBaseClass) base).getOffset();
-					CategoryPath cn =
-						getBaseCategoryName("BaseClass_" + base.getBaseClassType().getName());
 					Member baseMember =
-						new Member("", baseDataType, false, null, offset, cn.toString());
+						new Member("", baseDataType, false, null, offset, BASE_COMMENT);
 					orderedBaseMembers.put(offset, baseMember);
 				}
 			}
 		}
 		for (Member baseMember : orderedBaseMembers.values()) {
-			addPdbMember(myDirectClassPdbMembers, baseMember);
+			addPdbMember(mySelfClassPdbMembers, baseMember);
 		}
-		return myDirectClassPdbMembers;
+		return mySelfClassPdbMembers;
 	}
 
 	//----------------------------------------------------------------------------------------------
@@ -796,80 +783,73 @@ public class CppCompositeType {
 		//  so with multiple virtual inheritance, a parent from multiple family lines will likely
 		//  get moved.
 		CategoryPath cn;
-		hasDirect = false;
-		switch (getLayout(layoutOptions)) {
+		hasSelfBase = false;
+		switch (layoutOptions) {
 			case MEMBERS_ONLY:
 				addLayoutPdbMembers(memberData, layoutMembers);
 				break;
-			case BASIC_SIMPLE_COMPLEX:
-				cn = composite.getCategoryPath();
-				addLayoutPdbMembers(memberData, layoutMembers);
-				insertVirtualFunctionTablePointers(memberData);
-				break;
-			// TODO: evaluate... not really getting difference I thought we could get... so far
-			//  BASIC and SIMPLE seem to yield the same results.  I might be doing something wrong.
-			case SIMPLE_COMPLEX:
-			case COMPLEX:
-				cn = createDirectCategoryPath(this);
-				Composite directDataType = new StructureDataType(cn.getParent(), cn.getName(), 0,
+			case CLASS_HIERARCHY:
+				cn = createSelfBaseCategoryPath(this);
+				Composite selfBaseType = new StructureDataType(cn.getParent(), cn.getName(), 0,
 					composite.getDataTypeManager());
+				selfBaseType.setDescription("Base of " + cn.getName());
 
 				List<LayoutBaseClass> myAccumulatedDirectBases = new ArrayList<>();
 				List<VirtualLayoutBaseClass> myAccumulatedVirtualBases = new ArrayList<>();
-				List<ClassPdbMember> directClassPdbMembers = new ArrayList<>();
-				processBaseClassesRecursive(this, true, directClassPdbMembers,
+				List<ClassPdbMember> selfBasePdbMembers = new ArrayList<>();
+				processBaseClassesRecursive(this, true, selfBasePdbMembers,
 					myAccumulatedDirectBases, myAccumulatedVirtualBases, 0, monitor);
 
 				// TODO: consider moving down below next line.
 				boolean allVbtFound =
 					reconcileVirtualBaseTables(composite.getDataTypeManager(), vxtManager);
 
-				addLayoutPdbMembers(directClassPdbMembers, layoutMembers);
-				insertVirtualFunctionTablePointers(directClassPdbMembers);
+				addLayoutPdbMembers(selfBasePdbMembers, layoutMembers);
+				insertVirtualFunctionTablePointers(selfBasePdbMembers);
 
-				if (!DefaultCompositeMember.applyDataTypeMembers(directDataType, false, false, 0,
-					directClassPdbMembers, msg -> Msg.warn(this, msg), monitor)) {
-					clearComponents(directDataType);
+				if (!DefaultCompositeMember.applyDataTypeMembers(selfBaseType, false, false, 0,
+					selfBasePdbMembers, msg -> Msg.warn(this, msg), monitor)) {
+					clearComponents(selfBaseType);
 				}
-				int directClassLength = getCompositeLength(directDataType);
+				int selfBaseLength = getCompositeLength(selfBaseType);
 
-				if (directClassLength == 0) {
-					// Not using the direct type (only used it to get the directClassLength), so
+				if (selfBaseLength == 0) {
+					// Not using the direct type (only used it to get the selfBaseLength), so
 					//  remove it and add the members to the main type instead.
-					directDataType.getDataTypeManager().remove(directDataType, monitor);
+					selfBaseType.getDataTypeManager().remove(selfBaseType, monitor);
 				}
 				else {
 					// this does not deal with the case where more members from memberData get
 					// added below and must still fit in "size."
-					if (directClassLength > size) {
+					if (selfBaseLength > size) {
 						// Redo it with the size of the overall structure/class
-						directDataType.getDataTypeManager().remove(directDataType, monitor);
-						directDataType = new StructureDataType(cn.getParent(), cn.getName(), 0,
+						selfBaseType.getDataTypeManager().remove(selfBaseType, monitor);
+						selfBaseType = new StructureDataType(cn.getParent(), cn.getName(), 0,
 							composite.getDataTypeManager());
-						if (!DefaultCompositeMember.applyDataTypeMembers(directDataType, false,
-							false, size, directClassPdbMembers, msg -> Msg.warn(this, msg),
+						if (!DefaultCompositeMember.applyDataTypeMembers(selfBaseType, false,
+							false, size, selfBasePdbMembers, msg -> Msg.warn(this, msg),
 							monitor)) {
-							clearComponents(directDataType);
+							clearComponents(selfBaseType);
 						}
-						directClassLength = getCompositeLength(directDataType);
+						selfBaseLength = getCompositeLength(selfBaseType);
 					}
-					if (getLayout(layoutOptions) == ObjectOrientedClassLayout.SIMPLE_COMPLEX) {
+					if (getNumLayoutVirtualBaseClasses() == 0) {
 						// Not using the dummy/direct type (only used it to get the
-						//  directClassLength), so remove it and add the members to the main
+						//  selfBaseLength), so remove it and add the members to the main
 						//  type instead.
-						directDataType.getDataTypeManager().remove(directDataType, monitor);
-						memberData.addAll(directClassPdbMembers);
+						selfBaseType.getDataTypeManager().remove(selfBaseType, monitor);
+						memberData.addAll(selfBasePdbMembers);
 						//addLayoutPdbMembers(memberData, layoutMembers, monitor);
 					}
 					else {
 						ClassPdbMember directClassPdbMember =
-							new ClassPdbMember("", directDataType, false, 0, null);
+							new ClassPdbMember("", selfBaseType, false, 0, SELF_BASE_COMMENT);
 						memberData.add(directClassPdbMember);
-						hasDirect = true;
+						hasSelfBase = true;
 					}
 				}
 
-				addVirtualBasesSpeculatively(directClassLength, memberData,
+				addVirtualBasesSpeculatively(selfBaseLength, memberData,
 					myAccumulatedVirtualBases, monitor);
 
 				break;
@@ -902,10 +882,8 @@ public class CppCompositeType {
 					if (!baseComposite.isZeroSize()) {
 						Composite baseDataType = base.getDirectDataType();
 						int offset = ((DirectLayoutBaseClass) base).getOffset();
-						CategoryPath cn =
-							getBaseCategoryName("BaseClass_" + base.getBaseClassType().getName());
 						Member baseMember =
-							new Member("", baseDataType, false, null, offset, cn.toString());
+							new Member("", baseDataType, false, null, offset, BASE_COMMENT);
 						addPdbMember(myPdbMembers, baseMember);
 					}
 					myAccumulatedDirectBases.add(base);
@@ -1012,10 +990,10 @@ public class CppCompositeType {
 		CppParentageAndMember cAndP = findDirectBaseParentageAndMember(this, 0, vbtptrOffset);
 
 		if (cAndP == null) {
-			insertMember("{vbptr}", vbptr, false, vbtptrOffset, "{vbptr} for " + myClass);
+			insertMember(ClassUtils.VBPTR, ClassUtils.VXPTR_TYPE, false, vbtptrOffset, null);
 		}
-		else if (!"{vbptr}".equals(cAndP.member().getName())) {
-			String message = "PDB: Collision of non-{vbptr}.";
+		else if (!ClassUtils.VBPTR.equals(cAndP.member().getName())) {
+			String message = "PDB: Collision of non-" + ClassUtils.VBPTR;
 			PdbLog.message(message);
 			Msg.info(this, message);
 			return false;
@@ -1027,11 +1005,10 @@ public class CppCompositeType {
 			return false;
 		}
 		int entrySize = 4; // Default to something (could be wrong)
-		if (vbptr instanceof PointerDataType) {
-			entrySize = ((PointerDataType) vbptr).getDataType().getLength();
+		if (vbptr instanceof Pointer ptr) {
+			entrySize = ptr.getDataType().getLength();
 		}
-		boolean x = findVbt(table, mvxtManager, entrySize, symbolPath, parentage);
-		return x;
+		return findVbt(table, mvxtManager, entrySize, symbolPath, parentage);
 	}
 
 	private boolean findVbt(PlaceholderVirtualBaseTable table, MsftVxtManager mvbtm, int entrySize,
@@ -1052,7 +1029,7 @@ public class CppCompositeType {
 	}
 
 	private CppParentageAndMember findDirectBaseParentageAndMember(CppCompositeType cppType,
-			int offsetCppType, int vbtptrOffset) throws PdbException {
+			int offsetCppType, int offset) throws PdbException {
 		for (LayoutBaseClass base : cppType.layoutBaseClasses) {
 			if (!(base instanceof DirectLayoutBaseClass)) {
 				continue;
@@ -1060,13 +1037,13 @@ public class CppCompositeType {
 			DirectLayoutBaseClass directBase = (DirectLayoutBaseClass) base;
 			int directBaseOffset = directBase.getOffset() + offsetCppType;
 			int directBaseLength = directBase.getDirectDataType().getLength();
-			if (vbtptrOffset >= directBaseOffset &&
-				vbtptrOffset < directBaseOffset + directBaseLength) {
+			if (offset >= directBaseOffset &&
+				offset < directBaseOffset + directBaseLength) {
 				CppCompositeType childCppType = directBase.getBaseClassType();
 				CppParentageAndMember cAndP =
-					findDirectBaseParentageAndMember(childCppType, directBaseOffset, vbtptrOffset);
+					findDirectBaseParentageAndMember(childCppType, directBaseOffset, offset);
 				if (cAndP == null) {
-					Member member = childCppType.findLayoutMemberOrVftPtrMember(vbtptrOffset);
+					Member member = childCppType.findLayoutMember(offset);
 					if (member == null) {
 						return null;
 					}
@@ -1080,17 +1057,20 @@ public class CppCompositeType {
 		return null;
 	}
 
-	private Member findLayoutMemberOrVftPtrMember(int offset) {
+	private Member findLayoutMember(int offset) {
+		// the following will report the basic, such as int *
 		for (Member member : layoutMembers) {
 			if (member.getOffset() == offset) {
 				return member;
 			}
 		}
-		for (Member member : layoutVftPtrMembers) {
-			if (member.getOffset() == offset) {
-				return member;
-			}
-		}
+		// the following will report the more info, such as shape1234 *, but this code will not
+		//  hit here, as we should not have an entry for the offset if it did not find it above
+//		for (Member member : layoutVftPtrMembers) {
+//			if (member.getOffset() == offset) {
+//				return member;
+//			}
+//		}
 		return null;
 	}
 
@@ -1136,6 +1116,8 @@ public class CppCompositeType {
 	private void addVirtualBases(int startOffset, List<ClassPdbMember> pdbMembers,
 			List<VirtualLayoutBaseClass> virtualBases, boolean allVbtFound, TaskMonitor monitor)
 			throws PdbException, CancelledException {
+		// We accumulate the comment because if there are any empty base classes, they take
+		// no space and will be at the same offset as a non-empty base class
 		String accumulatedComment = "";
 		int memberOffset = startOffset;
 		List<VirtualLayoutBaseClass> orderedBases = new ArrayList<>();
@@ -1160,18 +1142,20 @@ public class CppCompositeType {
 //			}
 			memberOffset += basePointerOffset;
 			if (virtualBaseLength != 0) {
-				String comment =
-					"(Virtual Base " + virtualBase.getDataTypePath().getDataTypeName() + ")";
-				accumulatedComment += comment;
+				String comment = VIRTUAL_BASE_COMMENT;
+				if (!accumulatedComment.isEmpty()) {
+					comment += " and previous " + accumulatedComment;
+				}
+				//accumulatedComment += "Virtual Base";
 				ClassPdbMember virtualClassPdbMember =
-					new ClassPdbMember("", baseDataType, false, memberOffset, accumulatedComment);
+					new ClassPdbMember("", baseDataType, false, memberOffset, comment);
 				pdbMembers.add(virtualClassPdbMember);
 				memberOffset += virtualBaseLength;
 				accumulatedComment = "";
 			}
 			else {
-				String comment = "(Virtual Base (empty) " +
-					virtualBase.getDataTypePath().getDataTypeName() + ")";
+				String comment =
+					"(Empty Virtual Base " + virtualBase.getDataTypePath().getDataTypeName() + ")";
 				accumulatedComment += comment;
 			}
 			// If last base is empty, then its comment and any accumulated to this point
@@ -1211,6 +1195,8 @@ public class CppCompositeType {
 	private void addVirtualBasesSpeculatively(int startOffset, List<ClassPdbMember> pdbMembers,
 			List<VirtualLayoutBaseClass> virtualBases, TaskMonitor monitor)
 			throws CancelledException {
+		// We accumulate the comment because if there are any empty base classes, they take
+		// no space and will be at the same offset as a non-empty base class
 		String accumulatedComment = "";
 		int memberOffset = startOffset;
 		for (VirtualLayoutBaseClass virtualBase : virtualBases) {
@@ -1219,8 +1205,7 @@ public class CppCompositeType {
 			int virtualBaseLength = getCompositeLength(baseDataType);
 
 			if (virtualBaseLength != 0) {
-				String comment = "((Speculative Placement) Virtual Base " +
-					virtualBase.getDataTypePath().getDataTypeName() + ")";
+				String comment = VIRTUAL_BASE_SPECULATIVE_COMMENT;
 				accumulatedComment += comment;
 				ClassPdbMember virtualClassPdbMember =
 					new ClassPdbMember("", baseDataType, false, memberOffset, accumulatedComment);
@@ -1285,8 +1270,8 @@ public class CppCompositeType {
 		return false;
 	}
 
-	CategoryPath getBaseCategoryName(String baseName) {
-		CategoryPath cn = getCategoryPath();
+	CategoryPath getSelfBaseCategoryName(String baseName) {
+		CategoryPath cn = getInternalsCategoryPath();
 		return new CategoryPath(cn, baseName);
 	}
 
@@ -1328,10 +1313,6 @@ public class CppCompositeType {
 			return attributes;
 		}
 
-		ObjectOrientedClassLayout getLayoutMode(ObjectOrientedClassLayout layoutOptions) {
-			return baseClassType.getLayout(layoutOptions);
-		}
-
 		DataTypePath getDataTypePath() {
 			return new DataTypePath(baseClassType.getCategoryPath().getParent(),
 				baseClassType.getCategoryPath().getName());
@@ -1346,21 +1327,19 @@ public class CppCompositeType {
 		}
 
 		Composite getDirectDataType() {
-			Composite c = getBaseClassType().getComposite();
-			if (c.getNumComponents() == 0) {
-				return c;
+			CppCompositeType cct = getBaseClassType();
+			CategoryPath selfBasePath = createSelfBaseCategoryPath(cct);
+			DataTypeManager dtm = cct.getComposite().getDataTypeManager();
+			DataType base = dtm.getDataType(selfBasePath.getParent(), selfBasePath.getName());
+			if (base == null) {
+				// There is no self base meaning that the layout class is simple and no self
+				//  base was needed.  So return the layout class
+				base = cct.getComposite();
 			}
-			if (!baseClassType.hasDirect) {
-				return c;
+			if (base instanceof Structure s) {
+				return s;
 			}
-			DataTypeComponent dtc = c.getComponent(0); // by construction this should be "Direct"
-			DataType dt = dtc.getDataType();
-			Structure bdt;
-			if (!(dt instanceof Structure)) {
-				throw new AssertException("Not Structure for Direct");
-			}
-			bdt = (Structure) dt;
-			return bdt;
+			throw new AssertException("Cannot find Base type");
 		}
 
 	}

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/ObjectOrientedClassLayout.java

@@ -19,17 +19,20 @@ package ghidra.app.util.pdb.pdbapplicator;
  * PDB Analyzer user algorithmic choice for performing object oriented class layout.
  * <p>
  * Actual algorithms determination is as follows:
- * {@link #MEMBERS_ONLY} is a fixed setting.  The others weigh the setting against the data
- * available in the class records.  They all start out as {@link #COMPLEX} but fall back to a
- * more simplistic layout when the data permits:
- * <p> {@link #SIMPLE_COMPLEX} can fall back to "simple" and
- * <p> {@link #BASIC_SIMPLE_COMPLEX} can fall back to "simple" or "basic"
+ * <p> {@link #MEMBERS_ONLY} is a the legacy output that only shows members of the current class.
+ * <p> {@link #CLASS_HIERARCHY} provides a nested layout more suitable for understanding
+ * the composition of classes from base classes and members from the Structure Editor perspective.
  */
 public enum ObjectOrientedClassLayout {
-	MEMBERS_ONLY("Legacy"),
-	BASIC_SIMPLE_COMPLEX("Complex with Basic Fallback"),
-	SIMPLE_COMPLEX("Complex with Simple Fallback"),
-	COMPLEX("Complex Always");
+	/**
+	 * Processes members of the current class only; legacy solution
+	 */
+	MEMBERS_ONLY("No C++ Hierarchy (Legacy)"),
+	/**
+	 * Include base class hierarchies and other C++-isms into a class layout that is suited for
+	 * understanding the hierarchies and components from the Structure Editor perspective
+	 */
+	CLASS_HIERARCHY("Class Hierarchy (Experimental)");
 
 	private final String label;