From bf43007117eda735b3b1ce7dabf6ae71928b95f0 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Mon, 7 Dec 2020 15:55:25 -0500 Subject: [PATCH] Fixing some silent exceptions in FillOutStructureCmd --- .../Decompiler/src/main/doc/pcoderef.xml | 2 +- .../actions/FillOutStructureCmd.java | 94 +++++++------------ .../languages/html/pcodedescription.html | 2 +- 3 files changed, 36 insertions(+), 62 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/main/doc/pcoderef.xml b/Ghidra/Features/Decompiler/src/main/doc/pcoderef.xml index 00c452a1c5..70f219d2f6 100644 --- a/Ghidra/Features/Decompiler/src/main/doc/pcoderef.xml +++ b/Ghidra/Features/Decompiler/src/main/doc/pcoderef.xml @@ -411,7 +411,7 @@ input0 and output must be the same. This instruction loads data from a dynamic location into the output variable by dereferencing a pointer. The “pointer” comes in two pieces. One piece, input1, is a normal variable containing the offset -of the object being pointed at. The other piece, input1, is a constant +of the object being pointed at. The other piece, input0, is a constant indicating the space into which the offset applies. The data in input1 is interpreted as an unsigned offset and should have the same size as the space referred to by the ID, i.e. a 4-byte address space requires diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FillOutStructureCmd.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FillOutStructureCmd.java index 590f5d4a77..ea02b7458f 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FillOutStructureCmd.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FillOutStructureCmd.java @@ -67,7 +67,7 @@ public class FillOutStructureCmd extends BackgroundCommand { private int maxCallDepth = 1; private NoisyStructureBuilder componentMap = new NoisyStructureBuilder(); - private HashMap addressToCallInputMap = new HashMap<>(); + private HashMap addressToCallInputMap = new HashMap<>(); private Program currentProgram; private ProgramLocation currentLocation; @@ -147,12 +147,11 @@ public class FillOutStructureCmd extends BackgroundCommand { } fillOutStructureDef(var); + pushIntoCalls(); structDT = createStructure(structDT, var, rootFunction, isThisParam); populateStructure(structDT); - pushIntoCalls(structDT); - DataType pointerDT = new PointerDataType(structDT); // Delay adding to the manager until full structure is accumulated @@ -171,33 +170,6 @@ public class FillOutStructureCmd extends BackgroundCommand { return true; } - /** - * Method to create a structure data type for a variable in the given function. - * @param var a parameter, local variable, or global variable used in the given function - * @param function the function to process - * @return a filled-in structure or null if one could not be created - */ - public Structure processStructure(HighVariable var, Function function) { - - if (var == null || var.getSymbol() == null || var.getOffset() >= 0) { - return null; - } - - Structure structDT; - - try { - fillOutStructureDef(var); - structDT = createStructure(null, var, function, false); - populateStructure(structDT); - pushIntoCalls(structDT); - } - catch (Exception e) { - return null; - } - - return structDT; - } - /** * Retrieve the component map that was generated when structure was created using decomiler info * @return componentMap @@ -226,16 +198,24 @@ public class FillOutStructureCmd extends BackgroundCommand { /** - * Retrieve the (likely) storage address for a function parameter given its index - * @param function is the function - * @param paramIndex is the index of the parameter - * @param pointerDt is the pointer to structure datatype - * @return the storage address or null + * Retrieve the (likely) storage address of a function parameter given + * the inputs to a CALL p-code op and particular Varnode slot within the inputs. + * We compute the address from the point of view of the called function (callee) + * which may be different from the point of view of the caller, which may be + * different from the address of the Varnode currently holding the parameter. + * @param inputs is the array of Varnode inputs to the CALL + * @param slot is the index of the Varnode holding the parameter we want. + * @return the starting address of the parameter or null if the address can't be identified */ - private Address computeParamAddress(Function function, int paramIndex, DataType pointerDt) { + private Address computeParamAddress(Varnode[] inputs, int slot) { + Address funcAddr = inputs[0].getAddress(); + Function function = currentProgram.getFunctionManager().getFunctionAt(funcAddr); + if (function == null) { + return null; + } Parameter[] parameters = function.getParameters(); - if (paramIndex < parameters.length) { - return parameters[paramIndex].getMinAddress(); + if (slot - 1 < parameters.length) { + return parameters[slot - 1].getMinAddress(); } PrototypeModel model = function.getCallingConvention(); if (model == null) { @@ -244,26 +224,29 @@ public class FillOutStructureCmd extends BackgroundCommand { return null; } } - VariableStorage argLocation = - model.getArgLocation(paramIndex, null, pointerDt, currentProgram); - return argLocation.getMinAddress(); + DataType typeList[] = new DataType[slot + 1]; + typeList[0] = DataType.DEFAULT; // Default function return data-type + for (int i = 1; i < slot + 1; ++i) { + typeList[i] = inputs[i].getHigh().getDataType(); + } + VariableStorage[] storageLocations = + model.getStorageLocations(currentProgram, typeList, false); + return storageLocations[slot].getMinAddress(); } /** * Recursively visit calls that take the structure pointer as a parameter. * Add any new references to the offsetToDataTypeMap. - * @param structDT is the structure to populate */ - private void pushIntoCalls(Structure structDT) { + private void pushIntoCalls() { AddressSet doneSet = new AddressSet(); - DataType pointerDT = new PointerDataType(structDT); while (addressToCallInputMap.size() > 0) { currentCallDepth += 1; if (currentCallDepth > maxCallDepth) { return; } - HashMap savedList = addressToCallInputMap; + HashMap savedList = addressToCallInputMap; addressToCallInputMap = new HashMap<>(); Set
keys = savedList.keySet(); Iterator
keyIter = keys.iterator(); @@ -275,12 +258,10 @@ public class FillOutStructureCmd extends BackgroundCommand { } doneSet.addRange(addr, addr); Function func = currentProgram.getFunctionManager().getFunctionAt(addr); - int paramIndex = savedList.get(addr); - Address storageAddr = computeParamAddress(func, paramIndex, pointerDT); + Address storageAddr = savedList.get(addr); HighVariable paramHighVar = computeHighVariable(storageAddr, func); if (paramHighVar != null) { fillOutStructureDef(paramHighVar); - populateStructure(structDT); } } } @@ -621,7 +602,7 @@ public class FillOutStructureCmd extends BackgroundCommand { for (Varnode vn : instances) { doneList.add(vn); // Mark instances as done to avoid recursion issues if (vn != startVN) { - todoList.add(new PointerRef(startVN, 0)); // Make sure all instances are on the todo list + todoList.add(new PointerRef(vn, 0)); // Make sure all instances are on the todo list } } @@ -724,7 +705,10 @@ public class FillOutStructureCmd extends BackgroundCommand { // find it as an input int slot = pcodeOp.getSlot(currentRef.varnode); if (slot > 0 && slot < pcodeOp.getNumInputs()) { - putOnCallParamList(inputs[0].getAddress(), slot - 1); + Address storageAddr = computeParamAddress(inputs, slot); + if (storageAddr != null) { + addressToCallInputMap.put(inputs[0].getAddress(), storageAddr); + } } } else { @@ -742,16 +726,6 @@ public class FillOutStructureCmd extends BackgroundCommand { } } - /** - * Note that flow has hit a CALL instruction at a particular input parameter so that - * pushIntoCalls() can recurse into the call. - * @param address is the destination of the CALL - * @param j is the parameter index where flow hit - */ - private void putOnCallParamList(Address address, int j) { - addressToCallInputMap.put(address, j); - } - private long getSigned(Varnode varnode) { long mask = 0x80L << ((varnode.getSize() - 1) * 8); long value = varnode.getOffset(); diff --git a/GhidraDocs/languages/html/pcodedescription.html b/GhidraDocs/languages/html/pcodedescription.html index b1d2eb32dc..6f038f2318 100644 --- a/GhidraDocs/languages/html/pcodedescription.html +++ b/GhidraDocs/languages/html/pcodedescription.html @@ -123,7 +123,7 @@ input0 and output must be the same. This instruction loads data from a dynamic location into the output variable by dereferencing a pointer. The “pointer” comes in two pieces. One piece, input1, is a normal variable containing the offset -of the object being pointed at. The other piece, input1, is a constant +of the object being pointed at. The other piece, input0, is a constant indicating the space into which the offset applies. The data in input1 is interpreted as an unsigned offset and should have the same size as the space referred to by the ID, i.e. a 4-byte address space requires