GP-5559 - PDB - Work around issue with finding primary vxt by symbol

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/classtype/MsftVxtManager.java

@@ -124,8 +124,8 @@ public class MsftVxtManager extends VxtManager {
 	//  memory are the same order that their pointers appear in the classes... this is based solely
 	//  on limited experience.  The solution stinks and would benefit from the original direction
 	//  of using the parentage.  We will try to use the parentage as a litmus test on retrieval
-	private Map<ClassID, TreeMap<Address, VirtualBaseTable>> vbtsByOwner;
+	private Map<ClassID, List<VirtualBaseTable>> vbtsByOwner;
-	private Map<ClassID, TreeMap<Address, VirtualFunctionTable>> vftsByOwner;
+	private Map<ClassID, List<VirtualFunctionTable>> vftsByOwner;
 
 	// Used for locating vft and vbt
 	// These are explicitly used for storing/retrieving the "program" versions which result from
@@ -173,27 +173,73 @@ public class MsftVxtManager extends VxtManager {
 	/**
 	 * Finds the putative {@link VirtualBaseTable} in memory requested for the owning class
 	 * @param owner the owning class of the table
+	 * @param parentage the primary parentage (only used if creating placeholder)
 	 * @return the table
 	 */
-	public VirtualBaseTable findPrimaryVbt(ClassID owner) {
+	public VirtualBaseTable findPrimaryVbt(ClassID owner, List<ClassID> parentage) {
-		TreeMap<Address, VirtualBaseTable> map = vbtsByOwner.get(owner);
+		List<VirtualBaseTable> list = vbtsByOwner.get(owner);
-		if (map == null) {
+		if (list == null) {
 			return null;
 		}
-		return map.firstEntry().getValue();
+		VirtualBaseTable result = null;
+		VirtualBaseTable firstPlaceholder = null;
+		Address addrResult = null;
+		for (VirtualBaseTable table : list) {
+			if (table instanceof ProgramVirtualBaseTable pvbt) {
+				Address addr = pvbt.getAddress();
+				if (addrResult == null || addr.compareTo(addrResult) < 0) {
+					addrResult = addr;
+					result = table;
+				}
+			}
+			else if (firstPlaceholder == null) {
+				firstPlaceholder = table;
+			}
+		}
+		if (result == null) {
+			if (firstPlaceholder != null) {
+				return firstPlaceholder;
+			}
+			result = new PlaceholderVirtualBaseTable(owner, parentage);
+			list.add(result);
+		}
+		return result;
 	}
 
 	/**
 	 * Finds the putative {@link VirtualFunctionTable} in memory requested for the owning class
 	 * @param owner the owning class of the table
+	 * @param parentage the primary parentage (only used if creating placeholder)
 	 * @return the table
 	 */
-	public VirtualFunctionTable findPrimaryVft(ClassID owner) {
+	public VirtualFunctionTable findPrimaryVft(ClassID owner, List<ClassID> parentage) {
-		TreeMap<Address, VirtualFunctionTable> map = vftsByOwner.get(owner);
+		List<VirtualFunctionTable> list = vftsByOwner.get(owner);
-		if (map == null) {
+		if (list == null) {
 			return null;
 		}
-		return map.firstEntry().getValue();
+		VirtualFunctionTable result = null;
+		VirtualFunctionTable firstPlaceholder = null;
+		Address addrResult = null;
+		for (VirtualFunctionTable table : list) {
+			if (table instanceof ProgramVirtualFunctionTable pvft) {
+				Address addr = pvft.getAddress();
+				if (addrResult == null || addr.compareTo(addrResult) < 0) {
+					addrResult = addr;
+					result = table;
+				}
+			}
+			else if (firstPlaceholder == null) {
+				firstPlaceholder = table;
+			}
+		}
+		if (result == null) {
+			if (firstPlaceholder != null) {
+				return firstPlaceholder;
+			}
+			result = new PlaceholderVirtualFunctionTable(owner, parentage);
+			list.add(result);
+		}
+		return result;
 	}
 
 	/**
@@ -211,7 +257,7 @@ public class MsftVxtManager extends VxtManager {
 	 * @return the IDs
 	 */
 	public ClassID[] getAllVftOwners() {
-		ClassID ids[] = new ClassID[vbtsByOwner.size()];
+		ClassID ids[] = new ClassID[vftsByOwner.size()];
 		Set<ClassID> set = vftsByOwner.keySet();
 		return set.toArray(ids);
 	}
@@ -222,12 +268,11 @@ public class MsftVxtManager extends VxtManager {
 	 * @return the tables
 	 */
 	public VirtualBaseTable[] getVbts(ClassID owner) {
-		TreeMap<Address, VirtualBaseTable> map = vbtsByOwner.get(owner);
+		List<VirtualBaseTable> list = vbtsByOwner.get(owner);
-		if (map == null) {
+		if (list == null) {
 			return null;
 		}
-		Collection<VirtualBaseTable> values = map.values();
+		return list.toArray(new VirtualBaseTable[list.size()]);
-		return values.toArray(new VirtualBaseTable[values.size()]);
 	}
 
 	/**
@@ -236,12 +281,11 @@ public class MsftVxtManager extends VxtManager {
 	 * @return the tables
 	 */
 	public VirtualFunctionTable[] getVfts(ClassID owner) {
-		TreeMap<Address, VirtualFunctionTable> map = vftsByOwner.get(owner);
+		List<VirtualFunctionTable> list = vftsByOwner.get(owner);
-		if (map == null) {
+		if (list == null) {
 			return null;
 		}
-		Collection<VirtualFunctionTable> values = map.values();
+		return list.toArray(new VirtualFunctionTable[list.size()]);
-		return values.toArray(new VirtualFunctionTable[values.size()]);
 	}
 
 	/**
@@ -264,6 +308,7 @@ public class MsftVxtManager extends VxtManager {
 			vbt = new PlaceholderVirtualBaseTable(owner, parentage);
 		}
 		vbtsByOwnerParentage.put(op, vbt);
+		storeVbt(owner, vbt);
 		return vbt;
 	}
 
@@ -309,6 +354,7 @@ public class MsftVxtManager extends VxtManager {
 			vft = new PlaceholderVirtualFunctionTable(owner, parentage);
 		}
 		vftsByOwnerParentage.put(op, vft);
+		storeVft(owner, vft);
 		return vft;
 	}
 
@@ -417,7 +463,7 @@ public class MsftVxtManager extends VxtManager {
 				}
 				node.setVBTable(prvbt);
 				vbtByAddress.put(address, prvbt);
-				storeVbt(owner, address, prvbt); // temp solution?
+				storeVbt(owner, prvbt); // temp solution?
 				break;
 
 			case VFT:
@@ -429,7 +475,7 @@ public class MsftVxtManager extends VxtManager {
 				}
 				node.setVFTable(vft);
 				vftByAddress.put(address, vft);
-				storeVft(owner, address, vft); // temp solution?
+				storeVft(owner, vft); // temp solution?
 				break;
 
 			default:
@@ -438,22 +484,49 @@ public class MsftVxtManager extends VxtManager {
 		return true;
 	}
 
-	private void storeVbt(ClassID owner, Address address, VirtualBaseTable vbt) {
+	private void storeVbt(ClassID owner, VirtualBaseTable vbt) {
-		TreeMap<Address, VirtualBaseTable> map = vbtsByOwner.get(owner);
+		ClassID own = vbt.getOwner();
-		if (map == null) {
+		List<VirtualBaseTable> list = vbtsByOwner.get(own);
-			map = new TreeMap<>();
+		if (list == null) {
-			vbtsByOwner.put(owner, map);
+			list = new ArrayList<>();
+			vbtsByOwner.put(owner, list);
 		}
-		map.put(address, vbt);
+		List<ClassID> parentage = vbt.getParentage();
+		for (VirtualBaseTable table : list) {
+			if (isEqual(table.getParentage(), parentage)) {
+				return; // return without saving
+			}
+		}
+		list.add(vbt);
 	}
 
-	private void storeVft(ClassID owner, Address address, VirtualFunctionTable vft) {
+	private void storeVft(ClassID owner, VirtualFunctionTable vft) {
-		TreeMap<Address, VirtualFunctionTable> map = vftsByOwner.get(owner);
+		List<VirtualFunctionTable> list = vftsByOwner.get(owner);
-		if (map == null) {
+		if (list == null) {
-			map = new TreeMap<>();
+			list = new ArrayList<>();
-			vftsByOwner.put(owner, map);
+			vftsByOwner.put(owner, list);
 		}
-		map.put(address, vft);
+		List<ClassID> parentage = vft.getParentage();
+		for (VirtualFunctionTable table : list) {
+			if (isEqual(table.getParentage(), parentage)) {
+				return; // return without saving
+			}
+		}
+		list.add(vft);
+	}
+
+	private boolean isEqual(List<ClassID> parentage1, List<ClassID> parentage2) {
+		int diff = parentage1.size() - parentage2.size();
+		if (diff != 0) {
+			return false;
+		}
+		Iterator<ClassID> iter2 = parentage2.iterator();
+		for (ClassID element : parentage1) {
+			if (!element.equals(iter2.next())) {
+				return false;
+			}
+		}
+		return true;
 	}
 
 	private ParentageNode findNode(ClassID owner, List<ClassID> parentage, ParentageNode root) {

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

@@ -1231,19 +1231,25 @@ public class CppCompositeType {
 			// Note that if the parent has already had its layout done, it will not have
 			//  used the vxtManager that we are passing in here; it will have used whatever
 			//  was passed to the layout method for that class
-			for (VxtPtrInfo parentInfo : cppBaseType.getPropagatedSelfBaseVfts()) {
+			if (cppBaseType.getPropagatedSelfBaseVfts() != null) {
-				VxtPtrInfo newInfo =
+				for (VxtPtrInfo parentInfo : cppBaseType.getPropagatedSelfBaseVfts()) {
-					createSelfOwnedDirectVxtPtrInfo(parentInfo, baseId, baseOffset);
+					VxtPtrInfo newInfo =
-				updateVft(vxtManager, baseId, newInfo, parentInfo);
+						createSelfOwnedDirectVxtPtrInfo(parentInfo, baseId, baseOffset);
-				storeVxtInfo(propagatedSelfBaseVfts, finalVftPtrInfoByOffset, vftTableIdByOffset,
+					updateVft(vxtManager, baseId, newInfo, parentInfo);
-					vftOffsetByTableId, newInfo);
+					storeVxtInfo(propagatedSelfBaseVfts, finalVftPtrInfoByOffset,
+						vftTableIdByOffset,
+						vftOffsetByTableId, newInfo);
+				}
 			}
-			for (VxtPtrInfo parentInfo : cppBaseType.getPropagatedSelfBaseVbts()) {
+			if (cppBaseType.getPropagatedSelfBaseVbts() != null) {
-				VxtPtrInfo newInfo =
+				for (VxtPtrInfo parentInfo : cppBaseType.getPropagatedSelfBaseVbts()) {
-					createSelfOwnedDirectVxtPtrInfo(parentInfo, baseId, baseOffset);
+					VxtPtrInfo newInfo =
-				updateVbt(vxtManager, baseId, newInfo, parentInfo);
+						createSelfOwnedDirectVxtPtrInfo(parentInfo, baseId, baseOffset);
-				storeVxtInfo(propagatedSelfBaseVbts, finalVbtPtrInfoByOffset, vbtTableIdByOffset,
+					updateVbt(vxtManager, baseId, newInfo, parentInfo);
-					vbtOffsetByTableId, newInfo);
+					storeVxtInfo(propagatedSelfBaseVbts, finalVbtPtrInfoByOffset,
+						vbtTableIdByOffset,
+						vbtOffsetByTableId, newInfo);
+				}
 			}
 		}
 	}
@@ -1253,8 +1259,9 @@ public class CppCompositeType {
 	 *  virtual bases.  Gathers results from the accumulation of all "direct" virtual base classes;
 	 *  we are not relying on the "indirect" virtual base class information from the PDB.  This
 	 *  is done this way so that we can collect parentage information for the pointers.
+	 * @throws PdbException upon issue finding base offset
 	 */
-	private void findVirtualBaseVxtPtrs(MsftVxtManager vxtManager) {
+	private void findVirtualBaseVxtPtrs(MsftVxtManager vxtManager) throws PdbException {
 		// Walk direct bases to find vxts of virtual bases.  TODO: also notate all rolled up
 		//  virtuals for each direct base.
 		for (DirectLayoutBaseClass base : directLayoutBaseClasses) {
@@ -1396,10 +1403,16 @@ public class CppCompositeType {
 	 * Converts VxtPtrInfo from virtual-based-owned direct or indirect virtual base for this class
 	 * @param baseInfo the vxt info from the base
 	 * @return the new VxtPtrInfo for this class
+	 * @throws PdbException upon issue getting base offset
 	 */
-	private VxtPtrInfo createVirtualOwnedVirtualVxtPtrInfo(VxtPtrInfo baseInfo) {
+	private VxtPtrInfo createVirtualOwnedVirtualVxtPtrInfo(VxtPtrInfo baseInfo)
+			throws PdbException {
 		Long accumOffset = baseInfo.accumOffset();
-		Long finalOffset = accumOffset + baseOffsetById.get(baseInfo.baseId());
+		Long baseOffset = baseOffsetById.get(baseInfo.baseId());
+		if (baseOffset == null) {
+			throw new PdbException("Cannot find base offset");
+		}
+		Long finalOffset = accumOffset + baseOffset;
 		return new VxtPtrInfo(finalOffset, accumOffset, baseInfo.baseId(),
 			updateParentage(baseInfo));
 	}
@@ -1440,20 +1453,10 @@ public class CppCompositeType {
 				myVftPtrOffset = vftPtrTypeByOffset.firstKey();
 				VxtPtrInfo info =
 					new VxtPtrInfo(myVftPtrOffset, myVftPtrOffset, myId, List.of(myId));
-				VirtualFunctionTable myVft =
+				VirtualFunctionTable myVft = vxtManager.findVft(myId, info.parentage());
-					vxtManager.findVft(myId, List.of(myId));
+				myVft.setPtrOffsetInClass(info.finalOffset());
-				if (myVft != null) {
+				propagatedSelfBaseVfts.add(info);
-					myVft.setPtrOffsetInClass(info.finalOffset());
+				finalVftByOffset.put(info.finalOffset(), myVft);
-					propagatedSelfBaseVfts.add(info);
-					finalVftByOffset.put(info.finalOffset(), myVft);
-				}
-				else {
-					PlaceholderVirtualFunctionTable t = new PlaceholderVirtualFunctionTable(
-						myId, List.of(myId));
-					t.setPtrOffsetInClass(info.finalOffset());
-					propagatedSelfBaseVfts.add(info);
-					finalVftByOffset.put(info.finalOffset(), t);
-				}
 				finalVftPtrInfoByOffset.put(info.accumOffset(), info);
 				OwnerParentage op = new OwnerParentage(info.baseId(), info.parentage());
 				vftTableIdByOffset.put(info.accumOffset(), op);
@@ -1473,8 +1476,8 @@ public class CppCompositeType {
 	 * structure
 	 */
 	private void findOrAllocateMainVbtPtr(MsftVxtManager vxtManager) {
-		if (propagatedSelfBaseVbts.isEmpty()) {
+		if (propagatedSelfBaseVbts.isEmpty()) { // a pointer might be available in a direct base
-			if (!virtualLayoutBaseClasses.isEmpty()) {
+			if (!virtualLayoutBaseClasses.isEmpty()) { // there is a need for a main vbtptr
 				TreeSet<Long> vbtOffsets = new TreeSet<>();
 				for (VirtualLayoutBaseClass base : virtualLayoutBaseClasses) {
 					vbtOffsets.add((long) base.getBasePointerOffset());
@@ -1487,12 +1490,10 @@ public class CppCompositeType {
 					Msg.warn(this, "Mismatch vbt location for " + myId);
 				}
 				VxtPtrInfo info = new VxtPtrInfo(vbtPtrOffset, vbtPtrOffset, myId, List.of(myId));
-				VirtualBaseTable myVbt = vxtManager.findVbt(myId, List.of(myId));
+				VirtualBaseTable myVbt = vxtManager.findVbt(myId, info.parentage());
-				if (myVbt != null) {
+				myVbt.setPtrOffsetInClass(info.finalOffset());
-					myVbt.setPtrOffsetInClass(info.finalOffset());
+				propagatedSelfBaseVbts.add(info);
-					propagatedSelfBaseVbts.add(info);
+				finalVbtByOffset.put(info.finalOffset(), myVbt);
-					finalVbtByOffset.put(info.finalOffset(), myVbt);
-				}
 				finalVbtPtrInfoByOffset.put(info.accumOffset(), info);
 				OwnerParentage op = new OwnerParentage(info.baseId(), info.parentage());
 				vbtTableIdByOffset.put(info.accumOffset(), op);
@@ -1699,7 +1700,7 @@ public class CppCompositeType {
 	private VirtualFunctionTable getMainVft(MsftVxtManager vxtManager) throws PdbException {
 		if (!finalVftPtrInfoByOffset.isEmpty()) {
 			VxtPtrInfo firstVftPtrInfo = finalVftPtrInfoByOffset.firstEntry().getValue();
-			VirtualFunctionTable vft = vxtManager.findVft(myId, firstVftPtrInfo.parentage());
+			VirtualFunctionTable vft = vxtManager.findPrimaryVft(myId, firstVftPtrInfo.parentage());
 			return vft;
 			// Following is for consideration for testing without a program:
 //				if (vft instanceof ProgramVirtualFunctionTable pvft) {
@@ -1723,7 +1724,7 @@ public class CppCompositeType {
 	private VirtualBaseTable getMainVbt(MsftVxtManager vxtManager) throws PdbException {
 		if (!finalVbtPtrInfoByOffset.isEmpty()) {
 			VxtPtrInfo firstVbtPtrInfo = finalVbtPtrInfoByOffset.firstEntry().getValue();
-			VirtualBaseTable vbt = vxtManager.findVbt(myId, firstVbtPtrInfo.parentage());
+			VirtualBaseTable vbt = vxtManager.findPrimaryVbt(myId, firstVbtPtrInfo.parentage());
 			if (vbt instanceof ProgramVirtualBaseTable pvbt) {
 				return pvbt;
 			}
@@ -1747,8 +1748,10 @@ public class CppCompositeType {
 				return plvbt;
 			}
 			else {
-				throw new PdbException(
+				if (vbt != null) {
-					"VBT type not expected: " + vbt.getClass().getSimpleName());
+					throw new PdbException(
+						"VBT type not expected: " + vbt.getClass().getSimpleName());
+				}
 			}
 		}
 		return null;