diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/nav/Navigatable.java b/Ghidra/Features/Base/src/main/java/ghidra/app/nav/Navigatable.java index cb5734d784..a2fa898406 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/nav/Navigatable.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/nav/Navigatable.java @@ -35,6 +35,7 @@ public interface Navigatable { /** * Commands this navigatable to goto (display) the given program and location + * @param program the program * * @param location the location in that program to display * @return true if the goto was successful diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java index 606ce532e3..79202a7e6e 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java @@ -217,7 +217,7 @@ public class AddressTable { * @param end end index (inclusive) * @param createIndex don't create index if false * @param autoLabel true if labels should be created on the table - * @return + * @return true if tablecreated else false */ public boolean makeTable(Program program, int start, int end, boolean createIndex, boolean autoLabel) { @@ -236,13 +236,17 @@ public class AddressTable { //TODO: Do I need to do something special for the 3 byte pointers or will it know // how to make it automatically? + DataTypeManager dtm = program.getDataTypeManager(); if (shiftedAddr) { adt = ShiftedAddressDataType.dataType; } - else { - adt = new PointerDataType(DataType.DEFAULT, addrSize); + else if (addrSize == program.getDefaultPointerSize()) { + adt = new PointerDataType(DataType.DEFAULT, dtm); } - adt = program.getDataTypeManager().resolve(adt, null); + else { + adt = new PointerDataType(DataType.DEFAULT, addrSize, dtm); + } + adt = dtm.resolve(adt, null); Address newAddress = currentAddress; diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleHost.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleHost.java index 8b30a4bdf1..5e1b57d316 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleHost.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleHost.java @@ -691,15 +691,9 @@ public class BundleHost { public void activateAll(Collection bundles, TaskMonitor monitor, PrintWriter console) { - List availableBundles = getGhidraBundles().stream() - .filter(GhidraBundle::isEnabled) - .collect(Collectors.toList()); - - BundleDependencyGraph dependencyGraph = - new BundleDependencyGraph(availableBundles, bundles, monitor); + BundleDependencyGraph dependencyGraph = new BundleDependencyGraph(bundles, monitor); monitor.setMaximum(dependencyGraph.vertexSet().size()); - for (GhidraBundle bundle : dependencyGraph.inTopologicalOrder()) { if (monitor.isCancelled()) { break; @@ -728,21 +722,25 @@ public class BundleHost { */ public void activateInStages(Collection bundles, TaskMonitor monitor, PrintWriter console) { - List bundlesRemaining = new ArrayList<>(bundles); + Map> requirementMap = new HashMap<>(); + for (GhidraBundle bundle : bundles) { + try { + requirementMap.put(bundle, bundle.getAllRequirements()); + } + catch (GhidraBundleException e) { + fireBundleException(e); + } + } + List bundlesRemaining = new ArrayList<>(requirementMap.keySet()); monitor.setMaximum(bundlesRemaining.size()); while (!bundlesRemaining.isEmpty() && !monitor.isCancelled()) { - List resolvableBundles = bundlesRemaining.stream().filter(bundle -> { - try { - return canResolveAll(bundle.getAllRequirements()); - } - catch (GhidraBundleException e) { - // failure in last round will set fireBundleException - } - return false; - }).collect(Collectors.toList()); + List resolvableBundles = bundlesRemaining.stream() + .filter(bundle -> canResolveAll(requirementMap.get(bundle))) + .collect(Collectors.toList()); if (resolvableBundles.isEmpty()) { - // final round, try everything we couldn't resolve to generate errors + // final round, try everything that hasn't already been eliminated. + // this can generate helpful error messages resolvableBundles = bundlesRemaining; bundlesRemaining = Collections.emptyList(); } @@ -927,18 +925,29 @@ public class BundleHost { // exists only to be distinguished by id } - private static class BundleDependencyGraph - extends DirectedMultigraph { + /** + * Utility class to build a dependency graph from bundles where capabilities map to requirements. + */ + private class BundleDependencyGraph extends DirectedMultigraph { final Map> capabilityMap = new HashMap<>(); final List availableBundles; final TaskMonitor monitor; - BundleDependencyGraph(List availableBundles, - Collection startingBundles, TaskMonitor monitor) { + BundleDependencyGraph(Collection startingBundles, TaskMonitor monitor) { super(null, null, false); - this.availableBundles = availableBundles; this.monitor = monitor; + // maintain a list of bundles available for resolution, starting with all of the enabled bundles + this.availableBundles = new ArrayList<>(); + for (GhidraBundle bundle : getGhidraBundles()) { + if (bundle.isEnabled()) { + addToAvailable(bundle); + } + } + // An edge A->B indicates that the capabilities of A resolve some requirement(s) of B + + // "front" accumulates bundles and links to bundles already in the graph that they provide capabilities for. + // e.g. if front[A]=[B,...] then A->B, B is already in the graph, and we will add A next iteration. Map> front = new HashMap<>(); for (GhidraBundle bundle : startingBundles) { front.put(bundle, null); @@ -951,6 +960,7 @@ public class BundleHost { for (GhidraBundle bundle : front.keySet()) { resolve(bundle, newFront); } + handleBackEdges(newFront); front = newFront; } @@ -969,7 +979,9 @@ public class BundleHost { GhidraBundle source = entry.getKey(); if (containsVertex(source)) { for (GhidraBundle destination : entry.getValue()) { - addEdge(source, destination, new Dependency()); + if (source != destination) { + addEdge(source, destination, new Dependency()); + } } newFrontIter.remove(); } @@ -979,31 +991,48 @@ public class BundleHost { void addFront(Map> front) { for (Entry> e : front.entrySet()) { GhidraBundle source = e.getKey(); - availableBundles.add(source); - addVertex(source); - Set destinations = e.getValue(); - if (destinations != null) { - for (GhidraBundle destination : destinations) { - addEdge(source, destination, new Dependency()); + if (addToAvailable(source)) { + addVertex(source); + Set destinations = e.getValue(); + if (destinations != null) { + for (GhidraBundle destination : destinations) { + addEdge(source, destination, new Dependency()); + } } } } } + boolean addToAvailable(GhidraBundle bundle) { + try { + capabilityMap.put(bundle, bundle.getAllCapabilities()); + availableBundles.add(bundle); + return true; + } + catch (GhidraBundleException ex) { + fireBundleException(ex); + return false; + } + } + + // populate newFront with edges depBundle -> bundle, + // where depBundle has a capability that resolves a requirement of bundle void resolve(GhidraBundle bundle, Map> newFront) { List requirements; try { requirements = new ArrayList<>(bundle.getAllRequirements()); + if (requirements.isEmpty()) { + return; + } } catch (GhidraBundleException e) { - throw new RuntimeException(e); - } - if (requirements.isEmpty()) { + fireBundleException(e); + removeVertex(bundle); return; } for (GhidraBundle depBundle : availableBundles) { - for (BundleCapability capability : getCapabilities(depBundle)) { + for (BundleCapability capability : capabilityMap.get(depBundle)) { if (monitor.isCancelled()) { return; } @@ -1020,18 +1049,8 @@ public class BundleHost { } } } - } - - List getCapabilities(GhidraBundle bundle) { - return capabilityMap.computeIfAbsent(bundle, b -> { - try { - return b.getAllCapabilities(); - } - catch (GhidraBundleException e) { - Msg.error(this, "getting capabilities", e); - return null; - } - }); + // if requirements remain, some will be resolved by system + // and others will generate helpful errors for the user during activation } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleStatusComponentProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleStatusComponentProvider.java index 95fe3c29c5..3125ae9df7 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleStatusComponentProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/BundleStatusComponentProvider.java @@ -36,8 +36,7 @@ import ghidra.app.services.ConsoleService; import ghidra.framework.plugintool.ComponentProviderAdapter; import ghidra.framework.plugintool.PluginTool; import ghidra.framework.preferences.Preferences; -import ghidra.util.HelpLocation; -import ghidra.util.Msg; +import ghidra.util.*; import ghidra.util.exception.CancelledException; import ghidra.util.filechooser.GhidraFileChooserModel; import ghidra.util.filechooser.GhidraFileFilter; @@ -321,14 +320,18 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter { } private void notifyTableRowChanged(BundleStatus status) { - int modelRowIndex = bundleStatusTableModel.getRowIndex(status); - int viewRowIndex = filterPanel.getViewRow(modelRowIndex); - bundleStatusTable - .notifyTableChanged(new TableModelEvent(bundleStatusTableModel, viewRowIndex)); + Swing.runIfSwingOrRunLater(() -> { + int modelRowIndex = bundleStatusTableModel.getRowIndex(status); + int viewRowIndex = filterPanel.getViewRow(modelRowIndex); + bundleStatusTable + .notifyTableChanged(new TableModelEvent(bundleStatusTableModel, viewRowIndex)); + }); } private void notifyTableDataChanged() { - bundleStatusTable.notifyTableChanged(new TableModelEvent(bundleStatusTableModel)); + Swing.runIfSwingOrRunLater(() -> { + bundleStatusTable.notifyTableChanged(new TableModelEvent(bundleStatusTableModel)); + }); } @Override @@ -374,24 +377,22 @@ public class BundleStatusComponentProvider extends ComponentProviderAdapter { @Override public void run(TaskMonitor monitor) throws CancelledException { deactivateBundlesTask.run(monitor); - if (!monitor.isCancelled()) { - // partition bundles into system (bundles.get(true)) and non-system (bundles.get(false)). - Map> bundles = statuses.stream() - .map(bs -> bundleHost.getExistingGhidraBundle(bs.getFile())) - .collect(Collectors.partitioningBy(GhidraBundle::isSystemBundle)); + monitor.checkCanceled(); + // partition bundles into system (bundles.get(true)) and non-system (bundles.get(false)). + Map> bundles = statuses.stream() + .map(bs -> bundleHost.getExistingGhidraBundle(bs.getFile())) + .collect(Collectors.partitioningBy(GhidraBundle::isSystemBundle)); - List systemBundles = bundles.get(true); - if (!systemBundles.isEmpty()) { - StringBuilder stringBuilder = new StringBuilder(); - for (GhidraBundle bundle : systemBundles) { - stringBuilder.append(Path.toPathString(bundle.getFile()) + "\n"); - } - Msg.showWarn(this, BundleStatusComponentProvider.this.getComponent(), - "Unabled to remove", - "System bundles cannot be removed:\n" + stringBuilder.toString()); + List systemBundles = bundles.get(true); + if (!systemBundles.isEmpty()) { + StringBuilder buff = new StringBuilder(); + for (GhidraBundle bundle : systemBundles) { + buff.append(Path.toPathString(bundle.getFile()) + "\n"); } - bundleHost.remove(bundles.get(false)); + Msg.showWarn(this, BundleStatusComponentProvider.this.getComponent(), + "Unabled to remove", "System bundles cannot be removed:\n" + buff.toString()); } + bundleHost.remove(bundles.get(false)); } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraBundleException.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraBundleException.java index 36681948a3..1f9ec1cf1a 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraBundleException.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraBundleException.java @@ -53,6 +53,18 @@ public class GhidraBundleException extends OSGiException { this.bundleLocation = bundleLocation; } + /** + * Construct a new exception originating with the bundle having location identifier {@code bundleLocation}. + * + * @param bundleLocation the bundle location identifier (since no bundle is available) + * @param msg a contextual message + */ + public GhidraBundleException(String bundleLocation, String msg) { + super(msg); + this.bundle = null; + this.bundleLocation = bundleLocation; + } + /** * @return the associated bundle, or null. If null, the bundle location identifier will be non-null */ diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraJarBundle.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraJarBundle.java index 6076abde3e..918ee8836a 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraJarBundle.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraJarBundle.java @@ -23,11 +23,13 @@ import java.util.jar.Manifest; import java.util.stream.Collectors; import org.apache.felix.framework.util.manifestparser.ManifestParser; +import org.osgi.framework.BundleException; import org.osgi.framework.wiring.BundleCapability; import org.osgi.framework.wiring.BundleRequirement; import aQute.bnd.osgi.Jar; import generic.jar.ResourceFile; +import ghidra.util.exception.AssertException; /** * Proxy to an ordinary OSGi Jar bundle. {@link GhidraJarBundle#build(PrintWriter)} does nothing. @@ -64,9 +66,12 @@ public class GhidraJarBundle extends GhidraBundle { return bundleLocation; } - protected ManifestParser createManifestParser() { + protected ManifestParser createManifestParser() throws GhidraBundleException { try (Jar jar = new Jar(file.getFile(true))) { Manifest manifest = jar.getManifest(); + if (manifest == null) { + throw new GhidraBundleException(bundleLocation, "jar bundle with no manifest"); + } Attributes mainAttributes = manifest.getMainAttributes(); Map headerMap = mainAttributes.entrySet() .stream() @@ -74,8 +79,14 @@ public class GhidraJarBundle extends GhidraBundle { Collectors.toMap(e -> e.getKey().toString(), e -> e.getValue().toString())); return new ManifestParser(null, null, null, headerMap); } + catch (BundleException e) { + throw new GhidraBundleException(bundleLocation, "parsing manifest", e); + } + catch (GhidraBundleException e) { + throw e; + } catch (Exception e) { - throw new RuntimeException(e); + throw new AssertException("Unexpected exception while parsing manifest", e); } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java index 693cce573b..241135958a 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/osgi/GhidraSourceBundle.java @@ -713,11 +713,31 @@ public class GhidraSourceBundle extends GhidraBundle { } } catch (Throwable e) { - Msg.error(this, "Exception searching ", e); - e.printStackTrace(); + Msg.error(this, "Exception while searching for file system discrepancies ", e); } } + /* requirements that resolve internally are never "missing", but will only resolve _after_ build/install */ + private boolean resolveInternally(List requirements) + throws GhidraBundleException { + if (requirements.isEmpty()) { + return true; + } + List capabilities = getAllCapabilities(); + Iterator requirementIterator = requirements.iterator(); + boolean anyMissing = false; + while (requirementIterator.hasNext()) { + BundleRequirement requirement = requirementIterator.next(); + if (capabilities.stream().anyMatch(requirement::matches)) { + requirementIterator.remove(); + } + else { + anyMissing = true; + } + } + return !anyMissing; + } + /* * when calling the java compiler programmatically, we map import requests to files with * a custom {@link JavaFileManager}. We wrap the system JavaFileManager with one that @@ -740,7 +760,7 @@ public class GhidraSourceBundle extends GhidraBundle { List requirements = getAllRequirements(); List bundleWirings = bundleHost.resolve(requirements); - if (!requirements.isEmpty()) { + if (!resolveInternally(requirements)) { writer.printf("%d import requirement%s remain%s unresolved:\n", requirements.size(), requirements.size() > 1 ? "s" : "", requirements.size() > 1 ? "" : "s"); for (BundleRequirement requirement : requirements) { @@ -1097,8 +1117,7 @@ public class GhidraSourceBundle extends GhidraBundle { } try (Stream paths = Files.list(directory)) { - classToClassFilesMap = paths - .filter(p -> Files.isRegularFile(p)) + classToClassFilesMap = paths.filter(p -> Files.isRegularFile(p)) .filter(p -> p.getFileName().toString().endsWith(".class")) .collect(groupingBy(this::getClassName)); } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfDefaultGotPltMarkup.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfDefaultGotPltMarkup.java index 9ad6a24656..8b185e485d 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfDefaultGotPltMarkup.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfDefaultGotPltMarkup.java @@ -122,20 +122,37 @@ public class ElfDefaultGotPltMarkup { if (relocationTable == null) { return; } - ElfRelocation[] relocations = relocationTable.getRelocations(); - int count = relocations.length; - // First few entries of GOT do not correspond to dynamic symbols. - // First relocation address must be used to calculate GOT end address - // based upon the total number of relocation entries. - long firstGotEntryOffset = relocations[0].getOffset() + imageBaseAdj; + // External dynamic symbol entries in the PLTGOT, if any, will be placed + // after any local symbol entries. + + // While DT_PLTGOT identifies the start of the PLTGOT it does not + // specify its length. If there are dynamic non-local entries in the + // PLTGOT they should have relocation entries in the table identified + // by DT_JMPREL. It is important to note that this relocation table + // can include entries which affect other processor-specific PLTGOT + // tables (e.g., MIPS_PLTGOT) so we must attempt to isolate the + // entries which correspond to DT_PLTGOT. + + // WARNING: This implementation makes a potentially bad assumption that + // the last relocation entry will identify the endof the PLTGOT if its + // offset is beyond the start of the PLTGOT. This assumption could + // easily be violated by a processor-specific PLTGOT which falls after + // the standard PLTGOT in memory and shares the same relocation table. long pltgot = elf.adjustAddressForPrelink( - dynamicTable.getDynamicValue(ElfDynamicType.DT_PLTGOT)) + imageBaseAdj; + dynamicTable.getDynamicValue(ElfDynamicType.DT_PLTGOT)); + + ElfRelocation[] relocations = relocationTable.getRelocations(); + + long lastGotOffset = relocations[relocations.length - 1].getOffset(); + if (lastGotOffset < pltgot) { + return; + } + + Address gotStart = defaultSpace.getAddress(pltgot + imageBaseAdj); + Address gotEnd = defaultSpace.getAddress(lastGotOffset + imageBaseAdj); - Address gotStart = defaultSpace.getAddress(pltgot); - Address gotEnd = defaultSpace.getAddress( - firstGotEntryOffset + (count * defaultSpace.getPointerSize()) - 1); processGOT(gotStart, gotEnd, monitor); processDynamicPLT(gotStart, gotEnd, monitor); } @@ -162,7 +179,7 @@ public class ElfDefaultGotPltMarkup { try { Address newImageBase = null; - while (gotStart.compareTo(gotEnd) < 0) { + while (gotStart.compareTo(gotEnd) <= 0) { monitor.checkCanceled(); Data data = createPointer(gotStart, true); @@ -350,7 +367,7 @@ public class ElfDefaultGotPltMarkup { return null; } int pointerSize = program.getDataTypeManager().getDataOrganization().getPointerSize(); - Pointer pointer = PointerDataType.dataType; + Pointer pointer = PointerDataType.dataType.clone(program.getDataTypeManager()); if (elf.is32Bit() && pointerSize != 4) { pointer = Pointer32DataType.dataType; } @@ -364,20 +381,50 @@ public class ElfDefaultGotPltMarkup { } data = listing.createData(addr, pointer); } - Address refAddr = (Address) data.getValue(); - if (keepRefWhenValid) { - if (memory.contains(refAddr)) { - return data; - } - Symbol syms[] = program.getSymbolTable().getSymbols(refAddr); - if (syms != null && syms.length > 0 && syms[0].getSource() != SourceType.DEFAULT) { - return data; - } + if (keepRefWhenValid && isValidPointer(data)) { + setConstant(data); + } + else { + removeMemRefs(data); } - removeMemRefs(data); return data; } + /** + * Set specified data as constant if contained within a writable block. It can be helpful + * to the decompiler results if constant pointers are marked as such (e.g., GOT entries) + * @param data program data + */ + public static void setConstant(Data data) { + Memory memory = data.getProgram().getMemory(); + MemoryBlock block = memory.getBlock(data.getAddress()); + if (!block.isWrite() || block.getName().startsWith(ElfSectionHeaderConstants.dot_got)) { + // .got blocks will be force to read-only by ElfDefaultGotPltMarkup + return; + } + data.setLong(MutabilitySettingsDefinition.MUTABILITY, + MutabilitySettingsDefinition.CONSTANT); + } + + /** + * Determine if pointerData refers to a valid memory address or symbol + * @param pointerData pointer data + * @return true if pointer data refers to valid memory address or symbol + */ + public static boolean isValidPointer(Data pointerData) { + Program program = pointerData.getProgram(); + Memory memory = program.getMemory(); + Address refAddr = (Address) pointerData.getValue(); + if (memory.contains(refAddr)) { + return true; + } + Symbol syms[] = program.getSymbolTable().getSymbols(refAddr); + if (syms != null && syms.length > 0 && syms[0].getSource() != SourceType.DEFAULT) { + return true; + } + return false; + } + private void removeMemRefs(Data data) { if (data != null) { Reference[] refs = data.getValueReferences(); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/navigation/GoToServiceImpl.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/navigation/GoToServiceImpl.java index 897002a98d..3f56cc5e88 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/navigation/GoToServiceImpl.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/navigation/GoToServiceImpl.java @@ -105,7 +105,7 @@ public class GoToServiceImpl implements GoToService { programLocation = override.goTo(goToAddress); } if (programLocation == null) { - programLocation = helper.getProgramLocationForAddress(goToAddress, program); + programLocation = GoToHelper.getProgramLocationForAddress(goToAddress, program); } else { program = programLocation.getProgram(); @@ -116,7 +116,7 @@ public class GoToServiceImpl implements GoToService { @Override public boolean goTo(Address goToAddress, Program program) { - ProgramLocation location = helper.getProgramLocationForAddress(goToAddress, program); + ProgramLocation location = GoToHelper.getProgramLocationForAddress(goToAddress, program); return helper.goTo(defaultNavigatable, location, program); } diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java index 8358fd236c..2edf4ce150 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java @@ -100,9 +100,6 @@ public abstract class AbstractDecompilerAction extends DockingAction { HighVariable variable = token.getHighVariable(); HighSymbol highSymbol = null; if (variable == null) { - if (highFunction == null) { - return null; - } // Token may be from a variable reference, in which case we have to dig to find the actual symbol Function function = highFunction.getFunction(); if (function == null) { diff --git a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerClangTest.java b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerClangTest.java index 4e84a4518c..d8acb778f7 100644 --- a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerClangTest.java +++ b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerClangTest.java @@ -945,14 +945,14 @@ public class DecompilerClangTest extends AbstractDecompilerTest { private void refresh() { DockingActionIf action = getAction(decompiler, "Refresh"); - performAction(action); + performAction(action, provider.getActionContext(null), true); waitForDecompiler(); } private DecompilerProvider cloneDecompiler() { DockingActionIf action = getAction(decompiler, "Decompile Clone"); - performAction(action); + performAction(action, provider.getActionContext(null), true); waitForSwing(); @SuppressWarnings("unchecked") @@ -1035,7 +1035,7 @@ public class DecompilerClangTest extends AbstractDecompilerTest { private void backwardSlice() { DockingActionIf action = getAction(decompiler, BackwardsSliceAction.NAME); - performAction(action); + performAction(action, provider.getActionContext(null), true); } private void middleMouse() { @@ -1064,7 +1064,7 @@ public class DecompilerClangTest extends AbstractDecompilerTest { private void rename(String newName) { DockingActionIf action = getAction(decompiler, "Rename Variable"); - performAction(action, false); + performAction(action, provider.getActionContext(null), false); InputDialog dialog = waitForDialogComponent(InputDialog.class); runSwing(() -> dialog.setValue(newName)); @@ -1077,7 +1077,7 @@ public class DecompilerClangTest extends AbstractDecompilerTest { DockingActionIf highlightAction = getAction(decompiler, RemoveAllSecondaryHighlightsAction.NAME); - performAction(highlightAction); + performAction(highlightAction, provider.getActionContext(null), true); } private Color highlight() { @@ -1085,7 +1085,7 @@ public class DecompilerClangTest extends AbstractDecompilerTest { ClangToken token = getToken(); DockingActionIf highlightAction = getAction(decompiler, SetSecondaryHighlightAction.NAME); - performAction(highlightAction); + performAction(highlightAction, provider.getActionContext(null), true); HighlightToken ht = getSecondaryHighlight(token); assertNotNull("No highlight for token: " + token, ht); @@ -1106,7 +1106,7 @@ public class DecompilerClangTest extends AbstractDecompilerTest { DockingActionIf highlightAction = getAction(decompiler, SetSecondaryHighlightColorChooserAction.NAME); - performAction(highlightAction, false); + performAction(highlightAction, provider.getActionContext(null), false); Window w = waitForWindow("Please Choose a Color"); GhidraColorChooser colorChooser = findComponent(w, GhidraColorChooser.class); @@ -1238,7 +1238,7 @@ public class DecompilerClangTest extends AbstractDecompilerTest { for (DockingActionIf action : actions) { Object service = getInstanceField("clipboardService", action); if (service.getClass().toString().contains("Decomp")) { - performAction(action); + performAction(action, provider.getActionContext(null), true); return; } } diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java b/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java index 2ef6e8c676..b39b5aa743 100644 --- a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java +++ b/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/AbstractFunctionGraphTest.java @@ -2166,12 +2166,6 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte Double scale = getGraphScale(getPrimaryGraphViewer()); int result = Double.compare(scale, 1.0); assertEquals("Graph not fully zoomed-in; scale: " + scale, 0, result); - - FGVertex v = getFocusedVertex(); - Rectangle cursorBounds = v.getCursorBounds(); - Window graphWindow = windowForComponent(getPrimaryGraphViewer()); - Rectangle windowBounds = graphWindow.getBounds(); - assertTrue(windowBounds.contains(cursorBounds)); } protected void assertZoomedOut() { @@ -2277,7 +2271,7 @@ public abstract class AbstractFunctionGraphTest extends AbstractGhidraHeadedInte protected void goTo(Address address) { GoToService goToService = tool.getService(GoToService.class); - goToService.goTo(address); + runSwing(() -> goToService.goTo(address)); waitForBusyGraph(); } diff --git a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java b/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java index 326b4516d7..c30e7044b1 100644 --- a/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java +++ b/Ghidra/Features/FunctionGraph/src/test/java/ghidra/app/plugin/core/functiongraph/FunctionGraphPlugin1Test.java @@ -843,7 +843,7 @@ public class FunctionGraphPlugin1Test extends AbstractFunctionGraphTest { private void setNavigationHistoryOption(NavigationHistoryChoices choice) throws Exception { FGController controller = getFunctionGraphController(); FunctionGraphOptions options = controller.getFunctionGraphOptions(); - setInstanceField("navigationHistoryChoice", options, choice); + runSwing(() -> setInstanceField("navigationHistoryChoice", options, choice)); waitForSwing(); } diff --git a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java index d6e08558d4..7f704c9a78 100644 --- a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java +++ b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplay.java @@ -64,6 +64,7 @@ import ghidra.graph.job.GraphJobRunner; import ghidra.service.graph.*; import ghidra.util.*; import ghidra.util.exception.CancelledException; +import ghidra.util.task.TaskLauncher; import ghidra.util.task.TaskMonitor; import resources.Icons; @@ -128,10 +129,6 @@ public class DefaultGraphDisplay implements GraphDisplay { * provides graph displays for supplied graphs */ private final DefaultGraphDisplayProvider graphDisplayProvider; - /** - * a 'busy' dialog to show while the layout algorithm is working - */ - private LayoutWorkingDialog layoutWorkingDialog; /** * the vertex that has been nominated to be 'focused' in the graph display and listing */ @@ -359,20 +356,6 @@ public class DefaultGraphDisplay implements GraphDisplay { .onActionStateChanged((s, t) -> layoutChanged(s.getName())) .addStates(getLayoutActionStates()) .buildAndInstallLocal(componentProvider); - - // show a 'busy' dialog while the layout algorithm is computing vertex locations - viewer.getVisualizationModel() - .getLayoutModel() - .getLayoutStateChangeSupport() - .addLayoutStateChangeListener( - evt -> { - if (evt.active) { - Swing.runLater(this::showLayoutWorking); - } - else { - Swing.runLater(this::hideLayoutWorking); - } - }); } private void createPopupActions() { @@ -569,7 +552,8 @@ public class DefaultGraphDisplay implements GraphDisplay { */ private void layoutChanged(String layoutName) { if (layoutTransitionManager != null) { - layoutTransitionManager.setLayout(layoutName); + new TaskLauncher(new SetLayoutTask(viewer, layoutTransitionManager, layoutName), null, + 1000); } } @@ -587,27 +571,6 @@ public class DefaultGraphDisplay implements GraphDisplay { componentProvider.getTool().showDialog(filterDialog); } - /** - * show the 'busy' dialog indicating that the layout algorithm is working - */ - protected void showLayoutWorking() { - if (this.layoutWorkingDialog != null) { - layoutWorkingDialog.close(); - } - this.layoutWorkingDialog = - new LayoutWorkingDialog(viewer.getVisualizationModel().getLayoutAlgorithm()); - componentProvider.getTool().showDialog(layoutWorkingDialog); - } - - /** - * hide the 'busy' dialog for the layout algorithm work - */ - protected void hideLayoutWorking() { - if (this.layoutWorkingDialog != null) { - layoutWorkingDialog.close(); - } - } - /** * add or remove the satellite viewer * @param context information about the event @@ -1216,7 +1179,7 @@ public class DefaultGraphDisplay implements GraphDisplay { @Override public void addAction(DockingAction action) { - componentProvider.addLocalAction(action); + Swing.runLater(() -> componentProvider.addLocalAction(action)); } @Override @@ -1286,4 +1249,19 @@ public class DefaultGraphDisplay implements GraphDisplay { public AttributedGraph getGraph() { return graph; } + + /** + * Removes all externally added actions. This is called before re-using the graph window for a + * new graph which may add its own set of actions for that particular graph. + */ + void restoreToDefaultSetOfActions() { + Swing.runLater(() -> { + // remove all actions + componentProvider.removeAllLocalActions(); + // put the standard graph actions back + createToolbarActions(); + createPopupActions(); + }); + } + } diff --git a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayComponentProvider.java b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayComponentProvider.java index cdbd9d5b53..5faaaf9765 100644 --- a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayComponentProvider.java +++ b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayComponentProvider.java @@ -69,4 +69,10 @@ public class DefaultGraphDisplayComponentProvider extends ComponentProviderAdapt public ActionContext getActionContext(MouseEvent event) { return display.getActionContext(event); } + + // overridden to make it accessible + @Override + public void removeAllLocalActions() { + super.removeAllLocalActions(); + } } diff --git a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayProvider.java b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayProvider.java index 2c936323e7..aa4be85a83 100644 --- a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayProvider.java +++ b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/DefaultGraphDisplayProvider.java @@ -51,7 +51,9 @@ public class DefaultGraphDisplayProvider implements GraphDisplayProvider { TaskMonitor monitor) { if (reuseGraph && !displays.isEmpty()) { - return getVisibleGraph(); + DefaultGraphDisplay visibleGraph = getVisibleGraph(); + visibleGraph.restoreToDefaultSetOfActions(); + return visibleGraph; } DefaultGraphDisplay display = @@ -73,9 +75,11 @@ public class DefaultGraphDisplayProvider implements GraphDisplayProvider { * return one from the Set via its iterator * @return a display that is showing */ - private GraphDisplay getVisibleGraph() { - return displays.stream().filter(d -> d.getComponent().isShowing()) - .findAny().orElse(displays.iterator().next()); + private DefaultGraphDisplay getVisibleGraph() { + return displays.stream() + .filter(d -> d.getComponent().isShowing()) + .findAny() + .orElse(displays.iterator().next()); } @Override @@ -100,4 +104,5 @@ public class DefaultGraphDisplayProvider implements GraphDisplayProvider { public void remove(DefaultGraphDisplay defaultGraphDisplay) { displays.remove(defaultGraphDisplay); } + } diff --git a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/LayoutWorkingDialog.java b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/LayoutWorkingDialog.java deleted file mode 100644 index 5880caa834..0000000000 --- a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/LayoutWorkingDialog.java +++ /dev/null @@ -1,57 +0,0 @@ -/* ### - * 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.graph.visualization; - -import java.awt.BorderLayout; - -import javax.swing.*; - -import org.jungrapht.visualization.layout.algorithms.LayoutAlgorithm; - -import docking.DialogComponentProvider; -import ghidra.service.graph.AttributedVertex; - -/** - * Extends DialogComponentProvider to make a dialog with buttons to show that the - * layout arrangement algorithm is busy - */ -public class LayoutWorkingDialog extends DialogComponentProvider { - - public LayoutWorkingDialog(LayoutAlgorithm layoutAlgorithm) { - super("Working....", false); - super.addWorkPanel(createPanel(layoutAlgorithm)); - setRememberSize(false); - addDismissButton(); - setDefaultButton(dismissButton); - } - - /** - * Create a layout-formatted JComponent holding 2 vertical lists - * of buttons, one list for vertex filter buttons and one list for - * edge filter buttons. Each list has a border and title. - * @return a formatted JComponent (container) - */ - JComponent createPanel(LayoutAlgorithm layoutAlgorithm) { - JProgressBar progressBar = new JProgressBar(); - progressBar.setIndeterminate(true); - JPanel panel = new JPanel(new BorderLayout()); - panel.add(progressBar, BorderLayout.CENTER); - panel.add(new JLabel("Please wait......."), BorderLayout.NORTH); - addCancelButton(); - cancelButton.addActionListener(evt -> layoutAlgorithm.cancel()); - return panel; - } -} diff --git a/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/SetLayoutTask.java b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/SetLayoutTask.java new file mode 100644 index 0000000000..a3745d21f7 --- /dev/null +++ b/Ghidra/Features/GraphServices/src/main/java/ghidra/graph/visualization/SetLayoutTask.java @@ -0,0 +1,106 @@ +/* ### + * 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.graph.visualization; + +import java.util.concurrent.CountDownLatch; + +import org.jungrapht.visualization.VisualizationModel; +import org.jungrapht.visualization.VisualizationViewer; +import org.jungrapht.visualization.layout.event.LayoutStateChange.*; +import org.jungrapht.visualization.layout.model.LayoutModel; + +import ghidra.service.graph.AttributedEdge; +import ghidra.service.graph.AttributedVertex; +import ghidra.util.Swing; +import ghidra.util.exception.CancelledException; +import ghidra.util.task.*; + +/** + * Task to change the layout of the graph + */ +public class SetLayoutTask extends Task { + + private LayoutTransitionManager layoutTransitionManager; + private String layoutName; + private VisualizationViewer viewer; + private CountDownLatch taskDone = new CountDownLatch(1); + + public SetLayoutTask(VisualizationViewer viewer, + LayoutTransitionManager layoutTransitionManager, String layoutName) { + super("Changing Graph Layout to " + layoutName, true, false, true, false); + this.viewer = viewer; + this.layoutTransitionManager = layoutTransitionManager; + this.layoutName = layoutName; + } + + @Override + public void run(TaskMonitor monitor) throws CancelledException { + // add a callback for when/if the user cancels the layout, use a variable cause + // monitor uses a weak listener list and it would othewise get garbage collected. + CancelledListener cancelListener = this::taskCancelled; + monitor.addCancelledListener(cancelListener); + + // add a listener so we are notified when the layout starts and ends + VisualizationModel model = viewer.getVisualizationModel(); + LayoutModel layoutModel = model.getLayoutModel(); + Support support = layoutModel.getLayoutStateChangeSupport(); + Listener listener = this::layoutStateChanged; + support.addLayoutStateChangeListener(listener); + + // start the layout - needs to be done on swing thread to prevent issues and intermediate + // paints - should be changed in the future to not require it to be on the swing thread. + Swing.runNow(() -> layoutTransitionManager.setLayout(layoutName)); + + // some of the layouts are done on the calling thread and some aren't. If they are on + // the calling thread, then by now, we already got the "done" callback and the "taskDone" + // countdown latch has been triggered and won't wait. If, however, the layout has been + // diverted to another thread, we want to wait until the layout is completed + // There are two ways the latch will be triggered, the layout is completed or the user + // cancles the layout. + try { + taskDone.await(); + } + catch (InterruptedException e) { + model.getLayoutAlgorithm().cancel(); + } + + // clean up the listeners + support.removeLayoutStateChangeListener(listener); + monitor.removeCancelledListener(cancelListener); + } + + /** + * Notfication when the layout algorithm starts and stops. + * @param e the event. If the event.active is true, then the + * algorithm is starting, if false, the algorithm is done. + */ + private void layoutStateChanged(Event e) { + if (!e.active) { + // algorithm is done, release the latch + taskDone.countDown(); + } + } + + /** + * Callback if the user cancels the layout + */ + private void taskCancelled() { + // release the latch and tell the layout algorithm to cancel. + taskDone.countDown(); + viewer.getVisualizationModel().getLayoutAlgorithm().cancel(); + } + +} diff --git a/Ghidra/Framework/Docking/src/main/java/docking/ComponentProvider.java b/Ghidra/Framework/Docking/src/main/java/docking/ComponentProvider.java index 2e546433b2..b45a616933 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/ComponentProvider.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/ComponentProvider.java @@ -267,7 +267,7 @@ public abstract class ComponentProvider implements HelpDescriptor, ActionContext } /** - * Removes the given action from the system. + * Removes the given action from this component provider. * @param action The action to remove. */ protected void removeLocalAction(DockingAction action) { @@ -277,6 +277,16 @@ public abstract class ComponentProvider implements HelpDescriptor, ActionContext } } + /** + * Removes all local actions from this component provider + */ + protected void removeAllLocalActions() { + if (isInTool()) { + actionSet.forEach(action -> dockingTool.removeLocalAction(this, action)); + } + actionSet.clear(); + } + /** * Convenience method to show or hide this provider. * @param visible True shows the provider; false hides the provider diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/fieldpanel/support/HoverProvider.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/fieldpanel/support/HoverProvider.java index d8eaed33f9..0108764c49 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/fieldpanel/support/HoverProvider.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/fieldpanel/support/HoverProvider.java @@ -24,6 +24,7 @@ public interface HoverProvider { /** * Returns true if this service's popup window is currently visible + * @return true if this service's popup window is currently visible */ public boolean isShowing(); diff --git a/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskMonitorComponent.java b/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskMonitorComponent.java index 743e150d43..9899abe20b 100644 --- a/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskMonitorComponent.java +++ b/Ghidra/Framework/Docking/src/main/java/ghidra/util/task/TaskMonitorComponent.java @@ -273,6 +273,7 @@ public class TaskMonitorComponent extends JPanel implements TaskMonitor { * @return true if {@link #setIndeterminate(boolean)} with a value of true has * been called. */ + @Override public boolean isIndeterminate() { return isIndeterminate.get(); } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataUtilities.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataUtilities.java index 7d97369f6b..b4716f70dc 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataUtilities.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/DataUtilities.java @@ -125,12 +125,19 @@ public final class DataUtilities { existingDataLen = data.getLength(); existingDT = data.getDataType(); + if (data.isDefined() && newDataType.isEquivalent(existingDT)) { + return data; + } + // Check for external reference on pointer - Reference[] refs = refMgr.getReferencesFrom(addr); - for (Reference ref : refs) { - if (ref.getOperandIndex() == 0 && ref.isExternalReference()) { - extRef = ref; - break; + if (existingDT instanceof Pointer) { + // TODO: This can probably be eliminated + Reference[] refs = refMgr.getReferencesFrom(addr); + for (Reference ref : refs) { + if (ref.getOperandIndex() == 0 && ref.isExternalReference()) { + extRef = ref; + break; + } } } } @@ -192,11 +199,11 @@ public final class DataUtilities { private static void checkEnoughSpace(Program program, Address addr, int existingDataLen, DataTypeInstance dti, ClearDataMode mode) throws CodeUnitInsertionException { Listing listing = program.getListing(); - int newSize = dti.getLength(); - if (newSize <= existingDataLen) { - listing.clearCodeUnits(addr, addr, false); - return; - } +// int newSize = dti.getLength(); +// if (newSize <= existingDataLen) { +// listing.clearCodeUnits(addr, addr, false); +// return; +// } try { Address end = addr.addNoWrap(existingDataLen - 1); Address newEnd = addr.addNoWrap(dti.getLength() - 1); diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/PointerDataType.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/PointerDataType.java index 3f15f23a68..75c30d8967 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/PointerDataType.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/PointerDataType.java @@ -60,7 +60,7 @@ public class PointerDataType extends BuiltIn implements Pointer { * organization when resolved. */ public PointerDataType() { - this(null, -1, null); + this(DataType.DEFAULT, -1, null); } /** @@ -72,7 +72,7 @@ public class PointerDataType extends BuiltIn implements Pointer { * @param dtm data-type manager whose data organization should be used */ public PointerDataType(DataTypeManager dtm) { - this(null, -1, dtm); + this(DataType.DEFAULT, -1, dtm); } /** @@ -142,7 +142,7 @@ public class PointerDataType extends BuiltIn implements Pointer { } @Override - public final DataType clone(DataTypeManager dtm) { + public final Pointer clone(DataTypeManager dtm) { if (dtm == getDataTypeManager()) { return this; } diff --git a/Ghidra/Processors/MIPS/src/main/java/ghidra/app/util/bin/format/elf/extend/MIPS_ElfExtension.java b/Ghidra/Processors/MIPS/src/main/java/ghidra/app/util/bin/format/elf/extend/MIPS_ElfExtension.java index 99905b2361..99c1a8c366 100644 --- a/Ghidra/Processors/MIPS/src/main/java/ghidra/app/util/bin/format/elf/extend/MIPS_ElfExtension.java +++ b/Ghidra/Processors/MIPS/src/main/java/ghidra/app/util/bin/format/elf/extend/MIPS_ElfExtension.java @@ -744,7 +744,9 @@ public class MIPS_ElfExtension extends ElfExtension { Address gotEntryAddr = adjustTableEntryIfNonZero(gotBaseAddress, i, imageShift, elfLoadHelper); Data pointerData = elfLoadHelper.createData(gotEntryAddr, PointerDataType.dataType); - setConstant(pointerData); + if (ElfDefaultGotPltMarkup.isValidPointer(pointerData)) { + ElfDefaultGotPltMarkup.setConstant(pointerData); + } } // process global/external symbol got entries @@ -753,7 +755,7 @@ public class MIPS_ElfExtension extends ElfExtension { Address gotEntryAddr = adjustTableEntryIfNonZero(gotBaseAddress, gotIndex++, imageShift, elfLoadHelper); Data pointerData = elfLoadHelper.createData(gotEntryAddr, PointerDataType.dataType); - setConstant(pointerData); + ElfDefaultGotPltMarkup.setConstant(pointerData); if (elfSymbols[i].isFunction() && elfSymbols[i].getSectionHeaderIndex() == 0) { // ensure that external function/thunk are created in absence of sections Address refAddr = (Address) pointerData.getValue(); @@ -811,7 +813,7 @@ public class MIPS_ElfExtension extends ElfExtension { Address gotEntryAddr = adjustTableEntryIfNonZero(mipsPltgotBase, ++gotEntryIndex, imageShift, elfLoadHelper); Data pointerData = elfLoadHelper.createData(gotEntryAddr, PointerDataType.dataType); - setConstant(pointerData); + ElfDefaultGotPltMarkup.setConstant(pointerData); } } catch (NotFoundException e) { @@ -822,17 +824,6 @@ public class MIPS_ElfExtension extends ElfExtension { } } - private void setConstant(Data pointerData) { - Memory memory = pointerData.getProgram().getMemory(); - MemoryBlock block = memory.getBlock(pointerData.getAddress()); - if (!block.isWrite() || block.getName().startsWith(ElfSectionHeaderConstants.dot_got)) { - // .got blocks will be force to read-only by ElfDefaultGotPltMarkup - return; - } - pointerData.setLong(MutabilitySettingsDefinition.MUTABILITY, - MutabilitySettingsDefinition.CONSTANT); - } - private Address adjustTableEntryIfNonZero(Address tableBaseAddr, int entryIndex, long adjustment, ElfLoadHelper elfLoadHelper) throws MemoryAccessException { boolean is64Bit = elfLoadHelper.getElfHeader().is64Bit(); diff --git a/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/util/bin/format/elf/extend/PowerPC64_ElfExtension.java b/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/util/bin/format/elf/extend/PowerPC64_ElfExtension.java index 591262ef7b..8c0e6cab82 100644 --- a/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/util/bin/format/elf/extend/PowerPC64_ElfExtension.java +++ b/Ghidra/Processors/PowerPC/src/main/java/ghidra/app/util/bin/format/elf/extend/PowerPC64_ElfExtension.java @@ -368,6 +368,9 @@ public class PowerPC64_ElfExtension extends ElfExtension { return null; } + ElfDefaultGotPltMarkup.setConstant(refPtr); + ElfDefaultGotPltMarkup.setConstant(tocPtr); + Function function = program.getListing().getFunctionAt(refAddr); if (function == null) { // Check for potential pointer table (unsure a non-function would be referenced by OPD section)