Fix for NeLoader handling of external functions

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/function/CreateThunkFunctionCmd.java

@@ -228,11 +228,6 @@ public class CreateThunkFunctionCmd extends BackgroundCommand {
 			thunkFunction = functionMgr.createThunkFunction(name, namespace, entry, body,
 				referencedFunction, source);
 		}
-		catch (DuplicateNameException e) {
-			Msg.error(this, "Dynamically generated thunk name conlict: " + e.getMessage());
-			setStatusMsg("Thunk name conflict: " + e.getMessage());
-			return false;
-		}
 		catch (OverlappingFunctionException e) {
 			setStatusMsg("Specified body overlaps existing function(s): " + e.getMessage());
 			return false;

Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/NeLoader.java

@@ -30,6 +30,7 @@ import ghidra.app.util.bin.format.ne.Resource;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.app.util.importer.MessageLogContinuesFactory;
 import ghidra.framework.options.Options;
+import ghidra.program.database.function.OverlappingFunctionException;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
 import ghidra.program.model.lang.Register;
@@ -401,54 +402,60 @@ public class NeLoader extends AbstractLibrarySupportLoader {
 			ImportedNameTable imp, Program program, SegmentedAddressSpace space, MessageLog log,
 			TaskMonitor monitor) throws IOException {
 
-		int pointerSize = space.getAddress(0, 0).getPointerSize();
+		int thunkBodySize = 4;
 
-		PointerDataType ptr = new PointerDataType();
+		ExternalManager externalManager = program.getExternalManager();
-		String comment = "";
+		FunctionManager functionManager = program.getFunctionManager();
-		String source = "";
+		Namespace globalNamespace = program.getGlobalNamespace();
-		Listing listing = program.getListing();
-		SymbolTable symbolTable = program.getSymbolTable();
 
 		LengthStringSet[] names = mrt.getNames();
-		for (LengthStringSet name : names) {
+		String[][] mod2proclist = new String[names.length][];
-			String[] callnames = getCallNamesForModule(name.getString(), mrt, st, imp);
+		int length = 0;
-			int length = callnames.length * pointerSize;
+		for (int i = 0; i < names.length; ++i) {
+			String moduleName = names[i].getString();
+			String[] callnames = getCallNamesForModule(moduleName, mrt, st, imp);
+			mod2proclist[i] = callnames;
+			length += callnames.length * thunkBodySize;
+		}
+		if (length == 0) {
+			return;
+		}
 		int segment = space.getNextOpenSegment(program.getMemory().getMaxAddress());
-			Address start = space.getAddress(segment, 0);
+		Address addr = space.getAddress(segment, 0);
-			if (length > 0) {
+		String comment = "";
+		String source = "";
 		// This isn't a real block, just place holder addresses, so don't create an initialized block
-				MemoryBlockUtils.createUninitializedBlock(program, false, name.getString(), start,
+		MemoryBlockUtils.createUninitializedBlock(program, false, MemoryBlock.EXTERNAL_BLOCK_NAME,
-					length, comment, source, true, false, false, log);
+			addr, length, comment, source, true, false, false, log);
-			}
-			Address addr = start;
-			for (String callname : callnames) {
-				try {
-					listing.createData(addr, ptr, pointerSize);
-				}
-				catch (CodeUnitInsertionException e) {
-					log.appendMsg(e.getMessage() + "\n");
-					continue;
-				}
-				catch (DataTypeConflictException e) {
-					log.appendMsg(e.getMessage() + "\n");
-					continue;
-				}
-				try {
 
-					program.getReferenceManager().addExternalReference(addr, name.getString(),
+		for (int i = 0; i < names.length; ++i) {
-						callname, null, SourceType.IMPORTED, 0, RefType.EXTERNAL_REF);
+			String moduleName = names[i].getString();
-					symbolTable.createLabel(addr, name.getString() + "_" + callname,
+			String[] callnames = mod2proclist[i];
-						SourceType.IMPORTED);
+			for (String callname : callnames) {
+				Function refFunction = null;
+				try {
+					ExternalLocation loc;
+					loc = externalManager.addExtFunction(moduleName, callname, null, SourceType.IMPORTED);
+					refFunction = loc.getFunction();
 				}
 				catch (DuplicateNameException e) {
-					log.appendMsg(e.getMessage() + "\n");
+					log.appendMsg(e.getMessage() + '\n');
 					continue;
 				}
 				catch (InvalidInputException e) {
-					log.appendMsg(e.getMessage() + "\n");
+					log.appendMsg(e.getMessage() + '\n');
 					continue;
 				}
-				addr = addr.addWrap(pointerSize);
+				AddressSet body = new AddressSet();
+				body.add(addr, addr.add(thunkBodySize - 1));
+				try {
+					functionManager.createThunkFunction(null, globalNamespace, addr, body,
+						refFunction, SourceType.IMPORTED);
+				}
+				catch (OverlappingFunctionException e) {
+					log.appendMsg(e.getMessage() + '\n');
+				}
+				addr = addr.addWrap(thunkBodySize);
 			}
 		}
 	}
@@ -555,9 +562,28 @@ public class NeLoader extends AbstractLibrarySupportLoader {
 		createSymbols(rnt.getNames(), symbolTable);
 	}
 
+	private SegmentedAddress getImportSymbolByName(SymbolTable symbolTable, String modname,
+			String procname) {
+		Namespace libnamespace = symbolTable.getNamespace(modname, null);
+		List<Symbol> symbols = symbolTable.getSymbols(procname, libnamespace);
+		if (symbols.isEmpty()) {
+			return null;
+		}
+		Symbol symbol = symbols.get(0);
+		if (symbol.getSymbolType() == SymbolType.FUNCTION && symbol.isExternal()) {
+			Function func = (Function) symbol.getObject();
+			Address[] thunkAddresses = func.getFunctionThunkAddresses();
+			if (thunkAddresses != null && thunkAddresses.length != 0) {
+				return (SegmentedAddress) thunkAddresses[0];
+			}
+		}
+		return null;
+	}
+
 	private void processRelocations(SegmentTable st, ImportedNameTable imp,
 			ModuleReferenceTable mrt, RelocationTable relocTable, Program program, Memory memory,
 			SegmentedAddressSpace space, MessageLog log, TaskMonitor monitor) throws IOException {
+		SymbolTable symbolTable = program.getSymbolTable();
 		Segment[] segments = st.getSegments();
 		for (int s = 0; s < segments.length; ++s) {
 			if (monitor.isCancelled()) {
@@ -586,17 +612,13 @@ public class NeLoader extends AbstractLibrarySupportLoader {
 				else if (reloc.isImportName()) {
 					String modname = getRelocationModuleName(mrt, reloc);
 					String procname = imp.getNameAt(reloc.getTargetOffset()).getString();
-					Symbol symbol = SymbolUtilities.getLabelOrFunctionSymbol(program,
+					relocAddr = getImportSymbolByName(symbolTable, modname, procname);
-						modname + "_" + procname, err -> log.error("NE", err));
-					relocAddr = symbol == null ? null : (SegmentedAddress) symbol.getAddress();
 				}
 				else if (reloc.isImportOrdinal()) {
 					String modname = getRelocationModuleName(mrt, reloc);
 					int ordinal = Conv.shortToInt(reloc.getTargetOffset());
-					Symbol symbol = SymbolUtilities.getLabelOrFunctionSymbol(program,
+					String procname = SymbolUtilities.ORDINAL_PREFIX + ordinal;
-						modname + "_" + SymbolUtilities.ORDINAL_PREFIX + ordinal,
+					relocAddr = getImportSymbolByName(symbolTable, modname, procname);
-						err -> log.error("NE", err));
-					relocAddr = symbol == null ? null : (SegmentedAddress) symbol.getAddress();
 				}
 				else if (reloc.isOpSysFixup()) {
 					// short fixupType = relocs[r].getTargetSegment();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/function/FunctionManagerDB.java

@@ -326,7 +326,7 @@ public class FunctionManagerDB implements ManagerDB, FunctionManager {
 	@Override
 	public Function createThunkFunction(String name, Namespace nameSpace, Address entryPoint,
 			AddressSetView body, Function thunkedFunction, SourceType source)
-			throws DuplicateNameException, OverlappingFunctionException {
+			throws OverlappingFunctionException {
 		try {
 			return createFunction(name, nameSpace, entryPoint, body, thunkedFunction, source);
 		}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/listing/FunctionManager.java

@@ -107,13 +107,12 @@ public interface FunctionManager {
 	 * @param thunkedFunction referenced function (required is creating a thunk function)
 	 * @param source the source of this function
 	 * @return new function or null if one or more functions overlap the specified body address set.
-	 * @throws DuplicateNameException if a function with the same thunk name already exists
 	 * @throws OverlappingFunctionException if the address set of the body overlaps an existing
 	 *             function
 	 */
 	public Function createThunkFunction(String name, Namespace nameSpace, Address entryPoint,
 			AddressSetView body, Function thunkedFunction, SourceType source)
-			throws DuplicateNameException, OverlappingFunctionException;
+			throws OverlappingFunctionException;
 
 	/**
 	 * Returns the total number of functions in the program including external functions.

Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/program/model/FunctionManagerTestDouble.java

@@ -27,7 +27,8 @@ import ghidra.program.model.lang.PrototypeModel;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.symbol.Namespace;
 import ghidra.program.model.symbol.SourceType;
-import ghidra.util.exception.*;
+import ghidra.util.exception.CancelledException;
+import ghidra.util.exception.InvalidInputException;
 import ghidra.util.task.TaskMonitor;
 
 public class FunctionManagerTestDouble implements FunctionManager {
@@ -73,7 +74,7 @@ public class FunctionManagerTestDouble implements FunctionManager {
 	@Override
 	public Function createThunkFunction(String name, Namespace nameSpace, Address entryPoint,
 			AddressSetView body, Function thunkedFunction, SourceType source)
-			throws DuplicateNameException, OverlappingFunctionException {
+			throws OverlappingFunctionException {
 		throw new UnsupportedOperationException();
 	}