From 2ef2619e18d244c58c644723de87314b945bce32 Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Thu, 15 May 2025 16:33:33 -0400 Subject: [PATCH 1/6] GP-5679 - Data Type Filter fix --- .../core/datamgr/tree/DtFilterDialog.java | 2 +- .../core/datamgr/tree/DtFilterState.java | 24 +++++++------- .../datamgr/DataTypeManagerPluginTest.java | 31 ++++++++++++++++--- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterDialog.java index dcdc8ba196..f4af227db3 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterDialog.java @@ -178,7 +178,7 @@ public class DtFilterDialog extends DialogComponentProvider { this.type = type; this.typeCb = new GCheckBox(type); this.typeDefCb = new GCheckBox(); - this.typeDefCb.setName(type + "Typedefs"); + this.typeDefCb.setName(type + "TypeDefs"); } JComponent getLeft() { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterState.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterState.java index 235d892e45..6727b2cb30 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterState.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/tree/DtFilterState.java @@ -137,40 +137,38 @@ public class DtFilterState { DataType baseDt = DataTypeUtils.getBaseDataType(dt); if (dt instanceof Array) { - return passes(arraysFilter, dt, baseDt); + return passes(arraysFilter, dt); } if (dt instanceof Pointer) { - return passes(pointersFilter, dt, baseDt); + return passes(pointersFilter, dt); } if (baseDt instanceof Enum) { - return passes(enumsFilter, dt, baseDt); + return passes(enumsFilter, dt); } if (baseDt instanceof Function) { - return passes(functionsFilter, dt, baseDt); + return passes(functionsFilter, dt); } if (baseDt instanceof Structure) { - return passes(structuresFilter, dt, baseDt); + return passes(structuresFilter, dt); } if (baseDt instanceof Union) { - return passes(unionsFilter, dt, baseDt); + return passes(unionsFilter, dt); } return true; } - private boolean passes(DtTypeFilter filter, DataType dt, DataType baseDt) { - if (filter.isTypeActive()) { - return true; + private boolean passes(DtTypeFilter filter, DataType dt) { + if (dt instanceof TypeDef) { + return filter.isTypeDefActive(); } - if (filter.isTypeDefActive() && dt instanceof TypeDef) { - return true; - } - return false; + + return filter.isTypeActive(); } public void save(SaveState parentSaveState) { diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeManagerPluginTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeManagerPluginTest.java index fd8f7d7556..a9972ba0ee 100644 --- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeManagerPluginTest.java +++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/DataTypeManagerPluginTest.java @@ -16,6 +16,7 @@ package ghidra.app.plugin.core.datamgr; import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.*; import java.awt.Container; @@ -959,7 +960,7 @@ public class DataTypeManagerPluginTest extends AbstractGhidraHeadedIntegrationTe // Now also turn off typedefs performAction(action, provider, false); dialog = waitForDialogComponent(DtFilterDialog.class); - setToggleButtonSelected(dialog.getComponent(), "StructuresTypedefs", false); + setToggleButtonSelected(dialog.getComponent(), "StructuresTypeDefs", false); pressButtonByText(dialog, "OK"); waitForTree(); @@ -967,6 +968,26 @@ public class DataTypeManagerPluginTest extends AbstractGhidraHeadedIntegrationTe assertType("TypeDefToMyStruct", false); } + @Test + public void testFilter_Structures_HideTypeDefs() { + + assertStructures(true); + assertType("TypeDefToMyStruct", true); + + // press the filter button + DockingActionIf action = getAction(plugin, "Show Filter"); + performAction(action, provider, false); + DtFilterDialog dialog = waitForDialogComponent(DtFilterDialog.class); + + // turn off Structure TypeDefs + setToggleButtonSelected(dialog.getComponent(), "StructuresTypeDefs", false); + pressButtonByText(dialog, "OK"); + waitForTree(); + + assertStructures(true); // still have structures + assertType("TypeDefToMyStruct", false); // no longer have structure typedefs + } + @Test public void testFilter_ClonedProvider() { @@ -1058,14 +1079,14 @@ public class DataTypeManagerPluginTest extends AbstractGhidraHeadedIntegrationTe } private Map getNodes(DataTypeManager dtm) { - + DataTypeArchiveGTree gTree = provider.getGTree(); GTreeNode rootNode = gTree.getViewRoot(); GTreeNode dtmNode = rootNode.getChild(dtm.getName()); assertNotNull(dtmNode); - + expandNode(dtmNode); - + Map nodesByName = new HashMap<>(); Iterator it = dtmNode.iterator(true); for (GTreeNode node : CollectionUtils.asIterable(it)) { @@ -1076,7 +1097,7 @@ public class DataTypeManagerPluginTest extends AbstractGhidraHeadedIntegrationTe DataType dt = dtNode.getDataType(); nodesByName.put(dt.getName(), dtNode); } - + return nodesByName; } From c1e42d60d840c8c4175d165cc1d4f29811edda4f Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Fri, 16 May 2025 13:21:22 +0000 Subject: [PATCH 2/6] GP-5533: Document Android NDK lldb setup. --- .../src/main/help/help/topics/lldb/lldb.html | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/Ghidra/Debug/Debugger-agent-lldb/src/main/help/help/topics/lldb/lldb.html b/Ghidra/Debug/Debugger-agent-lldb/src/main/help/help/topics/lldb/lldb.html index 654d58ffe2..33e9057262 100644 --- a/Ghidra/Debug/Debugger-agent-lldb/src/main/help/help/topics/lldb/lldb.html +++ b/Ghidra/Debug/Debugger-agent-lldb/src/main/help/help/topics/lldb/lldb.html @@ -43,6 +43,9 @@ python3 -m pip install psutil protobuf==3.20.3 +

If you're using lldb from the Android NDK and do not have Pip, see Setup for Android NDK

+

If you are offline, or would like to use our provided packages, we still use Pip, but with a more complicated invocation:

@@ -205,5 +208,63 @@ perl -i -pe 's/(?<=pendingNMI\x00{4})\x00/\x01/' macOS_15-1234567.vmss

This has the same options as the LLDB via SSH launcher, which are necessary for connecting to the Android debugger, but executes via the normal lldb mechanism.

+ +

Setup for Android NDK

+ +

If you're using the copy of lldb included with the Android NDK (Native Development + Kit), it may not include pip. Notably, this is the case on Windows at the time of + writing. Fortunately, you can retrieve the components to install Pip into the NDK from an + official Python distribution.

+ +
    +
  1. + First, figure out the version of Python that is embedded in the NDK's build of LLDB, and + get its path. (If you know the path to lldb, you probably already know the path to its + Python.) From a Windows Command Prompt or Powershell: +
    +PS> C:\path\to\android-ndk\...\lldb
+(lldb) script
+>>> import sys
+>>> sys.version
+[copy down the version indicated]
+>>> sys.path
+[look for the paths ending with Lib and DLLs, and copy them down]
+
    +
    2. + +
  2. Now, obtain the same version of Python from the official Python website, and install or + unpack it.
    3. + +
  3. Locate your new installation of Python. If you don't already know where it landed, this + can be found by examining the Properties of the Python shortcut in your Start Menu.
    4. + +
  4. There should be a Lib\ensurepip directory in the official Python installation. + Copy this into the same place in the Android NDK's build of Python.
    5. + +
  5. + There are also three native modules that need to be copied from the official Python's + DLLs\ directory to the same in the NDK's build. This is to support SSL for + downloading packages from PyPI: (Substitue the ??'s appropriately.) + +
      +
    • _ssl.pyd
      • + +
    • libssl-??.dll
      • + +
    • libcrypto-??.dll
      • +
    +
    6. + +
  6. + We should now have enough to bootstrap the NDK's Python with Pip. Again at the Windows + Command Prompt or Powershell: +
    +PS> C:\path\to\android-ndk\...\python -m ensurepip
+PS> C:\path\to\android-ndk\...\python -m pip install ...
+
    + See the Setup section for the arguments to pass to pip install + .... +
    7. +
From 04609f52da292cd2058d516d1cce1c6ccd5c5674 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Fri, 16 May 2025 13:49:54 +0000 Subject: [PATCH 3/6] GP-0: Fix tests. --- .../java/agent/gdb/rmi/AbstractGdbTraceRmiTest.java | 6 ++++++ .../test.slow/java/agent/gdb/rmi/GdbCommandsTest.java | 9 ++++----- .../java/agent/lldb/rmi/AbstractLldbTraceRmiTest.java | 6 ++++++ .../test.slow/java/agent/lldb/rmi/LldbCommandsTest.java | 8 ++++---- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/AbstractGdbTraceRmiTest.java b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/AbstractGdbTraceRmiTest.java index d7fcd24d93..72fbf68a46 100644 --- a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/AbstractGdbTraceRmiTest.java +++ b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/AbstractGdbTraceRmiTest.java @@ -388,6 +388,12 @@ public abstract class AbstractGdbTraceRmiTest extends AbstractGhidraHeadedDebugg return new MemDump(address, buf.toByteArray()); } + protected void waitDomainObjectClosed(String path) { + DomainFile df = env.getProject().getProjectData().getFile(path); + assertNotNull(df); + waitForPass(() -> assertFalse(df.isOpen())); + } + protected ManagedDomainObject openDomainObject(String path) throws Exception { DomainFile df = env.getProject().getProjectData().getFile(path); assertNotNull(df); diff --git a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/GdbCommandsTest.java b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/GdbCommandsTest.java index a4a78e0520..6c11a8d000 100644 --- a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/GdbCommandsTest.java +++ b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/gdb/rmi/GdbCommandsTest.java @@ -160,7 +160,6 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest { @Test public void testStopTrace() throws Exception { - // TODO: This test assumes gdb and the target file bash are x86-64 runThrowError(addr -> """ %s ghidra trace connect %s @@ -169,10 +168,8 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest { ghidra trace stop quit """.formatted(PREAMBLE, addr)); - DomainFile dfBash = env.getProject().getProjectData().getFile("/New Traces/gdb/bash"); - assertNotNull(dfBash); - // TODO: Given the 'quit' command, I'm not sure this assertion is checking anything. - assertFalse(dfBash.isOpen()); + // NOTE: Given the 'quit' command, I'm not sure this assertion is checking anything. + waitDomainObjectClosed("/New Traces/gdb/bash"); } @Test @@ -278,6 +275,7 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest { ghidra trace tx-commit quit """.formatted(PREAMBLE, addr)); + waitDomainObjectClosed("/New Traces/no-save"); try (ManagedDomainObject mdo = openDomainObject("/New Traces/no-save")) { tb = new ToyDBTraceBuilder((Trace) mdo.get()); assertEquals(0, tb.trace.getTimeManager().getAllSnapshots().size()); @@ -294,6 +292,7 @@ public class GdbCommandsTest extends AbstractGdbTraceRmiTest { ghidra trace save quit """.formatted(PREAMBLE, addr)); + waitDomainObjectClosed("/New Traces/save"); try (ManagedDomainObject mdo = openDomainObject("/New Traces/save")) { tb = new ToyDBTraceBuilder((Trace) mdo.get()); assertEquals(1, tb.trace.getTimeManager().getAllSnapshots().size()); diff --git a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/AbstractLldbTraceRmiTest.java b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/AbstractLldbTraceRmiTest.java index 3789c29a4c..0406f53b9f 100644 --- a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/AbstractLldbTraceRmiTest.java +++ b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/AbstractLldbTraceRmiTest.java @@ -418,6 +418,12 @@ public abstract class AbstractLldbTraceRmiTest extends AbstractGhidraHeadedDebug return new MemDump(address, buf.toByteArray()); } + protected void waitDomainObjectClosed(String path) { + DomainFile df = env.getProject().getProjectData().getFile(path); + assertNotNull(df); + waitForPass(() -> assertFalse(df.isOpen())); + } + protected ManagedDomainObject openDomainObject(String path) throws Exception { DomainFile df = env.getProject().getProjectData().getFile(path); assertNotNull(df); diff --git a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/LldbCommandsTest.java b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/LldbCommandsTest.java index df9eaad041..03fab6613a 100644 --- a/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/LldbCommandsTest.java +++ b/Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/lldb/rmi/LldbCommandsTest.java @@ -167,10 +167,8 @@ public class LldbCommandsTest extends AbstractLldbTraceRmiTest { ghidra trace stop quit """.formatted(PREAMBLE, addr, getSpecimenPrint())); - DomainFile df = env.getProject().getProjectData().getFile("/New Traces/lldb/expPrint"); - assertNotNull(df); - // TODO: Given the 'quit' command, I'm not sure this assertion is checking anything. - assertFalse(df.isOpen()); + // NOTE: Given the 'quit' command, I'm not sure this assertion is checking anything. + waitDomainObjectClosed("/New Traces/lldb/expPrint"); } @Test @@ -270,6 +268,7 @@ public class LldbCommandsTest extends AbstractLldbTraceRmiTest { ghidra trace tx-commit quit """.formatted(PREAMBLE, addr, getSpecimenPrint())); + waitDomainObjectClosed("/New Traces/no-save"); try (ManagedDomainObject mdo = openDomainObject("/New Traces/no-save")) { tb = new ToyDBTraceBuilder((Trace) mdo.get()); assertEquals(0, tb.trace.getTimeManager().getAllSnapshots().size()); @@ -286,6 +285,7 @@ public class LldbCommandsTest extends AbstractLldbTraceRmiTest { ghidra trace save quit """.formatted(PREAMBLE, addr, getSpecimenPrint())); + waitDomainObjectClosed("/New Traces/save"); try (ManagedDomainObject mdo = openDomainObject("/New Traces/save")) { tb = new ToyDBTraceBuilder((Trace) mdo.get()); assertEquals(1, tb.trace.getTimeManager().getAllSnapshots().size()); From 2f51ec305c763dc0f8de47ecd4cf8cfeaf0966bf Mon Sep 17 00:00:00 2001 From: ghizard <50744617+ghizard@users.noreply.github.com> Date: Fri, 16 May 2025 10:47:00 -0400 Subject: [PATCH 4/6] GP-5697 - PDB Provide post-analysis pop-up when PDB file not found --- .../java/ghidra/app/plugin/core/analysis/PdbAnalyzer.java | 8 +++++--- .../app/plugin/core/analysis/PdbUniversalAnalyzer.java | 4 +++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbAnalyzer.java b/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbAnalyzer.java index 70b2af10a3..966ae37cb0 100644 --- a/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbAnalyzer.java +++ b/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbAnalyzer.java @@ -87,7 +87,9 @@ public class PdbAnalyzer extends AbstractAnalyzer { File pdbFile = PdbAnalyzerCommon.findPdb(this, program, searchUntrustedLocations, monitor); if (pdbFile == null) { - // warnings have already been logged + // Warnings have already been logged, but nice to have a post-analysis pop-up that + // PDB analysis was not done + log.appendMsg(NAME, "Aborted: Could not find an appropriate PDB file; see log"); return false; } @@ -156,7 +158,7 @@ public class PdbAnalyzer extends AbstractAnalyzer { * Normally the analyzer would locate the PDB file on its own, but if a * headless script wishes to override the analyzer's behaivor, it can * use this method to specify a file. - * + * * @param program {@link Program} * @param pdbFile the pdb file */ @@ -171,7 +173,7 @@ public class PdbAnalyzer extends AbstractAnalyzer { * Normally when the analyzer attempts to locate a matching PDB file it * will default to NOT searching untrusted symbol servers. A headless script could * use this method to allow the analyzer to search untrusted symbol servers. - * + * * @param program {@link Program} * @param allowUntrusted boolean flag, true means analyzer can search untrusted symbol * servers diff --git a/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbUniversalAnalyzer.java b/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbUniversalAnalyzer.java index ba8c6b8b75..6ff76eabd1 100644 --- a/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbUniversalAnalyzer.java +++ b/Ghidra/Features/PDB/src/main/java/ghidra/app/plugin/core/analysis/PdbUniversalAnalyzer.java @@ -167,7 +167,9 @@ public class PdbUniversalAnalyzer extends AbstractAnalyzer { pdbFile = PdbAnalyzerCommon.findPdb(this, program, searchUntrustedLocations, monitor); } if (pdbFile == null) { - // warnings have already been logged + // Warnings have already been logged, but nice to have a post-analysis pop-up that + // PDB analysis was not done + log.appendMsg(NAME, "Aborted: Could not find an appropriate PDB file; see log"); return false; } From 6c6eb609c22d4eb1cd5526df370d638c1296dd8e Mon Sep 17 00:00:00 2001 From: Ryan Kurtz Date: Fri, 16 May 2025 13:42:33 -0400 Subject: [PATCH 5/6] GP-5552: Integrated the UniversalMachoLoader into MachoLoader to address a library loading issue --- .../ghidra/app/util/opinion/MachoLoader.java | 243 +++++++++++++----- .../util/opinion/UniversalMachoLoader.java | 113 -------- 2 files changed, 174 insertions(+), 182 deletions(-) delete mode 100644 Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/UniversalMachoLoader.java diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MachoLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MachoLoader.java index 75a7e99987..8c51c0ecc9 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MachoLoader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MachoLoader.java @@ -65,50 +65,57 @@ public class MachoLoader extends AbstractLibrarySupportLoader { return loadSpecs; } - // Efficient check to fail fast - byte[] magicBytes = provider.readBytes(0, 4); - if (!MachConstants.isMagic(LittleEndianDataConverter.INSTANCE.getInt(magicBytes))) { - return loadSpecs; + // This loader can handle both Mach-O files as well as Universal Binary files. If it's a + // Universal Binary, each Mach-O it contains will be presented as a single "preferred" + // load spec, forcing the user to have to select the desired processor from the import + // dialog. + List allProviders = new ArrayList<>(); + boolean onlyPreferred; + if (isUniveralBinary(provider)) { + allProviders.addAll(getUniveralBinaryProviders(provider)); + onlyPreferred = true; + } + else { + allProviders.add(provider); + onlyPreferred = false; } - try { - MachHeader machHeader = new MachHeader(provider); - String magic = - CpuTypes.getMagicString(machHeader.getCpuType(), machHeader.getCpuSubType()); - String compiler = detectCompilerName(machHeader); - List results = QueryOpinionService.query(MACH_O_NAME, magic, compiler); - for (QueryResult result : results) { - loadSpecs.add(new LoadSpec(this, machHeader.getImageBase(), result)); + for (ByteProvider machoProvider : allProviders) { + byte[] magicBytes = machoProvider.readBytes(0, 4); + if (!MachConstants.isMagic(LittleEndianDataConverter.INSTANCE.getInt(magicBytes))) { + continue; } - if (loadSpecs.isEmpty()) { - loadSpecs.add(new LoadSpec(this, machHeader.getImageBase(), true)); + try { + MachHeader machHeader = new MachHeader(machoProvider); + String magic = + CpuTypes.getMagicString(machHeader.getCpuType(), machHeader.getCpuSubType()); + String compiler = detectCompilerName(machHeader); + List results = QueryOpinionService.query(MACH_O_NAME, magic, compiler); + for (QueryResult result : results) { + if (!onlyPreferred || result.preferred) { + loadSpecs.add(new LoadSpec(this, machHeader.getImageBase(), result)); + } + } + if (loadSpecs.isEmpty() && !onlyPreferred) { + loadSpecs.add(new LoadSpec(this, machHeader.getImageBase(), true)); + } + } + catch (MachException e) { + // not a problem, just don't add it } } - catch (MachException e) { - // not a problem, just don't add it - } + return loadSpecs; } - private String detectCompilerName(MachHeader machHeader) throws IOException { - List sectionNames = machHeader.parseSegments() - .stream() - .flatMap(seg -> seg.getSections().stream()) - .map(section -> section.getSectionName()) - .toList(); - if (SwiftUtils.isSwift(sectionNames)) { - return SwiftUtils.SWIFT_COMPILER; - } - if (GoRttiMapper.hasGolangSections(sectionNames)) { - return GoConstants.GOLANG_CSPEC_NAME; - } - return null; - } - @Override public void load(ByteProvider provider, LoadSpec loadSpec, List