From d90a600b88d04d13875063843b6400c88343e94d Mon Sep 17 00:00:00 2001 From: Egor Kin Date: Mon, 25 May 2020 13:38:59 +0300 Subject: [PATCH 01/22] Fix export unicode comments Current script make 1 character for each byte in comment line. So it`s cause double length and incorrect chars in international comments from IDA in Ghidra. This small patch fix it. --- GhidraBuild/IDAPro/Python/7xx/python/idaxml.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py b/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py index 51fc8eba99..3bd77cc691 100644 --- a/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py +++ b/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py @@ -632,7 +632,7 @@ class XmlExporter(IdaXml): # tag_remove seems to be losing last character # work around is to add a space cmt_text = ida_lines.tag_remove(cmt + ' ') - self.write_text(cmt_text) + self.write_text(cmt_text.decode('utf-8')) self.end_element(COMMENT, False) From 6201b9bcdfb1a877d3c08c119f8402f3b4ac7add Mon Sep 17 00:00:00 2001 From: cmasupra <74213879+cmasupra@users.noreply.github.com> Date: Mon, 9 Nov 2020 15:59:11 -0600 Subject: [PATCH 02/22] Update 8085.slaspec with "XRA M" or "XRA HL" This adds the instruction "XRA M" in addition to "XRA r", which already exists. "XRA M" is the Intel format, but the code being added will keep the Ghidra format of "XRA HL". Intel's MCS-80/85(tm) Family User's Manual from October 1979 indicates "XRA M" is a 1-byte instruction with value 0xAE. --- Ghidra/Processors/8085/data/languages/8085.slaspec | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Ghidra/Processors/8085/data/languages/8085.slaspec b/Ghidra/Processors/8085/data/languages/8085.slaspec index 2001b6e964..c9874f6b21 100644 --- a/Ghidra/Processors/8085/data/languages/8085.slaspec +++ b/Ghidra/Processors/8085/data/languages/8085.slaspec @@ -349,6 +349,14 @@ cc: "M" is bits3_3=0x7 { export S_flag; setResultFlags(A); } +:XRA (HL) is op0_8=0xae & HL { + AC_flag = 0; + CY_flag = 0; + P_flag = 0; + A = A ^ *:1 HL; + setResultFlags(A); +} + :XRI imm8 is op0_8=0xee; imm8 { AC_flag = 0; CY_flag = 0; From d211d5cd38e1260fe3a566f271fa24ca64861063 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Mon, 8 Feb 2021 15:42:18 -0500 Subject: [PATCH 03/22] GP-623: Assembler binds to focused listing, now. --- .../core/assembler/AssembleDockingAction.java | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/assembler/AssembleDockingAction.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/assembler/AssembleDockingAction.java index d77ec3267a..dac395839c 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/assembler/AssembleDockingAction.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/assembler/AssembleDockingAction.java @@ -37,7 +37,6 @@ import ghidra.app.plugin.assembler.Assembler; import ghidra.app.plugin.assembler.Assemblers; import ghidra.app.plugin.core.assembler.AssemblyDualTextField.*; import ghidra.app.plugin.core.codebrowser.CodeViewerProvider; -import ghidra.app.util.PluginConstants; import ghidra.app.util.viewer.field.ListingField; import ghidra.app.util.viewer.listingpanel.ListingModelAdapter; import ghidra.app.util.viewer.listingpanel.ListingPanel; @@ -77,7 +76,7 @@ public class AssembleDockingAction extends DockingAction { private Language lang; private Assembler assembler; private final MyListener listener = new MyListener(); - private PluginTool tool; + //private PluginTool tool; // Callback to keep the autocompleter positioned under the fields private FieldPanelOverLayoutListener autoCompleteMover = (FieldPanelOverLayoutEvent ev) -> { @@ -158,23 +157,7 @@ public class AssembleDockingAction extends DockingAction { */ public AssembleDockingAction(PluginTool tool, String name, String owner) { this(name, owner); - this.tool = tool; - } - - @Override - public void dispose() { - super.dispose(); - input.dispose(); - } - - protected void onFirstInvocation() { - ComponentProvider prov = tool.getComponentProvider(PluginConstants.CODE_BROWSER); - cv = (CodeViewerProvider) prov; - listpane = cv.getListingPanel(); - codepane = listpane.getFieldPanel(); - - fieldLayoutManager = new FieldPanelOverLayoutManager(codepane); - codepane.setLayout(fieldLayoutManager); + //this.tool = tool; // If I lose focus, cancel the assembly input.addFocusListener(new FocusListener() { @@ -188,14 +171,37 @@ public class AssembleDockingAction extends DockingAction { cancel(); } }); + input.getMnemonicField().setBorder(BorderFactory.createLineBorder(Color.RED, 2)); input.getOperandsField().setBorder(BorderFactory.createLineBorder(Color.RED, 2)); input.getAssemblyField().setBorder(BorderFactory.createLineBorder(Color.RED, 2)); input.getAutocompleter().addAutocompletionListener(listener); input.addKeyListener(listener); + } - fieldLayoutManager.addLayoutListener(autoCompleteMover); + @Override + public void dispose() { + super.dispose(); + input.dispose(); + } + + protected void prepareLayout(ActionContext context) { + ComponentProvider prov = context.getComponentProvider(); + if (cv != prov) { + if (cv != null) { + codepane.setLayout(null); + fieldLayoutManager.removeLayoutListener(autoCompleteMover); + } + + cv = (CodeViewerProvider) prov; + listpane = cv.getListingPanel(); + codepane = listpane.getFieldPanel(); + + fieldLayoutManager = new FieldPanelOverLayoutManager(codepane); + codepane.setLayout(fieldLayoutManager); + fieldLayoutManager.addLayoutListener(autoCompleteMover); + } } /** @@ -264,11 +270,13 @@ public class AssembleDockingAction extends DockingAction { @Override public void actionPerformed(ActionContext context) { - if (cv == null) { - onFirstInvocation(); + if (!(context instanceof ListingActionContext)) { + return; } + prepareLayout(context); + ListingActionContext lac = (ListingActionContext) context; - ProgramLocation cur = cv.getLocation(); + ProgramLocation cur = lac.getLocation(); prog = cur.getProgram(); addr = cur.getAddress(); @@ -351,9 +359,8 @@ public class AssembleDockingAction extends DockingAction { @Override public boolean isAddToPopup(ActionContext context) { - // currently on work on the listing - Object obj = context.getContextObject(); - if (obj instanceof ListingActionContext) { + // currently only works on listings + if (context instanceof ListingActionContext) { return true; } return false; From 98331405b84bb48a19db889d8241909fe6635ada Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Tue, 9 Feb 2021 08:03:40 -0500 Subject: [PATCH 04/22] GP-587: Assembling bytes into a side buffer, then patching all at once. --- .../app/plugin/assembler/Assembler.java | 36 ++++++++++------ .../assembler/sleigh/SleighAssembler.java | 41 ++++++++++++------- 2 files changed, 49 insertions(+), 28 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java index a05df28c67..8038db8620 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java @@ -21,8 +21,8 @@ import ghidra.app.plugin.assembler.sleigh.parse.AssemblyParseResult; import ghidra.app.plugin.assembler.sleigh.sem.*; import ghidra.program.model.address.Address; import ghidra.program.model.address.AddressOverflowException; -import ghidra.program.model.lang.InstructionBlock; import ghidra.program.model.listing.Instruction; +import ghidra.program.model.listing.InstructionIterator; import ghidra.program.model.mem.MemoryAccessException; /** @@ -36,8 +36,8 @@ public interface Assembler { * Assemble a sequence of instructions and place them at the given address. * * This method is only valid if the assembler is bound to a program. An instance may optionally - * implement this method without a program binding. In that case, the returned instruction - * block will refer to pseudo instructions. + * implement this method without a program binding. In that case, the returned instruction block + * will refer to pseudo instructions. * * NOTE: There must be an active transaction on the bound program for this method to succeed. * @@ -49,7 +49,8 @@ public interface Assembler { * @throws MemoryAccessException there is an issue writing the result to program memory * @throws AddressOverflowException the resulting block is beyond the valid address range */ - public InstructionBlock assemble(Address at, String... listing) throws AssemblySyntaxException, + public InstructionIterator assemble(Address at, String... listing) + throws AssemblySyntaxException, AssemblySemanticException, MemoryAccessException, AddressOverflowException; /** @@ -96,6 +97,7 @@ public interface Assembler { * Because all parse paths are attempted, it's possible to get many mixed results. For example, * The input line may be a valid instruction; however, there may be suggestions to continue the * line toward another valid instruction. + * * @param line the line (or partial line) to parse * @return the results of parsing */ @@ -108,8 +110,9 @@ public interface Assembler { * semantic error. Because all resolutions are attempted, it's possible to get many mixed * results. * - * NOTE: The resolved instructions are given as masks and values. Where the mask does not - * cover, you can choose any value. + * NOTE: The resolved instructions are given as masks and values. Where the mask does not cover, + * you can choose any value. + * * @param parse a parse result giving a valid tree * @param at the location of the start of the instruction * @param ctx the context register value at the start of the instruction @@ -125,8 +128,9 @@ public interface Assembler { * semantic error. Because all resolutions are attempted, it's possible to get many mixed * results. * - * NOTE: The resolved instructions are given as masks and values. Where the mask does not - * cover, you can choose any value. + * NOTE: The resolved instructions are given as masks and values. Where the mask does not cover, + * you can choose any value. + * * @param parse a parse result giving a valid tree * @param at the location of the start of the instruction * @return the results of semantic resolution @@ -138,6 +142,7 @@ public interface Assembler { * * This method works like {@link #resolveLine(Address, String, AssemblyPatternBlock)}, except * that it derives the context using {@link #getContextAt(Address)}. + * * @param at the location of the start of the instruction * @param line the textual assembly code * @return the collection of semantic resolution results @@ -152,6 +157,7 @@ public interface Assembler { * This method works like {@link #assembleLine(Address, String, AssemblyPatternBlock)}, except * that it returns all possible resolutions for the parse trees that pass the * {@link AssemblySelector}. + * * @param at the location of the start of the instruction * @param line the textual assembly code * @param ctx the context register value at the start of the instruction @@ -164,8 +170,9 @@ public interface Assembler { /** * Place a resolved (and fully-masked) instruction into the bound program. * - * This method is not valid without a program binding. Also, this method must be called during - * a program database transaction. + * This method is not valid without a program binding. Also, this method must be called during a + * program database transaction. + * * @param res the resolved and fully-masked instruction * @param at the location of the start of the instruction * @return the new {@link Instruction} code unit @@ -177,20 +184,23 @@ public interface Assembler { /** * Place an instruction into the bound program. * - * This method is not valid without a program binding. Also, this method must be called during - * a program database transaction. + * This method is not valid without a program binding. Also, this method must be called during a + * program database transaction. + * * @param insbytes the instruction data * @param at the location of the start of the instruction * @return the new {@link Instruction} code unit * @throws MemoryAccessException there is an issue writing the result to program memory */ - public Instruction patchProgram(byte[] insbytes, Address at) throws MemoryAccessException; + public InstructionIterator patchProgram(byte[] insbytes, Address at) + throws MemoryAccessException; /** * Get the context at a given address * * If there is a program binding, this will extract the actual context at the given address. * Otherwise, it will obtain the default context at the given address for the language. + * * @param addr the address * @return the context */ diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java index c3bab86f55..410117c287 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java @@ -15,6 +15,8 @@ */ package ghidra.app.plugin.assembler.sleigh; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.util.*; import ghidra.app.plugin.assembler.*; @@ -25,7 +27,8 @@ import ghidra.app.plugin.processors.sleigh.SleighLanguage; import ghidra.program.disassemble.Disassembler; import ghidra.program.disassemble.DisassemblerMessageListener; import ghidra.program.model.address.*; -import ghidra.program.model.lang.*; +import ghidra.program.model.lang.Register; +import ghidra.program.model.lang.RegisterValue; import ghidra.program.model.listing.*; import ghidra.program.model.mem.Memory; import ghidra.program.model.mem.MemoryAccessException; @@ -98,22 +101,26 @@ public class SleighAssembler implements Assembler { if (!res.getInstruction().isFullMask()) { throw new AssemblySelectionError("Selected instruction must have a full mask."); } - return patchProgram(res.getInstruction().getVals(), at); + return patchProgram(res.getInstruction().getVals(), at).next(); } @Override - public Instruction patchProgram(byte[] insbytes, Address at) throws MemoryAccessException { - listing.clearCodeUnits(at, at.add(insbytes.length - 1), false); + public InstructionIterator patchProgram(byte[] insbytes, Address at) + throws MemoryAccessException { + Address end = at.add(insbytes.length - 1); + listing.clearCodeUnits(at, end, false); memory.setBytes(at, insbytes); dis.disassemble(at, new AddressSet(at)); - return listing.getInstructionAt(at); + List result = new ArrayList<>(); + return listing.getInstructions(new AddressSet(at, end), true); } @Override - public InstructionBlock assemble(Address at, String... assembly) throws AssemblySyntaxException, - AssemblySemanticException, MemoryAccessException, AddressOverflowException { - InstructionBlock block = new InstructionBlock(at); - + public InstructionIterator assemble(Address at, String... assembly) + throws AssemblySyntaxException, AssemblySemanticException, MemoryAccessException, + AddressOverflowException { + Address start = at; + ByteArrayOutputStream buf = new ByteArrayOutputStream(); for (String part : assembly) { for (String line : part.split("\n")) { RegisterValue rv = program.getProgramContext().getDisassemblyContext(at); @@ -124,13 +131,16 @@ public class SleighAssembler implements Assembler { if (insbytes == null) { return null; } - - Instruction ins = patchProgram(insbytes, at); - block.addInstruction(ins); + try { + buf.write(insbytes); + } + catch (IOException e) { + throw new AssertionError(e); + } at = at.addNoWrap(insbytes.length); } } - return block; + return patchProgram(buf.toByteArray(), start); } @Override @@ -221,10 +231,11 @@ public class SleighAssembler implements Assembler { /** * A convenience to obtain a map of program labels strings to long values + * * @return the map * - * {@literal TODO Use a Map instead so that, if possible, symbol values can be checked} - * lest they be an invalid substitution for a given operand. + * {@literal TODO Use a Map instead so that, if possible, symbol values can be checked} + * lest they be an invalid substitution for a given operand. */ protected Map getProgramLabels() { Map labels = new HashMap<>(); From 669b14a7cfb2dc13597732f8585a97ad03dd8ac7 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Tue, 9 Feb 2021 08:27:55 -0500 Subject: [PATCH 05/22] GP-587: Testing that delay slots can be assembled. --- .../app/plugin/assembler/Assembler.java | 16 +++++ .../app/plugin/assembler/Assemblers.java | 14 ++-- .../assembler/sleigh/SleighAssembler.java | 1 - .../database/util/ProgramTransaction.java | 2 - .../assembler/sleigh/PublicAPITest.java | 66 ++++++++++++++++--- 5 files changed, 82 insertions(+), 17 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java index 8038db8620..e0bc082847 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java @@ -28,6 +28,7 @@ import ghidra.program.model.mem.MemoryAccessException; /** * The primary interface for performing assembly in Ghidra. * + *

* Use the {@link Assemblers} class to obtain a suitable implementation for a given program or * language. */ @@ -35,10 +36,12 @@ public interface Assembler { /** * Assemble a sequence of instructions and place them at the given address. * + *

* This method is only valid if the assembler is bound to a program. An instance may optionally * implement this method without a program binding. In that case, the returned instruction block * will refer to pseudo instructions. * + *

* NOTE: There must be an active transaction on the bound program for this method to succeed. * * @param at the location where the resulting instructions should be placed @@ -56,6 +59,7 @@ public interface Assembler { /** * Assemble a line instruction at the given address. * + *

* This method is valid with or without a bound program. Even if bound, the program is not * modified; however, the appropriate context information is taken from the bound program. * Without a program, the language's default context is taken at the given location. @@ -72,6 +76,7 @@ public interface Assembler { /** * Assemble a line instruction at the given address, assuming the given context. * + *

* This method works like {@link #assembleLine(Address, String)} except that it allows you to * override the assumed context at that location. * @@ -88,11 +93,13 @@ public interface Assembler { /** * Parse a line instruction. * + *

* Generally, you should just use {@link #assembleLine(Address, String)}, but if you'd like * access to the parse trees outside of an {@link AssemblySelector}, then this may be an * acceptable option. Most notably, this is an excellent way to obtain suggestions for * auto-completion. * + *

* Each item in the returned collection is either a complete parse tree, or a syntax error * Because all parse paths are attempted, it's possible to get many mixed results. For example, * The input line may be a valid instruction; however, there may be suggestions to continue the @@ -106,10 +113,12 @@ public interface Assembler { /** * Resolve a given parse tree at the given address, assuming the given context * + *

* Each item in the returned collection is either a completely resolved instruction, or a * semantic error. Because all resolutions are attempted, it's possible to get many mixed * results. * + *

* NOTE: The resolved instructions are given as masks and values. Where the mask does not cover, * you can choose any value. * @@ -124,10 +133,12 @@ public interface Assembler { /** * Resolve a given parse tree at the given address. * + *

* Each item in the returned collection is either a completely resolved instruction, or a * semantic error. Because all resolutions are attempted, it's possible to get many mixed * results. * + *

* NOTE: The resolved instructions are given as masks and values. Where the mask does not cover, * you can choose any value. * @@ -140,6 +151,7 @@ public interface Assembler { /** * Assemble a line instruction at the given address. * + *

* This method works like {@link #resolveLine(Address, String, AssemblyPatternBlock)}, except * that it derives the context using {@link #getContextAt(Address)}. * @@ -154,6 +166,7 @@ public interface Assembler { /** * Assemble a line instruction at the given address, assuming the given context. * + *

* This method works like {@link #assembleLine(Address, String, AssemblyPatternBlock)}, except * that it returns all possible resolutions for the parse trees that pass the * {@link AssemblySelector}. @@ -170,6 +183,7 @@ public interface Assembler { /** * Place a resolved (and fully-masked) instruction into the bound program. * + *

* This method is not valid without a program binding. Also, this method must be called during a * program database transaction. * @@ -184,6 +198,7 @@ public interface Assembler { /** * Place an instruction into the bound program. * + *

* This method is not valid without a program binding. Also, this method must be called during a * program database transaction. * @@ -198,6 +213,7 @@ public interface Assembler { /** * Get the context at a given address * + *

* If there is a program binding, this will extract the actual context at the given address. * Otherwise, it will obtain the default context at the given address for the language. * diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assemblers.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assemblers.java index 0780eb9936..d0cb43af4d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assemblers.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assemblers.java @@ -28,16 +28,15 @@ import ghidra.program.model.listing.Program; /** * The primary class for obtaining an {@link Assembler} for a Ghidra-supported language. * + *

* The general flow is: First, obtain an assembler for a language or program. Second, call its * {@link Assembler#assemble(Address, String...)} and related methods to perform assembly. More * advanced uses pass a {@link AssemblySelector} to control certain aspects of assembly instruction * selection, and to obtain advanced diagnostics, like detailed errors and code completion. * * 

- * {@code
  * Assembler asm = Assemblers.getAssembler(currentProgram);
  * asm.assemble(currentAddress, "ADD ...");
- * }
  *
*/ public final class Assemblers { @@ -45,6 +44,7 @@ public final class Assemblers { /** * Get a builder for the given language, possibly using a cached one. + * * @param lang the language * @return the builder for that language, if successful */ @@ -64,10 +64,11 @@ public final class Assemblers { /** * Get an assembler for the given program. * - * Provides an assembler suitable for the program's language, and bound to the program. Calls - * to its Assembler#assemble() function will cause modifications to the bound program. If this - * is the first time an assembler for the program's language has been requested, this function - * may take some time to build the assembler. + *

+ * Provides an assembler suitable for the program's language, and bound to the program. Calls to + * its Assembler#assemble() function will cause modifications to the bound program. If this is + * the first time an assembler for the program's language has been requested, this function may + * take some time to build the assembler. * * @param selector a method to select a single result from many * @param program the program for which an assembler is requested @@ -81,6 +82,7 @@ public final class Assemblers { /** * Get an assembler for the given language. * + *

* Provides a suitable assembler for the given language. Only calls to its * Assembler#assembleLine() method are valid. If this is the first time a language has been * requested, this function may take some time to build the assembler. Otherwise, it returns a diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java index 410117c287..a02cca1e2d 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/sleigh/SleighAssembler.java @@ -111,7 +111,6 @@ public class SleighAssembler implements Assembler { listing.clearCodeUnits(at, end, false); memory.setBytes(at, insbytes); dis.disassemble(at, new AddressSet(at)); - List result = new ArrayList<>(); return listing.getInstructions(new AddressSet(at, end), true); } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/util/ProgramTransaction.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/util/ProgramTransaction.java index 866a8e2547..4a4ad41de9 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/util/ProgramTransaction.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/util/ProgramTransaction.java @@ -24,12 +24,10 @@ import ghidra.program.model.listing.Program; * This is meant to be used as an idiom in a try-with-resources block: * * 

- * {@code
  * try (ProgramTransaction t = ProgramTransaction.open(program, "Demo")) {
  *     program.getMemory().....
  *     t.commit();
  * }
- * }
  *
* *

diff --git a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/app/plugin/assembler/sleigh/PublicAPITest.java b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/app/plugin/assembler/sleigh/PublicAPITest.java index 6776c32f47..721568c64f 100644 --- a/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/app/plugin/assembler/sleigh/PublicAPITest.java +++ b/Ghidra/Framework/SoftwareModeling/src/test/java/ghidra/app/plugin/assembler/sleigh/PublicAPITest.java @@ -15,36 +15,86 @@ */ package ghidra.app.plugin.assembler.sleigh; -import org.junit.Before; -import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.*; import generic.test.AbstractGenericTest; import ghidra.app.plugin.assembler.*; import ghidra.app.plugin.processors.sleigh.SleighLanguageProvider; +import ghidra.program.database.ProgramDB; +import ghidra.program.database.util.ProgramTransaction; +import ghidra.program.model.address.Address; +import ghidra.program.model.address.AddressOverflowException; import ghidra.program.model.lang.Language; import ghidra.program.model.lang.LanguageID; +import ghidra.program.model.listing.*; +import ghidra.util.exception.CancelledException; +import ghidra.util.task.TaskMonitor; public class PublicAPITest extends AbstractGenericTest { - private Language x86; + Language x86; + Language toy; + + Program program; @Before public void setUp() throws Exception { SleighLanguageProvider provider = new SleighLanguageProvider(); x86 = provider.getLanguage(new LanguageID("x86:LE:64:default")); + toy = provider.getLanguage(new LanguageID("Toy:BE:64:default")); + } + + @After + public void tearDown() throws Exception { + if (program != null) { + program.release(this); + } } @Test public void testADD0() throws AssemblySyntaxException, AssemblySemanticException { + // Mostly just test that it doesn't crash Assembler asm = Assemblers.getAssembler(x86); byte[] b = asm.assembleLine(x86.getDefaultSpace().getAddress(0x40000000), "ADD byte ptr [RBX],BL"); - printArray(b); + assertNotEquals(0, b.length); } - public static void printArray(byte[] arr) { - for (int i = 0; i < arr.length; i++) { - System.out.printf("%02x", arr[i]); + protected Address addr(long offset) { + return program.getAddressFactory().getDefaultAddressSpace().getAddress(offset); + } + + @Test + public void testAssembleWithDelaySlot() throws Exception, + AddressOverflowException, CancelledException { + program = new ProgramDB("test", toy, toy.getDefaultCompilerSpec(), this); + + InstructionIterator it; + try (ProgramTransaction tid = ProgramTransaction.open(program, "Test")) { + program.getMemory() + .createInitializedBlock(".text", addr(0x00400000), 0x1000, (byte) 0, + TaskMonitor.DUMMY, false); + Assembler asm = Assemblers.getAssembler(program); + + it = asm.assemble(addr(0x00400000), + "brds 0x00400004", + "add r0, #6"); + + tid.commit(); } - System.out.println(); + + List result = new ArrayList<>(); + while (it.hasNext()) { + result.add(it.next()); + } + + assertEquals(2, result.size()); + assertEquals("brds", result.get(0).getMnemonicString()); + assertEquals("_add", result.get(1).getMnemonicString()); } } From 3593434f28b05342a54f7ddfe1c3b6cb6bf0ee0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Harabie=C5=84?= Date: Sun, 21 Feb 2021 17:08:11 +0100 Subject: [PATCH 06/22] Fix non-ASCII characters handing in IDA 6.x exporter --- GhidraBuild/IDAPro/Python/6xx/plugins/xmlexp.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/GhidraBuild/IDAPro/Python/6xx/plugins/xmlexp.py b/GhidraBuild/IDAPro/Python/6xx/plugins/xmlexp.py index 8e1807e572..9540aba34f 100644 --- a/GhidraBuild/IDAPro/Python/6xx/plugins/xmlexp.py +++ b/GhidraBuild/IDAPro/Python/6xx/plugins/xmlexp.py @@ -246,7 +246,7 @@ class XmlExporter: elif ch == '\'' : return "'" elif ch == '"' : return """ elif ch == '\x7F': return '' - elif ord(ch) > 0x7F: return '&#x' + format((ord(ch),"x")) + ";" + elif ord(ch) > 0x7F: return '&#x' + format(ord(ch),"x") + ";" return ch From b940cdcc4fad6d9e4ab70d8f7b386481065dac59 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 8 Mar 2021 16:14:09 -0800 Subject: [PATCH 07/22] Correct typos and spacing in decompiler documentation --- .../Decompiler/src/decompile/cpp/doccore.hh | 66 +++++++++---------- .../Decompiler/src/decompile/cpp/docmain.hh | 8 +-- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/doccore.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/doccore.hh index e8f0cae6db..8640982188 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/doccore.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/doccore.hh @@ -27,7 +27,7 @@ implement these models provides the quickest inroad into obtaining an overall understanding of the code. - We list all these fundemental classes here, loosely grouped + We list all these fundamental classes here, loosely grouped as follows. There is one set of classes that describe the \e Syntax \e Trees, which are built up from the original p-code, and transformed during the decompiler's simplification process. @@ -43,18 +43,18 @@ - AddrSpace - A place within the reverse engineering model where data can be stored. The typical address spaces are \b ram, - modeling the main databus of a processor, and \b register, - modeling a processors on board registers. Data is stored a - byte at a time at \b offsets within the AddrSpace. + modeling the main databus of a processor, and \b register, + modeling a processor's on board registers. Data is stored a + byte at a time at \b offsets within the AddrSpace. . - Address - An AddrSpace and an offset within the space forms the - Address of the byte at that offset. + Address of the byte at that offset. . - Varnode - A contiguous set of bytes, given by an Address and a size, - encoding a single value in the model. In terms of SSA - syntax tree, a Varnode is also a node in the tree. + encoding a single value in the model. In terms of SSA + syntax tree, a Varnode is also a node in the tree. . - SeqNum - A \e sequence \e number that extends Address for distinguishing PcodeOps @@ -161,14 +161,14 @@ and local scope. \code - string & getName(); // get name of function - Address & getAddress(); // get Address of function's entry point - int4 numCalls(); // number of subfunctions called by this function + string & getName(); // get name of function + Address & getAddress(); // get Address of function's entry point + int4 numCalls(); // number of subfunctions called by this function FuncCallSpecs *getCallSpecs(int4 i); // get specs for one of the subfunctions - BlockGraph & getBasicBlocks(); // get the collection of basic blocks + BlockGraph & getBasicBlocks(); // get the collection of basic blocks iterator beginLoc(Address &); // Search for Varnodes in tree - iterator beginLoc(int4,Address &); // based on the Varnode's address + iterator beginLoc(int4,Address &); // based on the Varnode's address iterator beginLoc(int4,Address &,Address &,uintm); iterator beginDef(uint4,Address &); // Search for Varnode based on the // address of its defining operation @@ -221,14 +221,14 @@ array, and structure qualifiers. \code - class TypePointer : public Datatype { // pointer to (some other type) - Datatype *getBase(); // get Datatype being pointed to + class TypePointer : public Datatype { // pointer to (some other type) + Datatype *getBase(); // get Datatype being pointed to }; - class TypeArray : public Datatype { // array of (some other type) - Datatype *getBase(); // get Datatype of array element + class TypeArray : public Datatype { // array of (some other type) + Datatype *getBase(); // get Datatype of array element }; - class TypeStruct : public Datatype { // structure with fields of (some other types) - TypeField *getField(int4,int4,int4 *); // get Datatype of a field + class TypeStruct : public Datatype { // structure with fields of (some other types) + TypeField *getField(int4,int4,int4 *); // get Datatype of a field }; \endcode @@ -237,12 +237,12 @@ This is a container for Datatypes. \code - Datatype *findByName(string &); // find a Datatype by name - Datatype *getTypeVoid(); // retrieve common types + Datatype *findByName(string &); // find a Datatype by name + Datatype *getTypeVoid(); // retrieve common types Datatype *getTypeChar(); Datatype *getBase(int4 size,type_metatype); - Datatype *getTypePointer(int4,Datatype *,uint4); // get a pointer to another type - Datatype *getTypeArray(int4,Datatype *); // get an array of another type + Datatype *getTypePointer(int4,Datatype *,uint4); // get a pointer to another type + Datatype *getTypeArray(int4,Datatype *); // get an array of another type \endcode \section classhighvariable HighVariable @@ -257,7 +257,7 @@ \code int4 numInstances(); // get number of different Varnodes associated // with this variable. - Varnode * getInstance(int4); // get (one of the) Varnodes associated with + Varnode * getInstance(int4); // get (one of the) Varnodes associated with // this variable. Datatype * getType(); // get Datatype of this variable Symbol * getSymbol(); // get Symbol associated with this variable @@ -274,11 +274,11 @@ lives in a scope, has a name, and has a Datatype. \code - string & getName(); // get the name of the symbol - Datatype * getType(); // get the Datatype of the symbol - Scope * getScope(); // get the scope containing the symbol + string & getName(); // get the name of the symbol + Datatype * getType(); // get the Datatype of the symbol + Scope * getScope(); // get the scope containing the symbol SymbolEntry * getFirstWholeMap(); // get the (first) SymbolEntry associated - // with this symbol + // with this symbol \endcode \section classsymbolentry SymbolEntry @@ -300,16 +300,16 @@ This is a container for symbols. \code - SymbolEntry *findAddr(Address &,Address &); // find a Symbol by address + SymbolEntry *findAddr(Address &,Address &); // find a Symbol by address SymbolEntry *findContainer(Address &,int4,Address &); // find containing symbol - Funcdata * findFunction(Address &); // find a function by entry address - Symbol * findByName(string &); // find a Symbol by name - SymbolEntry *queryByAddr(Address &,Address &); // search for symbols across multiple scopes + Funcdata * findFunction(Address &); // find a function by entry address + Symbol * findByName(string &); // find a Symbol by name + SymbolEntry *queryByAddr(Address &,Address &); // search for symbols across multiple scopes SymbolEntry *queryContainer(Address &,int4,Address &); Funcdata * queryFunction(Address &); Scope * discoverScope(Address &,int4,Address &); // discover scope of an address - string & getName(); // get name of scope - Scope * getParent(); // get parent scope + string & getName(); // get name of scope + Scope * getParent(); // get parent scope \endcode \section classdatabase Database diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh index 9abe262728..23008d2685 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh @@ -38,7 +38,7 @@ the main code workflow. The library provides its own Register - Transfer Languate (RTL), referred to internally as \b p-code, + Transfer Language (RTL), referred to internally as \b p-code, which is designed specifically for reverse engineering applications. The disassembly of processor specific machine-code languages, and subsequent translation into \b p-code, forms @@ -275,7 +275,7 @@ about the variables it analyzes, as this kind of information is generally not present in the input binary. Some information can be gathered about a - variable, based on the instructions it is used in (.i.e + variable, based on the instructions it is used in (i.e. if it is used in a floating point instruction). Other information about type might be available from header files or from the user. Once this is gathered, the @@ -301,7 +301,7 @@ compiler would, but to simplify and normalize for easier understanding and recognition by human analysts (and follow on machine processing). Typical examples - of transforms include, copy propagation, constant + of transforms include: copy propagation, constant propagation, collecting terms, cancellation of operators and other algebraic simplifications, undoing multiplication and division optimizations, commuting @@ -373,7 +373,7 @@ Even after the initial merging of variables in phase 1, there are generally still too many for normal C code. So - the decompiler, does additional, more speculative merging. + the decompiler does additional, more speculative merging. It first tries to merge the inputs and outputs of copy operations, and then the inputs and outputs of more general operations. And finally, merging is attempted on From 58a9ff92af00d41021e074e2289c55dc38d2c3b6 Mon Sep 17 00:00:00 2001 From: Pokechu22 Date: Mon, 8 Mar 2021 16:44:13 -0800 Subject: [PATCH 08/22] Fix JavaDoc warning --- .../src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java | 1 - 1 file changed, 1 deletion(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java index 170c317d97..16a0468b89 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/elf/ElfSymbol.java @@ -111,7 +111,6 @@ public class ElfSymbol implements ByteArrayConverter { * @param reader to read symbol from * @param symbolIndex index of the symbol to read * @param symbolTable symbol table to associate the symbol to - * @param stringTable string table to read symbols from * @param header else header * @return newly created ElfSymbol * From 682bc88df07074baffbc65ee67c2a8221a21a50c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 14 Mar 2021 14:39:12 +0100 Subject: [PATCH 09/22] Fix monitor messages for constant propagation --- .../Base/ghidra_scripts/PropagateConstantReferences.java | 2 +- .../Base/ghidra_scripts/PropagateX86ConstantReferences.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Ghidra/Features/Base/ghidra_scripts/PropagateConstantReferences.java b/Ghidra/Features/Base/ghidra_scripts/PropagateConstantReferences.java index a996abab90..ed529a0cfb 100644 --- a/Ghidra/Features/Base/ghidra_scripts/PropagateConstantReferences.java +++ b/Ghidra/Features/Base/ghidra_scripts/PropagateConstantReferences.java @@ -36,7 +36,7 @@ public class PropagateConstantReferences extends GhidraScript { public void run() throws Exception { long numInstructions = currentProgram.getListing().getNumInstructions(); monitor.initialize((int) (numInstructions)); - monitor.setMessage("Constant Propogation Markup"); + monitor.setMessage("Constant Propagation Markup"); // set up the address set to restrict processing AddressSet restrictedSet = diff --git a/Ghidra/Features/Base/ghidra_scripts/PropagateX86ConstantReferences.java b/Ghidra/Features/Base/ghidra_scripts/PropagateX86ConstantReferences.java index 354c35519e..6d4379c26a 100644 --- a/Ghidra/Features/Base/ghidra_scripts/PropagateX86ConstantReferences.java +++ b/Ghidra/Features/Base/ghidra_scripts/PropagateX86ConstantReferences.java @@ -57,7 +57,7 @@ public class PropagateX86ConstantReferences extends GhidraScript { public void run() throws Exception { long numInstructions = currentProgram.getListing().getNumInstructions(); monitor.initialize((int) (numInstructions)); - monitor.setMessage("Constant Propogation Markup"); + monitor.setMessage("Constant Propagation Markup"); // set up the address set to restrict processing AddressSet restrictedSet = new AddressSet(currentSelection); From 0cc15688717fc473e72d3ee517f76958f551ec79 Mon Sep 17 00:00:00 2001 From: gtackett Date: Thu, 18 Mar 2021 09:57:45 -0400 Subject: [PATCH 10/22] Fix for #2844 re. addresses of TBLPAG and PSVPAG --- Ghidra/Processors/PIC/data/languages/PIC24.sinc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Ghidra/Processors/PIC/data/languages/PIC24.sinc b/Ghidra/Processors/PIC/data/languages/PIC24.sinc index 5392c6e1c7..4185851431 100644 --- a/Ghidra/Processors/PIC/data/languages/PIC24.sinc +++ b/Ghidra/Processors/PIC/data/languages/PIC24.sinc @@ -86,8 +86,8 @@ define ram offset=0x36 size=2 [ RCOUNT ]; # Repeat counter # define ram offset=0x38 size=2 [ DCOUNT ]; # 13 bits long DO Loop counter define ram offset=0x54 size=1 [ TBLPAG ]; # 7bit Data Table Page Address @else -define ram offset=0x31 size=1 [ TBLPAG ]; # 8bit Data Table Page Address -define ram offset=0x33 size=1 [ PSVPAG ]; # Program Memory Visibility Page Address Pointer +define ram offset=0x32 size=1 [ TBLPAG ]; # 8bit Data Table Page Address +define ram offset=0x34 size=1 [ PSVPAG ]; # Program Memory Visibility Page Address Pointer define ram offset=0x36 size=2 [ RCOUNT ]; # Repeat counter define ram offset=0x38 size=2 [ DCOUNT ]; # 13 bits long DO Loop counter define ram offset=0x3A size=3 [ DOSTART ]; From 076a4f7b858d2161d94ca6e9547836b824378259 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Fri, 19 Mar 2021 16:06:11 -0400 Subject: [PATCH 11/22] Don't send default options --- .../app/decompiler/DecompileOptions.java | 225 +++++++++--------- 1 file changed, 111 insertions(+), 114 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java index 4b1bc2edb9..e1021ed7cc 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java @@ -239,9 +239,7 @@ public class DecompileOptions { private boolean commentHeadInclude; public enum NamespaceStrategy { - Minimal("minimal", "Minimally"), - All("all", "Always"), - Never("none", "Never"); + Minimal("minimal", "Minimally"), All("all", "Always"), Never("none", "Never"); private String label; private String optionString; @@ -533,172 +531,129 @@ public class DecompileOptions { * @param program the program */ public void registerOptions(Plugin ownerPlugin, ToolOptions opt, Program program) { - opt.registerOption(PREDICATE_OPTIONSTRING, - PREDICATE_OPTIONDEFAULT, + opt.registerOption(PREDICATE_OPTIONSTRING, PREDICATE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisPredicate"), PREDICATE_OPTIONDESCRIPTION); - opt.registerOption(READONLY_OPTIONSTRING, - READONLY_OPTIONDEFAULT, + opt.registerOption(READONLY_OPTIONSTRING, READONLY_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisReadOnly"), READONLY_OPTIONDESCRIPTION); - opt.registerOption(ELIMINATE_UNREACHABLE_OPTIONSTRING, - ELIMINATE_UNREACHABLE_OPTIONDEFAULT, + opt.registerOption(ELIMINATE_UNREACHABLE_OPTIONSTRING, ELIMINATE_UNREACHABLE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisUnreachable"), ELIMINATE_UNREACHABLE_OPTIONDESCRIPTION); opt.registerOption(SIMPLIFY_DOUBLEPRECISION_OPTIONSTRING, SIMPLIFY_DOUBLEPRECISION_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisExtendedPrecision"), SIMPLIFY_DOUBLEPRECISION_OPTIONDESCRIPTION); - opt.registerOption(IGNOREUNIMPL_OPTIONSTRING, - IGNOREUNIMPL_OPTIONDEFAULT, + opt.registerOption(IGNOREUNIMPL_OPTIONSTRING, IGNOREUNIMPL_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisIgnoreUnimplemented"), IGNOREUNIMPL_OPTIONDESCRIPTION); - opt.registerOption(INFERCONSTPTR_OPTIONSTRING, - INFERCONSTPTR_OPTIONDEFAULT, + opt.registerOption(INFERCONSTPTR_OPTIONSTRING, INFERCONSTPTR_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisInferConstants"), INFERCONSTPTR_OPTIONDESCRIPTION); - opt.registerOption(ANALYZEFORLOOPS_OPTIONSTRING, - ANALYZEFORLOOPS_OPTIONDEFAULT, + opt.registerOption(ANALYZEFORLOOPS_OPTIONSTRING, ANALYZEFORLOOPS_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisForLoops"), ANALYZEFORLOOPS_OPTIONDESCRIPTION); - opt.registerOption(NULLTOKEN_OPTIONSTRING, - NULLTOKEN_OPTIONDEFAULT, - new HelpLocation(HelpTopics.DECOMPILER, "DisplayNull"), - NULLTOKEN_OPTIONDESCRIPTION); - opt.registerOption(INPLACEOP_OPTIONSTRING, - INPLACEOP_OPTIONDEFAULT, + opt.registerOption(NULLTOKEN_OPTIONSTRING, NULLTOKEN_OPTIONDEFAULT, + new HelpLocation(HelpTopics.DECOMPILER, "DisplayNull"), NULLTOKEN_OPTIONDESCRIPTION); + opt.registerOption(INPLACEOP_OPTIONSTRING, INPLACEOP_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisInPlace"), INPLACEOP_OPTIONDESCRIPTION); - opt.registerOption(ALIASBLOCK_OPTIONSTRING, - ALIASBLOCK_OPTIONDEFAULT, + opt.registerOption(ALIASBLOCK_OPTIONSTRING, ALIASBLOCK_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "AnalysisAliasBlocking"), ALIASBLOCK_OPTIONDESCRIPTION); - opt.registerOption(CONVENTION_OPTIONSTRING, - CONVENTION_OPTIONDEFAULT, + opt.registerOption(CONVENTION_OPTIONSTRING, CONVENTION_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayConvention"), CONVENTION_OPTIONDESCRIPTION); - opt.registerOption(NOCAST_OPTIONSTRING, - NOCAST_OPTIONDEFAULT, + opt.registerOption(NOCAST_OPTIONSTRING, NOCAST_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayDisableCasts"), NOCAST_OPTIONDESCRIPTION); - opt.registerOption(MAXWIDTH_OPTIONSTRING, - MAXWIDTH_OPTIONDEFAULT, - new HelpLocation(HelpTopics.DECOMPILER, "DisplayMaxChar"), - MAXWIDTH_OPTIONDESCRIPTION); - opt.registerOption(INDENTWIDTH_OPTIONSTRING, - INDENTWIDTH_OPTIONDEFAULT, + opt.registerOption(MAXWIDTH_OPTIONSTRING, MAXWIDTH_OPTIONDEFAULT, + new HelpLocation(HelpTopics.DECOMPILER, "DisplayMaxChar"), MAXWIDTH_OPTIONDESCRIPTION); + opt.registerOption(INDENTWIDTH_OPTIONSTRING, INDENTWIDTH_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayIndentLevel"), INDENTWIDTH_OPTIONDESCRIPTION); - opt.registerOption(COMMENTINDENT_OPTIONSTRING, - COMMENTINDENT_OPTIONDEFAULT, + opt.registerOption(COMMENTINDENT_OPTIONSTRING, COMMENTINDENT_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayCommentIndent"), COMMENTINDENT_OPTIONDESCRIPTION); - opt.registerOption(COMMENTSTYLE_OPTIONSTRING, - COMMENTSTYLE_OPTIONDEFAULT, + opt.registerOption(COMMENTSTYLE_OPTIONSTRING, COMMENTSTYLE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayCommentStyle"), COMMENTSTYLE_OPTIONDESCRIPTION); - opt.registerOption(COMMENTEOL_OPTIONSTRING, - COMMENTEOL_OPTIONDEFAULT, + opt.registerOption(COMMENTEOL_OPTIONSTRING, COMMENTEOL_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "CommentOptions"), COMMENTEOL_OPTIONDESCRIPTION); - opt.registerOption(COMMENTPRE_OPTIONSTRING, - COMMENTPRE_OPTIONDEFAULT, + opt.registerOption(COMMENTPRE_OPTIONSTRING, COMMENTPRE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "CommentOptions"), COMMENTPRE_OPTIONDESCRIPTION); - opt.registerOption(COMMENTPOST_OPTIONSTRING, - COMMENTPOST_OPTIONDEFAULT, + opt.registerOption(COMMENTPOST_OPTIONSTRING, COMMENTPOST_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "CommentOptions"), COMMENTPOST_OPTIONDESCRIPTION); - opt.registerOption(COMMENTPLATE_OPTIONSTRING, - COMMENTPLATE_OPTIONDEFAULT, + opt.registerOption(COMMENTPLATE_OPTIONSTRING, COMMENTPLATE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "CommentOptions"), COMMENTPLATE_OPTIONDESCRIPTION); - opt.registerOption(COMMENTWARN_OPTIONSTRING, - COMMENTWARN_OPTIONDEFAULT, + opt.registerOption(COMMENTWARN_OPTIONSTRING, COMMENTWARN_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayWarningComments"), COMMENTWARN_OPTIONDESCRIPTION); - opt.registerOption(COMMENTHEAD_OPTIONSTRING, - COMMENTHEAD_OPTIONDEFAULT, + opt.registerOption(COMMENTHEAD_OPTIONSTRING, COMMENTHEAD_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayHeaderComment"), COMMENTHEAD_OPTIONDESCRIPTION); - opt.registerOption(NAMESPACE_OPTIONSTRING, - NAMESPACE_OPTIONDEFAULT, + opt.registerOption(NAMESPACE_OPTIONSTRING, NAMESPACE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayNamespaces"), NAMESPACE_OPTIONDESCRIPTION); - opt.registerOption(INTEGERFORMAT_OPTIONSTRING, - INTEGERFORMAT_OPTIONDEFAULT, + opt.registerOption(INTEGERFORMAT_OPTIONSTRING, INTEGERFORMAT_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayIntegerFormat"), INTEGERFORMAT_OPTIONDESCRIPTION); - opt.registerOption(HIGHLIGHT_KEYWORD_MSG, - HIGHLIGHT_KEYWORD_DEF, + opt.registerOption(HIGHLIGHT_KEYWORD_MSG, HIGHLIGHT_KEYWORD_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting keywords."); - opt.registerOption(HIGHLIGHT_TYPE_MSG, - HIGHLIGHT_TYPE_DEF, + opt.registerOption(HIGHLIGHT_TYPE_MSG, HIGHLIGHT_TYPE_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting types."); - opt.registerOption(HIGHLIGHT_FUNCTION_MSG, - HIGHLIGHT_FUNCTION_DEF, + opt.registerOption(HIGHLIGHT_FUNCTION_MSG, HIGHLIGHT_FUNCTION_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting function names."); - opt.registerOption(HIGHLIGHT_COMMENT_MSG, - HIGHLIGHT_COMMENT_DEF, + opt.registerOption(HIGHLIGHT_COMMENT_MSG, HIGHLIGHT_COMMENT_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting comments."); - opt.registerOption(HIGHLIGHT_VARIABLE_MSG, - HIGHLIGHT_VARIABLE_DEF, + opt.registerOption(HIGHLIGHT_VARIABLE_MSG, HIGHLIGHT_VARIABLE_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting variables."); - opt.registerOption(HIGHLIGHT_CONST_MSG, - HIGHLIGHT_CONST_DEF, + opt.registerOption(HIGHLIGHT_CONST_MSG, HIGHLIGHT_CONST_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting constants."); - opt.registerOption(HIGHLIGHT_PARAMETER_MSG, - HIGHLIGHT_PARAMETER_DEF, + opt.registerOption(HIGHLIGHT_PARAMETER_MSG, HIGHLIGHT_PARAMETER_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting parameters."); - opt.registerOption(HIGHLIGHT_GLOBAL_MSG, - HIGHLIGHT_GLOBAL_DEF, + opt.registerOption(HIGHLIGHT_GLOBAL_MSG, HIGHLIGHT_GLOBAL_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayTokenColor"), "Color used for highlighting global variables."); - opt.registerOption(HIGHLIGHT_DEFAULT_MSG, - HIGHLIGHT_DEFAULT_DEF, + opt.registerOption(HIGHLIGHT_DEFAULT_MSG, HIGHLIGHT_DEFAULT_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayColorDefault"), "The color used when a specific color is not specified."); - opt.registerOption(CODE_VIEWER_BACKGROUND_COLOR_MSG, - CODE_VIEWER_BACKGROUND_COLOR, + opt.registerOption(CODE_VIEWER_BACKGROUND_COLOR_MSG, CODE_VIEWER_BACKGROUND_COLOR, new HelpLocation(HelpTopics.DECOMPILER, "DisplayBackgroundColor"), "The background color of the decompiler window."); - opt.registerOption(FONT_MSG, - DEFAULT_FONT, + opt.registerOption(FONT_MSG, DEFAULT_FONT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayFont"), "The font used to render text in the decompiler."); - opt.registerOption(SEARCH_HIGHLIGHT_MSG, - SEARCH_HIGHLIGHT_DEF, + opt.registerOption(SEARCH_HIGHLIGHT_MSG, SEARCH_HIGHLIGHT_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayFindHighlight"), "The color used to highlight matches using the Find Dialog."); - opt.registerOption(LINE_NUMBER_MSG, - LINE_NUMBER_DEF, + opt.registerOption(LINE_NUMBER_MSG, LINE_NUMBER_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayLineNumbers"), "Toggle for displaying line numbers in the decompiler."); - opt.registerOption(DECOMPILE_TIMEOUT, - SUGGESTED_DECOMPILE_TIMEOUT_SECS, + opt.registerOption(DECOMPILE_TIMEOUT, SUGGESTED_DECOMPILE_TIMEOUT_SECS, new HelpLocation(HelpTopics.DECOMPILER, "GeneralTimeout"), "The number of seconds to allow the decompiler to run before terminating the " + "decompiler.\nCurrently this does not affect the UI, which will run indefinitely. " + "This setting currently only affects background analysis that uses the decompiler."); - opt.registerOption(PAYLOAD_LIMIT, - SUGGESTED_MAX_PAYLOAD_BYTES, + opt.registerOption(PAYLOAD_LIMIT, SUGGESTED_MAX_PAYLOAD_BYTES, new HelpLocation(HelpTopics.DECOMPILER, "GeneralMaxPayload"), "The maximum size of the decompiler result payload in MBYtes (Suggested value: 50)."); - opt.registerOption(HIGHLIGHT_CURRENT_VARIABLE_MSG, - HIGHLIGHT_CURRENT_VARIABLE_DEF, + opt.registerOption(HIGHLIGHT_CURRENT_VARIABLE_MSG, HIGHLIGHT_CURRENT_VARIABLE_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayCurrentHighlight"), "Current variable highlight"); - opt.registerOption(CACHED_RESULTS_SIZE_MSG, - SUGGESTED_CACHED_RESULTS_SIZE, - new HelpLocation(HelpTopics.DECOMPILER, "GeneralCacheSize"), - CACHE_RESULTS_DESCRIPTION); + opt.registerOption(CACHED_RESULTS_SIZE_MSG, SUGGESTED_CACHED_RESULTS_SIZE, + new HelpLocation(HelpTopics.DECOMPILER, "GeneralCacheSize"), CACHE_RESULTS_DESCRIPTION); grabFromToolAndProgram(ownerPlugin, opt, program); } @@ -751,31 +706,73 @@ public class DecompileOptions { // Must set language early so that the object is in place before other option changes appendOption(buf, "setlanguage", displayLanguage.toString(), "", ""); - appendOption(buf, "ignoreunimplemented", ignoreunimpl ? "on" : "off", "", ""); - appendOption(buf, "inferconstptr", inferconstptr ? "on" : "off", "", ""); - appendOption(buf, "analyzeforloops", analyzeForLoops ? "on" : "off", "", ""); - appendOption(buf, "nullprinting", nullToken ? "on" : "off", "", ""); - appendOption(buf, "inplaceops", inplaceTokens ? "on" : "off", "", ""); - appendOption(buf, "aliasblock", aliasBlock.getOptionString(), "", ""); - appendOption(buf, "conventionprinting", conventionPrint ? "on" : "off", "", ""); - appendOption(buf, "nocastprinting", noCastPrint ? "on" : "off", "", ""); - appendOption(buf, "maxlinewidth", Integer.toString(maxwidth), "", ""); - appendOption(buf, "indentincrement", Integer.toString(indentwidth), "", ""); - - appendOption(buf, "commentindent", Integer.toString(commentindent), "", ""); - String curstyle = CommentStyleEnum.CPPStyle.equals(commentStyle) ? "cplusplus" : "c"; - appendOption(buf, "commentstyle", curstyle, "", ""); - appendOption(buf, "commentinstruction", "header", commentPLATEInclude ? "on" : "off", ""); - appendOption(buf, "commentinstruction", "user2", commentPREInclude ? "on" : "off", ""); - appendOption(buf, "commentinstruction", "user1", commentEOLInclude ? "on" : "off", ""); - appendOption(buf, "commentinstruction", "user3", commentPOSTInclude ? "on" : "off", ""); - appendOption(buf, "commentinstruction", "warning", commentWARNInclude ? "on" : "off", ""); - appendOption(buf, "commentheader", "header", commentHeadInclude ? "on" : "off", ""); - appendOption(buf, "commentheader", "warningheader", commentWARNInclude ? "on" : "off", ""); - - appendOption(buf, "namespacestrategy", namespaceStrategy.getOptionString(), "", ""); - - appendOption(buf, "integerformat", integerFormat.getOptionString(), "", ""); + if (ignoreunimpl) { // Must match Architecture::resetDefaultsInternal + appendOption(buf, "ignoreunimplemented", ignoreunimpl ? "on" : "off", "", ""); + } + if (!inferconstptr) { // Must match Architecture::resetDefaultsInternal + appendOption(buf, "inferconstptr", inferconstptr ? "on" : "off", "", ""); + } + if (!analyzeForLoops) { // Must match Architecture::resetDefaultsInternal + appendOption(buf, "analyzeforloops", analyzeForLoops ? "on" : "off", "", ""); + } + if (nullToken) { // Must match PrintC::resetDefaultsPrintC + appendOption(buf, "nullprinting", nullToken ? "on" : "off", "", ""); + } + if (inplaceTokens) { // Must match PrintC::resetDefaultsPrintC + appendOption(buf, "inplaceops", inplaceTokens ? "on" : "off", "", ""); + } + if (aliasBlock != AliasBlockEnum.Array) { // Must match Architecture::resetDefaultsInternal + appendOption(buf, "aliasblock", aliasBlock.getOptionString(), "", ""); + } + if (!conventionPrint) { // Must match PrintC::resetDefaultsPrintC + appendOption(buf, "conventionprinting", conventionPrint ? "on" : "off", "", ""); + } + if (noCastPrint) { // Must match PrintC::resetDefaultsPrintC + appendOption(buf, "nocastprinting", noCastPrint ? "on" : "off", "", ""); + } + if (maxwidth != 100) { // Must match EmitPrettyPrint::resetDefaultsPrettyPrint + appendOption(buf, "maxlinewidth", Integer.toString(maxwidth), "", ""); + } + if (indentwidth != 2) { // Must match EmitXml::resetDefaultsInternal + appendOption(buf, "indentincrement", Integer.toString(indentwidth), "", ""); + } + if (commentindent != 20) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentindent", Integer.toString(commentindent), "", ""); + } + if (commentStyle != CommentStyleEnum.CStyle) { // Must match PrintC::resetDefaultsPrintC + String curstyle = CommentStyleEnum.CPPStyle.equals(commentStyle) ? "cplusplus" : "c"; + appendOption(buf, "commentstyle", curstyle, "", ""); + } + if (commentPLATEInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentinstruction", "header", commentPLATEInclude ? "on" : "off", + ""); + } + if (!commentPREInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentinstruction", "user2", commentPREInclude ? "on" : "off", ""); + } + if (commentEOLInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentinstruction", "user1", commentEOLInclude ? "on" : "off", ""); + } + if (commentPOSTInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentinstruction", "user3", commentPOSTInclude ? "on" : "off", ""); + } + if (!commentWARNInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentinstruction", "warning", commentWARNInclude ? "on" : "off", + ""); + } + if (!commentHeadInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentheader", "header", commentHeadInclude ? "on" : "off", ""); + } + if (!commentWARNInclude) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "commentheader", "warningheader", commentWARNInclude ? "on" : "off", + ""); + } + if (namespaceStrategy != NamespaceStrategy.Minimal) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "namespacestrategy", namespaceStrategy.getOptionString(), "", ""); + } + if (integerFormat != IntegerFormatEnum.BestFit) { // Must match PrintLanguage::resetDefaultsInternal + appendOption(buf, "integerformat", integerFormat.getOptionString(), "", ""); + } appendOption(buf, "protoeval", protoEvalModel, "", ""); buf.append("\n"); From 7af6f237998a6239697a4887196a935f6abecaea Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Fri, 19 Mar 2021 17:36:14 -0400 Subject: [PATCH 12/22] New maxinstructions option --- .../src/main/doc/decompileplugin.xml | 11 ++ .../app/decompiler/DecompileOptions.java | 102 ++++++++++-------- 2 files changed, 71 insertions(+), 42 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/main/doc/decompileplugin.xml b/Ghidra/Features/Decompiler/src/main/doc/decompileplugin.xml index 7ea17934fb..928b8a5e26 100644 --- a/Ghidra/Features/Decompiler/src/main/doc/decompileplugin.xml +++ b/Ghidra/Features/Decompiler/src/main/doc/decompileplugin.xml @@ -2508,6 +2508,17 @@ + + Max Instructions per Function + + + This option sets a maximum number of machine instructions that the decompiler will attempt + to analyze for a single function, as a safeguard against analyzing a long sequence + of zeroes or other constant data. The decompiler will quickly throw an exception if it + traces control-flow into more than the indicated number of instructions. + + + diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java index e1021ed7cc..c71ef52159 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java @@ -75,21 +75,21 @@ public class DecompileOptions { private final static String IGNOREUNIMPL_OPTIONDESCRIPTION = "If set, instructions which do not have a p-code translation implemented are " + "treated as if they do nothing (like a NOP)"; - private final static boolean IGNOREUNIMPL_OPTIONDEFAULT = false; + private final static boolean IGNOREUNIMPL_OPTIONDEFAULT = false; // Must match Architecture::resetDefaultsInternal private boolean ignoreunimpl; private final static String INFERCONSTPTR_OPTIONSTRING = "Analysis.Infer constant pointers"; private final static String INFERCONSTPTR_OPTIONDESCRIPTION = "If set, constants which are not being explicitly used as pointers, but which can be interpreted " + "as a legitimate address, will still be treated as having a pointer datatype"; - private final static boolean INFERCONSTPTR_OPTIONDEFAULT = true; + private final static boolean INFERCONSTPTR_OPTIONDEFAULT = true; // Must match Architecture::resetDefaultsInternal private boolean inferconstptr; private final static String ANALYZEFORLOOPS_OPTIONSTRING = "Analysis.Recover -for- loops"; private final static String ANALYZEFORLOOPS_OPTIONDESCRIPTION = "If set, the decompiler attempts to recover for-loop variables, including their initializer, condition, " + "and incrementer statements. Loop variable bounds are displayed as a formal -for- loop header"; - private final static boolean ANALYZEFORLOOPS_OPTIONDEFAULT = true; + private final static boolean ANALYZEFORLOOPS_OPTIONDEFAULT = true; // Must match Architecture::resetDefaultsInternal private boolean analyzeForLoops; private final static String NULLTOKEN_OPTIONSTRING = "Display.Print 'NULL' for null pointers"; @@ -97,7 +97,7 @@ public class DecompileOptions { "If set, any zero valued pointer (null pointer) will " + "be printed using the token 'NULL'. Otherwise, a cast " + "of the number '0' is printed."; - private final static boolean NULLTOKEN_OPTIONDEFAULT = false; + private final static boolean NULLTOKEN_OPTIONDEFAULT = false; // Must match PrintC::resetDefaultsPrintC private boolean nullToken; private final static String INPLACEOP_OPTIONSTRING = @@ -105,7 +105,7 @@ public class DecompileOptions { private final static String INPLACEOP_OPTIONDESCRIPTION = "If set the inplace assignment operators will be used " + "for appropriate expressions. '+=' '*=' '&=' '<<=' etc."; - private final static boolean INPLACEOP_OPTIONDEFAULT = false; + private final static boolean INPLACEOP_OPTIONDEFAULT = false; // Must match PrintC::resetDefaultsPrintC private boolean inplaceTokens; private final static String ALIASBLOCK_OPTIONSTRING = "Analysis.Alias Blocking"; @@ -137,40 +137,40 @@ public class DecompileOptions { } } - private final static AliasBlockEnum ALIASBLOCK_OPTIONDEFAULT = AliasBlockEnum.Array; + private final static AliasBlockEnum ALIASBLOCK_OPTIONDEFAULT = AliasBlockEnum.Array; // Must match Architecture::resetDefaultsInternal private AliasBlockEnum aliasBlock; private final static String CONVENTION_OPTIONSTRING = "Display.Print calling convention name"; private final static String CONVENTION_OPTIONDESCRIPTION = "If set, the names of callling conventions (which differ " + "from the default) will be printed as part of the function prototype."; - private final static boolean CONVENTION_OPTIONDEFAULT = true; + private final static boolean CONVENTION_OPTIONDEFAULT = true; // Must match PrintC::resetDefaultsPrintC private boolean conventionPrint; private final static String NOCAST_OPTIONSTRING = "Display.Disable printing of type casts"; private final static String NOCAST_OPTIONDESCRIPTION = "If set, any C style type cast recovered by the decompiler will not be displayed. " + "The resulting C syntax may not parse correctly."; - private final static boolean NOCAST_OPTIONDEFAULT = false; + private final static boolean NOCAST_OPTIONDEFAULT = false; // Must match PrintC::resetDefaultsPrintC private boolean noCastPrint; private final static String MAXWIDTH_OPTIONSTRING = "Display.Maximum characters in a code line"; private final static String MAXWIDTH_OPTIONDESCRIPTION = "Maximum number of characters allowed per line before " + "before line breaks are forced."; - private final static int MAXWIDTH_OPTIONDEFAULT = 100; + private final static int MAXWIDTH_OPTIONDEFAULT = 100; // Must match EmitPrettyPrint::resetDefaultsPrettyPrint private int maxwidth; private final static String INDENTWIDTH_OPTIONSTRING = "Display.Number of characters per indent level"; private final static String INDENTWIDTH_OPTIONDESCRIPTION = "Number of characters indented for each level of control-flow " + "or scope nesting"; - private final static int INDENTWIDTH_OPTIONDEFAULT = 2; + private final static int INDENTWIDTH_OPTIONDEFAULT = 2; // Must match EmitXml::resetDefaultsInternal private int indentwidth; private final static String COMMENTINDENT_OPTIONSTRING = "Display.Comment line indent level"; private final static String COMMENTINDENT_OPTIONDESCRIPTION = "Number of characters each line of comments is indented"; - private final static int COMMENTINDENT_OPTIONDEFAULT = 20; + private final static int COMMENTINDENT_OPTIONDEFAULT = 20; // Must match PrintLanguage::resetDefaultsInternal private int commentindent; private final static String COMMENTSTYLE_OPTIONSTRING = "Display.Comment style"; @@ -178,6 +178,7 @@ public class DecompileOptions { "Choice between either the C style comments /* */ or C++ style // "; public static final int SUGGESTED_DECOMPILE_TIMEOUT_SECS = 30; public static final int SUGGESTED_MAX_PAYLOAD_BYTES = 50; + public static final int SUGGESTED_MAX_INSTRUCTIONS = 100000; // Must match Architecture::resetDefaultsInternal public enum CommentStyleEnum { @@ -195,47 +196,47 @@ public class DecompileOptions { } } - private final static CommentStyleEnum COMMENTSTYLE_OPTIONDEFAULT = CommentStyleEnum.CStyle; + private final static CommentStyleEnum COMMENTSTYLE_OPTIONDEFAULT = CommentStyleEnum.CStyle; // Must match PrintC::resetDefaultsPrintC private CommentStyleEnum commentStyle; private final static String COMMENTPRE_OPTIONSTRING = "Display.Display PRE comments"; private final static String COMMENTPRE_OPTIONDESCRIPTION = "If set, disassembly pre-instruction (PRE) comments are displayed " + "in the decompiler C output"; - private final static boolean COMMENTPRE_OPTIONDEFAULT = true; + private final static boolean COMMENTPRE_OPTIONDEFAULT = true; // Must match PrintLanguage::resetDefaultsInternal private boolean commentPREInclude; private final static String COMMENTPLATE_OPTIONSTRING = "Display.Display PLATE comments"; private final static String COMMENTPLATE_OPTIONDESCRIPTION = "If set, disassembly plate comments are displayed " + "in the decompiler C output"; - private final static boolean COMMENTPLATE_OPTIONDEFAULT = false; + private final static boolean COMMENTPLATE_OPTIONDEFAULT = false; // Must match PrintLanguage::resetDefaultsInternal private boolean commentPLATEInclude; private final static String COMMENTPOST_OPTIONSTRING = "Display.Display POST comments"; private final static String COMMENTPOST_OPTIONDESCRIPTION = "If set, disassembly post-instruction (POST) comments are displayed " + "in the decompiler C output"; - private final static boolean COMMENTPOST_OPTIONDEFAULT = false; + private final static boolean COMMENTPOST_OPTIONDEFAULT = false; // Must match PrintLanguage::resetDefaultsInternal private boolean commentPOSTInclude; private final static String COMMENTEOL_OPTIONSTRING = "Display.Display EOL comments"; private final static String COMMENTEOL_OPTIONDESCRIPTION = "If set, disassembly end-of-line (EOL) comments are displayed " + "in the decompiler C output"; - private final static boolean COMMENTEOL_OPTIONDEFAULT = false; + private final static boolean COMMENTEOL_OPTIONDEFAULT = false; // Must match PrintLanguage::resetDefaultsInternal private boolean commentEOLInclude; private final static String COMMENTWARN_OPTIONSTRING = "Display.Display Warning comments"; private final static String COMMENTWARN_OPTIONDESCRIPTION = "If set, warnings generated by the decompiler embedded in the displayed " + "code as comments"; - private final static boolean COMMENTWARN_OPTIONDEFAULT = true; + private final static boolean COMMENTWARN_OPTIONDEFAULT = true; // Must match PrintLanguage::resetDefaultsInternal private boolean commentWARNInclude; private final static String COMMENTHEAD_OPTIONSTRING = "Display.Display Header comment"; private final static String COMMENTHEAD_OPTIONDESCRIPTION = "If set, the entry point plate comment is displayed as " + "a function header comment."; - private final static boolean COMMENTHEAD_OPTIONDEFAULT = true; + private final static boolean COMMENTHEAD_OPTIONDEFAULT = true; // Must match PrintLanguage::resetDefaultsInternal private boolean commentHeadInclude; public enum NamespaceStrategy { @@ -262,7 +263,7 @@ public class DecompileOptions { private final static String NAMESPACE_OPTIONSTRING = "Display.Display Namespaces"; private final static String NAMESPACE_OPTIONDESCRIPTION = "Choose how/if namespace tokens should be displayed along with symbol names"; - private final static NamespaceStrategy NAMESPACE_OPTIONDEFAULT = NamespaceStrategy.Minimal; + private final static NamespaceStrategy NAMESPACE_OPTIONDEFAULT = NamespaceStrategy.Minimal; // Must match PrintLanguage::resetDefaultsInternal private NamespaceStrategy namespaceStrategy; private final static String INTEGERFORMAT_OPTIONSTRING = "Display.Integer format"; @@ -293,7 +294,7 @@ public class DecompileOptions { } } - private final static IntegerFormatEnum INTEGERFORMAT_OPTIONDEFAULT = IntegerFormatEnum.BestFit; + private final static IntegerFormatEnum INTEGERFORMAT_OPTIONDEFAULT = IntegerFormatEnum.BestFit; // Must match PrintLanguage::resetDefaultsInternal private IntegerFormatEnum integerFormat; private final static Color HIGHLIGHT_MIDDLE_MOUSE_DEF = new Color(255, 255, 0, 128); @@ -354,10 +355,12 @@ public class DecompileOptions { private final static String LINE_NUMBER_MSG = "Display.Display Line Numbers"; private final static String DECOMPILE_TIMEOUT = "Decompiler Timeout (seconds)"; private final static String PAYLOAD_LIMIT = "Decompiler Max-Payload (MBytes)"; + private final static String MAX_INSTRUCTIONS = "Max Instructions per Function"; private final static Boolean LINE_NUMBER_DEF = Boolean.TRUE; private boolean displayLineNumbers; private int decompileTimeoutSeconds; private int payloadLimitMBytes; + private int maxIntructionsPer; private int cachedResultsSize; private DecompilerLanguage displayLanguage; // Output language displayed by the decompiler @@ -405,6 +408,7 @@ public class DecompileOptions { protoEvalModel = "default"; decompileTimeoutSeconds = SUGGESTED_DECOMPILE_TIMEOUT_SECS; payloadLimitMBytes = SUGGESTED_MAX_PAYLOAD_BYTES; + maxIntructionsPer = SUGGESTED_MAX_INSTRUCTIONS; cachedResultsSize = SUGGESTED_CACHED_RESULTS_SIZE; } @@ -470,6 +474,7 @@ public class DecompileOptions { displayLineNumbers = opt.getBoolean(LINE_NUMBER_MSG, LINE_NUMBER_DEF); decompileTimeoutSeconds = opt.getInt(DECOMPILE_TIMEOUT, SUGGESTED_DECOMPILE_TIMEOUT_SECS); payloadLimitMBytes = opt.getInt(PAYLOAD_LIMIT, SUGGESTED_MAX_PAYLOAD_BYTES); + maxIntructionsPer = opt.getInt(MAX_INSTRUCTIONS, SUGGESTED_MAX_INSTRUCTIONS); cachedResultsSize = opt.getInt(CACHED_RESULTS_SIZE_MSG, SUGGESTED_CACHED_RESULTS_SIZE); grabFromToolOptions(ownerPlugin); @@ -649,6 +654,9 @@ public class DecompileOptions { opt.registerOption(PAYLOAD_LIMIT, SUGGESTED_MAX_PAYLOAD_BYTES, new HelpLocation(HelpTopics.DECOMPILER, "GeneralMaxPayload"), "The maximum size of the decompiler result payload in MBYtes (Suggested value: 50)."); + opt.registerOption(MAX_INSTRUCTIONS, SUGGESTED_MAX_INSTRUCTIONS, + new HelpLocation(HelpTopics.DECOMPILER, "GeneralMaxInstruction"), + "The maximum number of instructions decompiled in a single function"); opt.registerOption(HIGHLIGHT_CURRENT_VARIABLE_MSG, HIGHLIGHT_CURRENT_VARIABLE_DEF, new HelpLocation(HelpTopics.DECOMPILER, "DisplayCurrentHighlight"), "Current variable highlight"); @@ -706,74 +714,76 @@ public class DecompileOptions { // Must set language early so that the object is in place before other option changes appendOption(buf, "setlanguage", displayLanguage.toString(), "", ""); - if (ignoreunimpl) { // Must match Architecture::resetDefaultsInternal + if (ignoreunimpl != IGNOREUNIMPL_OPTIONDEFAULT) { appendOption(buf, "ignoreunimplemented", ignoreunimpl ? "on" : "off", "", ""); } - if (!inferconstptr) { // Must match Architecture::resetDefaultsInternal + if (inferconstptr != INFERCONSTPTR_OPTIONDEFAULT) { appendOption(buf, "inferconstptr", inferconstptr ? "on" : "off", "", ""); } - if (!analyzeForLoops) { // Must match Architecture::resetDefaultsInternal + if (analyzeForLoops != ANALYZEFORLOOPS_OPTIONDEFAULT) { appendOption(buf, "analyzeforloops", analyzeForLoops ? "on" : "off", "", ""); } - if (nullToken) { // Must match PrintC::resetDefaultsPrintC + if (nullToken != NULLTOKEN_OPTIONDEFAULT) { appendOption(buf, "nullprinting", nullToken ? "on" : "off", "", ""); } - if (inplaceTokens) { // Must match PrintC::resetDefaultsPrintC + if (inplaceTokens != INPLACEOP_OPTIONDEFAULT) { appendOption(buf, "inplaceops", inplaceTokens ? "on" : "off", "", ""); } - if (aliasBlock != AliasBlockEnum.Array) { // Must match Architecture::resetDefaultsInternal + if (aliasBlock != ALIASBLOCK_OPTIONDEFAULT) { appendOption(buf, "aliasblock", aliasBlock.getOptionString(), "", ""); } - if (!conventionPrint) { // Must match PrintC::resetDefaultsPrintC + if (conventionPrint != CONVENTION_OPTIONDEFAULT) { appendOption(buf, "conventionprinting", conventionPrint ? "on" : "off", "", ""); } - if (noCastPrint) { // Must match PrintC::resetDefaultsPrintC + if (noCastPrint != NOCAST_OPTIONDEFAULT) { appendOption(buf, "nocastprinting", noCastPrint ? "on" : "off", "", ""); } - if (maxwidth != 100) { // Must match EmitPrettyPrint::resetDefaultsPrettyPrint + if (maxwidth != MAXWIDTH_OPTIONDEFAULT) { appendOption(buf, "maxlinewidth", Integer.toString(maxwidth), "", ""); } - if (indentwidth != 2) { // Must match EmitXml::resetDefaultsInternal + if (indentwidth != INDENTWIDTH_OPTIONDEFAULT) { appendOption(buf, "indentincrement", Integer.toString(indentwidth), "", ""); } - if (commentindent != 20) { // Must match PrintLanguage::resetDefaultsInternal + if (commentindent != COMMENTINDENT_OPTIONDEFAULT) { appendOption(buf, "commentindent", Integer.toString(commentindent), "", ""); } - if (commentStyle != CommentStyleEnum.CStyle) { // Must match PrintC::resetDefaultsPrintC + if (commentStyle != COMMENTSTYLE_OPTIONDEFAULT) { String curstyle = CommentStyleEnum.CPPStyle.equals(commentStyle) ? "cplusplus" : "c"; appendOption(buf, "commentstyle", curstyle, "", ""); } - if (commentPLATEInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentPLATEInclude != COMMENTPLATE_OPTIONDEFAULT) { appendOption(buf, "commentinstruction", "header", commentPLATEInclude ? "on" : "off", ""); } - if (!commentPREInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentPREInclude != COMMENTPRE_OPTIONDEFAULT) { appendOption(buf, "commentinstruction", "user2", commentPREInclude ? "on" : "off", ""); } - if (commentEOLInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentEOLInclude != COMMENTEOL_OPTIONDEFAULT) { appendOption(buf, "commentinstruction", "user1", commentEOLInclude ? "on" : "off", ""); } - if (commentPOSTInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentPOSTInclude != COMMENTPOST_OPTIONDEFAULT) { appendOption(buf, "commentinstruction", "user3", commentPOSTInclude ? "on" : "off", ""); } - if (!commentWARNInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentWARNInclude != COMMENTWARN_OPTIONDEFAULT) { appendOption(buf, "commentinstruction", "warning", commentWARNInclude ? "on" : "off", ""); } - if (!commentHeadInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentHeadInclude != COMMENTHEAD_OPTIONDEFAULT) { appendOption(buf, "commentheader", "header", commentHeadInclude ? "on" : "off", ""); } - if (!commentWARNInclude) { // Must match PrintLanguage::resetDefaultsInternal + if (commentWARNInclude != COMMENTWARN_OPTIONDEFAULT) { appendOption(buf, "commentheader", "warningheader", commentWARNInclude ? "on" : "off", ""); } - if (namespaceStrategy != NamespaceStrategy.Minimal) { // Must match PrintLanguage::resetDefaultsInternal + if (namespaceStrategy != NAMESPACE_OPTIONDEFAULT) { appendOption(buf, "namespacestrategy", namespaceStrategy.getOptionString(), "", ""); } - if (integerFormat != IntegerFormatEnum.BestFit) { // Must match PrintLanguage::resetDefaultsInternal + if (integerFormat != INTEGERFORMAT_OPTIONDEFAULT) { appendOption(buf, "integerformat", integerFormat.getOptionString(), "", ""); } - + if (maxIntructionsPer != SUGGESTED_MAX_INSTRUCTIONS) { + appendOption(buf, "maxinstruction", Integer.toString(maxIntructionsPer), "", ""); + } appendOption(buf, "protoeval", protoEvalModel, "", ""); buf.append("\n"); return buf.toString(); @@ -955,6 +965,14 @@ public class DecompileOptions { payloadLimitMBytes = mbytes; } + public int getMaxInstructions() { + return maxIntructionsPer; + } + + public void setMaxInstructions(int num) { + maxIntructionsPer = num; + } + public CommentStyleEnum getCommentStyle() { return commentStyle; } From 996f052a7901d74e51212a69adc10f144bc8435f Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Tue, 16 Mar 2021 16:31:03 -0400 Subject: [PATCH 13/22] Adjustments to onlyOpUse --- .../src/decompile/cpp/architecture.cc | 2 +- .../src/decompile/cpp/coreaction.cc | 2 +- .../Decompiler/src/decompile/cpp/fspec.cc | 4 +- .../Decompiler/src/decompile/cpp/funcdata.hh | 12 +- .../src/decompile/cpp/funcdata_varnode.cc | 114 +++++++++++++----- .../Decompiler/src/decompile/cpp/varnode.hh | 7 ++ 6 files changed, 105 insertions(+), 36 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/architecture.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/architecture.cc index 4a7c40afe6..e04d172dab 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/architecture.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/architecture.cc @@ -27,7 +27,7 @@ vector ArchitectureCapability::thelist; const uint4 ArchitectureCapability::majorversion = 4; -const uint4 ArchitectureCapability::minorversion = 0; +const uint4 ArchitectureCapability::minorversion = 1; /// This builds a list of just the ArchitectureCapability extensions void ArchitectureCapability::initialize(void) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc index b9c43246fe..28dcd45df3 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc @@ -1852,7 +1852,7 @@ int4 ActionReturnRecovery::apply(Funcdata &data) int4 slot = trial.getSlot(); vn = op->getIn(slot); if (ancestorReal.execute(op,slot,&trial,false)) - if (data.ancestorOpUse(maxancestor,vn,op,trial)) + if (data.ancestorOpUse(maxancestor,vn,op,trial,0)) trial.markActive(); // This varnode sees active use as a parameter count += 1; } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc index bb40b24eef..08545ef28a 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc @@ -4744,7 +4744,7 @@ void FuncCallSpecs::checkInputTrialUse(Funcdata &data,AliasChecker &aliascheck) trial.markNoUse(); } else if (ancestorReal.execute(op,slot,&trial,false)) { - if (data.ancestorOpUse(maxancestor,vn,op,trial)) + if (data.ancestorOpUse(maxancestor,vn,op,trial,0)) trial.markActive(); else trial.markInactive(); @@ -4754,7 +4754,7 @@ void FuncCallSpecs::checkInputTrialUse(Funcdata &data,AliasChecker &aliascheck) } else { if (ancestorReal.execute(op,slot,&trial,true)) { - if (data.ancestorOpUse(maxancestor,vn,op,trial)) { + if (data.ancestorOpUse(maxancestor,vn,op,trial,0)) { trial.markActive(); if (trial.hasCondExeEffect()) activeinput.markNeedsFinalCheck(); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh index dc5c877889..39d22630cd 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh @@ -58,6 +58,11 @@ class Funcdata { baddata_present = 0x800, ///< Set if function flowed into bad data double_precis_on = 0x1000 ///< Set if we are performing double precision recovery }; + enum { + traverse_actionalt = 1, ///< Alternate path traverses a solid action or \e non-incidental COPY + traverse_indirect = 2, ///< Main path traverses an INDIRECT + traverse_indirectalt = 4 ///< Alternate path traverses an INDIRECT + }; uint4 flags; ///< Boolean properties associated with \b this function uint4 clean_up_index; ///< Creation index of first Varnode created after start of cleanup uint4 high_level_index; ///< Creation index of first Varnode created after HighVariables are created @@ -116,6 +121,7 @@ class Funcdata { void nodeSplitCloneVarnode(PcodeOp *op,PcodeOp *newop); void nodeSplitRawDuplicate(BlockBasic *b,BlockBasic *bprime); void nodeSplitInputPatch(BlockBasic *b,BlockBasic *bprime,int4 inedge); + static bool isAlternatePathValid(const Varnode *vn,uint4 flags); static bool descendantsOutside(Varnode *vn); static void saveVarnodeXml(ostream &s,VarnodeLocSet::const_iterator iter,VarnodeLocSet::const_iterator enditer); static bool checkIndirectUse(Varnode *vn); @@ -363,9 +369,9 @@ public: HighVariable *findHigh(const string &name) const; ///< Find a high-level variable by name void mapGlobals(void); ///< Make sure there is a Symbol entry for all global Varnodes - bool checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,const ParamTrial &trial) const; - bool onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial) const; - bool ancestorOpUse(int4 maxlevel,const Varnode *invn,const PcodeOp *op,ParamTrial &trial) const; + bool checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,uint4 flags,const ParamTrial &trial) const; + bool onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial,uint4 mainFlags) const; + bool ancestorOpUse(int4 maxlevel,const Varnode *invn,const PcodeOp *op,ParamTrial &trial,uint4 mainFlags) const; bool syncVarnodesWithSymbols(const ScopeLocal *lm,bool typesyes); void transferVarnodeProperties(Varnode *vn,Varnode *newVn,int4 lsbOffset); bool fillinReadOnly(Varnode *vn); ///< Replace the given Varnode with its (constant) value in the load image diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc index c01052b5ad..00258dcb96 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc @@ -4,9 +4,9 @@ * 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. @@ -189,7 +189,7 @@ Varnode *Funcdata::newVarnodeSpace(AddrSpace *spc) { Datatype *ct = glb->types->getBase(sizeof(spc),TYPE_UNKNOWN); - + Varnode *vn = vbank.create(sizeof(spc),glb->createConstFromSpace(spc),ct); assignHigh(vn); return vn; @@ -357,7 +357,7 @@ Varnode *Funcdata::setInputVarnode(Varnode *vn) } } } - + vn = vbank.setInput(vn); setVarnodeProperties(vn); uint4 effecttype = funcp.hasEffect(vn->getAddr(),vn->getSize()); @@ -393,7 +393,7 @@ void Funcdata::adjustInputVarnodes(const Address &addr,int4 size) throw LowlevelError("Cannot properly adjust input varnodes"); inlist.push_back(vn); } - + for(uint4 i=0;igetAddr(),vn->getSize(),false); @@ -405,7 +405,7 @@ void Funcdata::adjustInputVarnodes(const Address &addr,int4 size) Varnode *newvn = newVarnodeOut(vn->getSize(),vn->getAddr(),subop); // newvn must not be free in order to give all vn's descendants opInsertBegin(subop,(BlockBasic *)bblocks.getBlock(0)); - totalReplace(vn,newvn); + totalReplace(vn,newvn); deleteVarnode(vn); // Get rid of old input before creating new input inlist[i] = newvn; } @@ -546,7 +546,7 @@ bool Funcdata::fillinReadOnly(Varnode *vn) vn->clearFlags(Varnode::readonly); // Treat as writeable return true; } - + if (vn->getSpace()->isBigEndian()) { // Big endian res = 0; for(int4 i=0;igetSize();++i) { @@ -1472,6 +1472,30 @@ void Funcdata::mapGlobals(void) warningHeader("Globals starting with '_' overlap smaller symbols at the same address"); } +/// \brief Return \b true if the alternate path looks more valid than the main path. +/// +/// Two different paths from a common Varnode each terminate at a CALL, CALLIND, or RETURN. +/// Evaluate which path most likely represents actual parameter/return value passing, +/// based on traversal information about each path. +/// \param vn is the Varnode terminating the \e alternate path +/// \param flags indicates traversals for both paths +/// \return \b true if the alternate path is preferred +bool Funcdata::isAlternatePathValid(const Varnode *vn,uint4 flags) + +{ + if ((flags & (traverse_indirect | traverse_indirectalt)) == traverse_indirect) + // If main path traversed an INDIRECT but the alternate did not + return true; // Main path traversed INDIRECT, alternate did not + if ((flags & (traverse_indirect | traverse_indirectalt)) == traverse_indirectalt) + return false; // Alternate path traversed INDIRECT, main did not + if ((flags & traverse_actionalt) != 0) + return true; // Alternate path traversed a dedicated COPY + if (vn->loneDescend() == (PcodeOp*)0) return false; + const PcodeOp *op = vn->getDef(); + if (op == (PcodeOp*)0) return true; + return !op->isMarker(); // MULTIEQUAL or INDIRECT indicates multiple values +} + /// \brief Test for legitimate double use of a parameter trial /// /// The given trial is a \e putative input to first CALL, but can also trace its data-flow @@ -1480,9 +1504,10 @@ void Funcdata::mapGlobals(void) /// \param opmatch is the first CALL linked to the trial /// \param op is the second CALL /// \param vn is the Varnode parameter for the second CALL +/// \param flags indicates what p-code ops were crossed to reach \e vn /// \param trial is the given parameter trial /// \return \b true for a legitimate double use -bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,const ParamTrial &trial) const +bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,uint4 flags,const ParamTrial &trial) const { int4 j = op->getSlot(vn); @@ -1508,10 +1533,16 @@ bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const } } } - + if (fc->isInputActive()) { const ParamTrial &curtrial( fc->getActiveInput()->getTrialForInputVarnode(j) ); - if ((!curtrial.isChecked())||(!curtrial.isActive())) return true; + if (curtrial.isChecked()) { + if (curtrial.isActive()) + return false; + } + else if (isAlternatePathValid(vn,flags)) + return false; + return true; } return false; } @@ -1523,28 +1554,31 @@ bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const /// \param invn is the given Varnode /// \param opmatch is the putative CALL op using the Varnode for parameter passing /// \param trial is the parameter trial object associated with the Varnode +/// \param mainFlags are flags describing traversals along the \e main path, from \e invn to \e opmatch /// \return \b true if the Varnode seems only to be used as parameter to \b opmatch -bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial) const +bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial,uint4 mainFlags) const { - vector varlist; + vector varlist; list::const_iterator iter; const Varnode *vn,*subvn; const PcodeOp *op; int4 i; bool res = true; + varlist.reserve(64); invn->setMark(); // Marks prevent infinite loops - varlist.push_back(invn); - - i = 0; - while(i < varlist.size()) { - vn = varlist[i++]; + varlist.emplace_back(invn,mainFlags); + + for(i=0;i < varlist.size();++i) { + vn = varlist[i].vn; + uint4 baseFlags = varlist[i].flags; for(iter=vn->descend.begin();iter!=vn->descend.end();++iter) { op = *iter; if (op == opmatch) { if (op->getIn(trial.getSlot())==vn) continue; } + uint4 curFlags = baseFlags; switch(op->code()) { case CPUI_BRANCH: // These ops define a USE of a variable case CPUI_CBRANCH: @@ -1555,17 +1589,39 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT break; case CPUI_CALL: case CPUI_CALLIND: - if (checkCallDoubleUse(opmatch,op,vn,trial)) continue; + if (checkCallDoubleUse(opmatch,op,vn,curFlags,trial)) continue; res = false; break; + case CPUI_INDIRECT: + curFlags |= Funcdata::traverse_indirectalt; + break; + case CPUI_COPY: + if ((op->getOut()->getSpace()->getType()!=IPTR_INTERNAL)&&!op->isIncidentalCopy()&&!vn->isIncidentalCopy()) { + curFlags |= Funcdata::traverse_actionalt; + } + break; case CPUI_RETURN: if (opmatch->code()==CPUI_RETURN) { // Are we in a different return if (op->getIn(trial.getSlot())==vn) // But at the same slot continue; } + else if (activeoutput != (ParamActive *)0) { // Are we in the middle of analyzing returns + if (op->getIn(0) != vn) { // Unless we hold actual return value + if (!isAlternatePathValid(vn,curFlags)) + continue; // Don't consider this a "use" + } + } res = false; break; + case CPUI_MULTIEQUAL: + case CPUI_PIECE: + case CPUI_SUBPIECE: + case CPUI_INT_SEXT: + case CPUI_INT_ZEXT: + case CPUI_CAST: + break; default: + curFlags |= Funcdata::traverse_actionalt; break; } if (!res) break; @@ -1576,7 +1632,7 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT break; } if (!subvn->isMark()) { - varlist.push_back(subvn); + varlist.emplace_back(subvn,curFlags); subvn->setMark(); } } @@ -1584,7 +1640,7 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT if (!res) break; } for(i=0;iclearMark(); + varlist[i].vn->clearMark(); return res; } @@ -1596,9 +1652,10 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT /// \param invn is the given trial Varnode to test /// \param op is the given CALL or RETURN /// \param trial is the associated parameter trial object +/// \param mainFlags describes traversals along the path from \e invn to \e op /// \return \b true if the Varnode is only used for the CALL/RETURN bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn, - const PcodeOp *op,ParamTrial &trial) const + const PcodeOp *op,ParamTrial &trial,uint4 mainFlags) const { int4 i; @@ -1610,9 +1667,9 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn, if (!invn->isTypeLock()) return false; // If the input is typelocked // this is as good as being written - return onlyOpUse(invn,op,trial); // Test if varnode is only used in op + return onlyOpUse(invn,op,trial,mainFlags); // Test if varnode is only used in op } - + const PcodeOp *def = invn->getDef(); switch(def->code()) { case CPUI_INDIRECT: @@ -1620,7 +1677,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn, // as an "only use" if (def->isIndirectCreation()) return false; - return ancestorOpUse(maxlevel-1,def->getIn(0),op,trial); + return ancestorOpUse(maxlevel-1,def->getIn(0),op,trial,mainFlags | Funcdata::traverse_indirect); case CPUI_MULTIEQUAL: // Check if there is any ancestor whose only // use is in this op @@ -1628,7 +1685,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn, def->setMark(); // Mark that this MULTIEQUAL is on the path // Note: onlyOpUse is using Varnode::setMark for(i=0;inumInput();++i) { - if (ancestorOpUse(maxlevel-1,def->getIn(i),op,trial)) { + if (ancestorOpUse(maxlevel-1,def->getIn(i),op,trial, mainFlags)) { def->clearMark(); return true; } @@ -1637,13 +1694,12 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn, return false; case CPUI_COPY: if ((invn->getSpace()->getType()==IPTR_INTERNAL)||def->isIncidentalCopy()||def->getIn(0)->isIncidentalCopy()) { - if (!ancestorOpUse(maxlevel-1,def->getIn(0),op,trial)) return false; - return true; + return ancestorOpUse(maxlevel-1,def->getIn(0),op,trial,mainFlags); } break; case CPUI_PIECE: // Concatenation tends to be artificial, so recurse through the least significant part - return ancestorOpUse(maxlevel-1,def->getIn(1),op,trial); + return ancestorOpUse(maxlevel-1,def->getIn(1),op,trial,mainFlags); case CPUI_SUBPIECE: // This is a rather kludgy way to get around where a DIV (or other similar) instruction // causes a register that looks like the high precision piece of the function return @@ -1664,7 +1720,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn, break; } // This varnode must be top ancestor at this point - return onlyOpUse(invn,op,trial); // Test if varnode is only used in op + return onlyOpUse(invn,op,trial,mainFlags); // Test if varnode is only used in op } /// \return \b true if there are two input flows, one of which is a normal \e solid flow diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/varnode.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/varnode.hh index af6366a980..3abcce7b9c 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/varnode.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/varnode.hh @@ -376,6 +376,13 @@ public: #endif }; +/// \brief Node for a forward traversal of a Varnode expression +struct TraverseNode { + const Varnode *vn; ///< Varnode at the point of traversal + uint4 flags; ///< Flags associated with the node + TraverseNode(const Varnode *v,uint4 f) { vn = v; flags = f; } +}; + bool contiguous_test(Varnode *vn1,Varnode *vn2); ///< Test if Varnodes are pieces of a whole Varnode *findContiguousWhole(Funcdata &data,Varnode *vn1, Varnode *vn2); ///< Retrieve the whole Varnode given pieces From 872cd724cb206f7dee8dc34d90ba68e4f2a3f468 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Thu, 18 Mar 2021 15:20:19 -0400 Subject: [PATCH 14/22] Split out BE and LE cspec for MIPS --- .../src/decompile/cpp/funcdata_varnode.cc | 4 +- Ghidra/Processors/MIPS/certification.manifest | 3 +- .../Processors/MIPS/data/languages/mips.ldefs | 14 +-- .../{mips32.cspec => mips32be.cspec} | 0 .../MIPS/data/languages/mips32le.cspec | 90 +++++++++++++++++++ 5 files changed, 101 insertions(+), 10 deletions(-) rename Ghidra/Processors/MIPS/data/languages/{mips32.cspec => mips32be.cspec} (100%) create mode 100644 Ghidra/Processors/MIPS/data/languages/mips32le.cspec diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc index 00258dcb96..4956f44da0 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc @@ -4,9 +4,9 @@ * 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. diff --git a/Ghidra/Processors/MIPS/certification.manifest b/Ghidra/Processors/MIPS/certification.manifest index 182ff5a607..1152c9daf4 100644 --- a/Ghidra/Processors/MIPS/certification.manifest +++ b/Ghidra/Processors/MIPS/certification.manifest @@ -5,14 +5,15 @@ data/languages/mips.dwarf||GHIDRA||||END| data/languages/mips.ldefs||GHIDRA||||END| data/languages/mips.sinc||GHIDRA||||END| data/languages/mips16.sinc||GHIDRA||||END| -data/languages/mips32.cspec||GHIDRA||||END| data/languages/mips32.pspec||GHIDRA||||END| data/languages/mips32Instructions.sinc||GHIDRA||||END| data/languages/mips32R6.pspec||GHIDRA||||END| data/languages/mips32R6be.slaspec||GHIDRA||||END| data/languages/mips32R6le.slaspec||GHIDRA||||END| data/languages/mips32_fp64.cspec||GHIDRA||||END| +data/languages/mips32be.cspec||GHIDRA||||END| data/languages/mips32be.slaspec||GHIDRA||||END| +data/languages/mips32le.cspec||GHIDRA||||END| data/languages/mips32le.slaspec||GHIDRA||||END| data/languages/mips32micro.pspec||GHIDRA||||END| data/languages/mips64.cspec||GHIDRA||||END| diff --git a/Ghidra/Processors/MIPS/data/languages/mips.ldefs b/Ghidra/Processors/MIPS/data/languages/mips.ldefs index abedb93a34..e7281050ad 100644 --- a/Ghidra/Processors/MIPS/data/languages/mips.ldefs +++ b/Ghidra/Processors/MIPS/data/languages/mips.ldefs @@ -10,8 +10,8 @@ manualindexfile="../manuals/mipsM16.idx" id="MIPS:BE:32:default"> MIPS32 32-bit addresses, big endian, with mips16e - - + + @@ -26,8 +26,8 @@ manualindexfile="../manuals/mipsM16.idx" id="MIPS:LE:32:default"> MIPS32 32-bit addresses, little endian, with mips16e - - + + @@ -284,7 +284,7 @@ manualindexfile="../manuals/mipsMic.idx" id="MIPS:BE:32:micro"> MIPS32 32-bit addresses, big endian, with microMIPS - + @@ -299,8 +299,8 @@ manualindexfile="../manuals/mipsMic.idx" id="MIPS:LE:32:micro"> MIPS32 32-bit addresses, little endian, with microMIPS - - + + diff --git a/Ghidra/Processors/MIPS/data/languages/mips32.cspec b/Ghidra/Processors/MIPS/data/languages/mips32be.cspec similarity index 100% rename from Ghidra/Processors/MIPS/data/languages/mips32.cspec rename to Ghidra/Processors/MIPS/data/languages/mips32be.cspec diff --git a/Ghidra/Processors/MIPS/data/languages/mips32le.cspec b/Ghidra/Processors/MIPS/data/languages/mips32le.cspec new file mode 100644 index 0000000000..628b8b4756 --- /dev/null +++ b/Ghidra/Processors/MIPS/data/languages/mips32le.cspec @@ -0,0 +1,90 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From b8024cb747ba770d22454c1696b55abffb273d81 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Tue, 23 Mar 2021 14:56:49 -0400 Subject: [PATCH 15/22] Remove stack placeholders before guarding calls --- .../Decompiler/src/decompile/cpp/heritage.cc | 82 ++++++++++++++----- .../Decompiler/src/decompile/cpp/heritage.hh | 9 +- 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc index 2e2b76494d..7f8bb2b1e4 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc @@ -130,6 +130,45 @@ FlowBlock *PriorityQueue::extract(void) return res; } +/// Initialize heritage state information for a particular address space +/// \param spc is the address space +HeritageInfo::HeritageInfo(AddrSpace *spc) + +{ + if (spc == (AddrSpace *)0) { + space = (AddrSpace *)0; + delay = 0; + deadcodedelay = 0; + hasCallPlaceholders = false; + } + else if (!spc->isHeritaged()) { + space = (AddrSpace *)0; + delay = spc->getDelay(); + deadcodedelay = spc->getDeadcodeDelay(); + hasCallPlaceholders = false; + } + else { + space = spc; + delay = spc->getDelay(); + deadcodedelay = spc->getDeadcodeDelay(); + hasCallPlaceholders = (spc->getType() == IPTR_SPACEBASE); + } + deadremoved = 0; + warningissued = false; + loadGuardSearch = false; +} + +void HeritageInfo::reset(void) + +{ + // Leave any override intact: deadcodedelay = delay; + deadremoved = 0; + if (space != (AddrSpace *)0) + hasCallPlaceholders = (space->getType() == IPTR_SPACEBASE); + warningissued = false; + loadGuardSearch = false; +} + /// Instantiate the heritage manager for a particular function. /// \param data is the function Heritage::Heritage(Funcdata *data) @@ -1180,16 +1219,10 @@ void Heritage::guardCalls(uint4 flags,const Address &addr,int4 size,vectorgetType() == IPTR_SPACEBASE) { - if (fc->getStackPlaceholderSlot() < 0) { // Any stack resolution is complete (or never started) - if (fc->getSpacebaseOffset() != FuncCallSpecs::offset_unknown) - off = spc->wrapOffset(off - fc->getSpacebaseOffset()); - else - tryregister = false; // Do not attempt to register this stack loc as a trial - } - else { // Stack has not been resolved, so we need to abort - fc->abortSpacebaseRelative(*fd); - tryregister = false; - } + if (fc->getSpacebaseOffset() != FuncCallSpecs::offset_unknown) + off = spc->wrapOffset(off - fc->getSpacebaseOffset()); + else + tryregister = false; // Do not attempt to register this stack loc as a trial } Address transAddr(spc,off); // Address relative to callee's stack if (tryregister) { @@ -1695,6 +1728,19 @@ static void verify_dfs(const vector &list,vectornumCalls(); + for(int4 i=0;igetCallSpecs(i)->abortSpacebaseRelative(*fd); + } + info->hasCallPlaceholders = false; // Mark that clear has taken place +} + /// \brief Perform one level of Varnode splitting to match a JoinRecord /// /// Split all the pieces in \b lastcombo, putting them into \b nextlev in order, @@ -2288,16 +2334,9 @@ void Heritage::buildInfoList(void) { if (!infolist.empty()) return; const AddrSpaceManager *manage = fd->getArch(); - infolist.resize(manage->numSpaces()); - for(int4 i=0;inumSpaces();++i) { - AddrSpace *spc = manage->getSpace(i); - if (spc == (AddrSpace *)0) - infolist[i].set((AddrSpace *)0,0,0); - else if (!spc->isHeritaged()) - infolist[i].set((AddrSpace *)0,spc->getDelay(),spc->getDeadcodeDelay()); - else - infolist[i].set(spc,spc->getDelay(),spc->getDeadcodeDelay()); - } + infolist.reserve(manage->numSpaces()); + for(int4 i=0;inumSpaces();++i) + infolist.emplace_back(manage->getSpace(i)); } /// From any address space that is active for this pass, free Varnodes are collected @@ -2328,6 +2367,9 @@ void Heritage::heritage(void) info = &infolist[i]; if (!info->isHeritaged()) continue; if (pass < info->delay) continue; // It is too soon to heritage this space + if (info->hasCallPlaceholders) + clearStackPlaceholders(info); + if (!info->loadGuardSearch) { info->loadGuardSearch = true; if (discoverIndexedStackPointers(info->space,freeStores,true)) { diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh index 4b5f6f23c9..4f29953607 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh @@ -90,11 +90,11 @@ class HeritageInfo { int4 deadremoved; ///< >0 if Varnodes in this space have been eliminated bool loadGuardSearch; ///< \b true if the search for LOAD ops to guard has been performed bool warningissued; ///< \b true if warning issued previously - void set(AddrSpace *spc,int4 dl,int4 dcdl) { - space=spc; delay=dl; deadcodedelay=dcdl; deadremoved=0; warningissued=false; loadGuardSearch = false; } ///< Set all fields + bool hasCallPlaceholders; ///< \b true for the \e stack space, if stack placeholders have not been removed bool isHeritaged(void) const { return (space != (AddrSpace *)0); } ///< Return \b true if heritage is performed on this space - void reset(void) { - deadremoved = 0; deadcodedelay = delay; warningissued = false; loadGuardSearch = false; } ///< Reset + void reset(void); ///< Reset the state +public: + HeritageInfo(AddrSpace *spc); ///< Constructor }; /// \brief Description of a LOAD operation that needs to be guarded @@ -222,6 +222,7 @@ class Heritage { /// \brief Get the heritage status for the given address space const HeritageInfo *getInfo(AddrSpace *spc) const { return &(infolist[spc->getIndex()]); } + void clearStackPlaceholders(HeritageInfo *info); ///< Clear remaining stack placeholder LOADs on any call void splitJoinLevel(vector &lastcombo,vector &nextlev,JoinRecord *joinrec); void splitJoinRead(Varnode *vn,JoinRecord *joinrec); void splitJoinWrite(Varnode *vn,JoinRecord *joinrec); From 71ca7532e7207ecca45c2e6de4f13382e17f66c7 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Tue, 30 Mar 2021 11:57:07 -0400 Subject: [PATCH 16/22] Inform decompiler when opaque data-types are variable length --- .../ghidra/program/model/pcode/PcodeDataTypeManager.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/PcodeDataTypeManager.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/PcodeDataTypeManager.java index fecc0f4cbc..8a5f340318 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/PcodeDataTypeManager.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/PcodeDataTypeManager.java @@ -588,8 +588,10 @@ public class PcodeDataTypeManager { } else { int sz = type.getLength(); + boolean isVarLength = false; if (sz <= 0) { sz = size; + isVarLength = true; } appendNameIdAttributes(resBuf, origType); if (sz < 16) { @@ -601,6 +603,9 @@ public class PcodeDataTypeManager { // Build an "opaque" structure with no fields SpecXmlUtils.encodeStringAttribute(resBuf, "metatype", "struct"); SpecXmlUtils.encodeSignedIntegerAttribute(resBuf, "size", sz); + if (isVarLength) { + SpecXmlUtils.encodeBooleanAttribute(resBuf, "varlength", isVarLength); + } resBuf.append('>'); } } From b7e048df77b6f3ec1f64aa85730990e72b02076d Mon Sep 17 00:00:00 2001 From: Paul Sorensen Date: Tue, 30 Mar 2021 13:39:59 -0400 Subject: [PATCH 17/22] Fix idaxml set_member_cmt func call The script originally was incorrectly calling `idc.set_member_cmt` which takes different arguments to calculate the member struct and offset. And then it passes the results into the `ida_struct` version. However, this is already done, so we can just go straight to the `ida_struct` version. --- GhidraBuild/IDAPro/Python/7xx/python/idaxml.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py b/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py index 5a88be0742..0b7e8e67e6 100644 --- a/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py +++ b/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py @@ -3111,11 +3111,11 @@ class XmlImporter(IdaXml): """ regcmt = member.find(REGULAR_CMT) if regcmt != None: - idc.set_member_cmt(mbr, regcmt.text, False) + ida_struct.set_member_cmt(mbr, regcmt.text, False) self.update_counter(MEMBER + ':' + REGULAR_CMT) rptcmt = member.find(REPEATABLE_CMT) if rptcmt != None: - idc.set_member_cmt(mbr, rptcmt.text, True) + ida_struct.set_member_cmt(mbr, rptcmt.text, True) self.update_counter(MEMBER + ':' + REPEATABLE_CMT) From aee17d9999c3f655e435a3e36c1cb05bc4323cb8 Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Tue, 30 Mar 2021 15:08:41 -0400 Subject: [PATCH 18/22] setReturnBytesConsumed correctly evaluates if change has occurred --- .../Decompiler/certification.manifest | 1 + .../Decompiler/src/decompile/cpp/fspec.cc | 17 ++++++----- .../src/decompile/datatests/multiret.xml | 30 +++++++++++++++++++ 3 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 Ghidra/Features/Decompiler/src/decompile/datatests/multiret.xml diff --git a/Ghidra/Features/Decompiler/certification.manifest b/Ghidra/Features/Decompiler/certification.manifest index 1bf74fa054..58568e61c6 100644 --- a/Ghidra/Features/Decompiler/certification.manifest +++ b/Ghidra/Features/Decompiler/certification.manifest @@ -17,6 +17,7 @@ src/decompile/datatests/forloop_thruspecial.xml||GHIDRA||||END| src/decompile/datatests/forloop_varused.xml||GHIDRA||||END| src/decompile/datatests/forloop_withskip.xml||GHIDRA||||END| src/decompile/datatests/loopcomment.xml||GHIDRA||||END| +src/decompile/datatests/multiret.xml||GHIDRA||||END| src/decompile/datatests/namespace.xml||GHIDRA||||END| src/decompile/datatests/nestedoffset.xml||GHIDRA||||END| src/decompile/datatests/noforloop_alias.xml||GHIDRA||||END| diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc index bb40b24eef..e15ac703a5 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/fspec.cc @@ -3174,17 +3174,20 @@ void FuncProto::setOutputLock(bool val) store->getOutput()->setTypeLock(val); } -/// This value can be used as a hint as to how much of the return value is important and -/// is used to inform the dead code \e consume algorithm. -/// \param val is the estimated number of bytes or 0 -/// \return \b true if the value was changed +/// Provide a hint as to how many bytes of the return value are important. +/// The smallest hint is used to inform the dead-code removal algorithm. +/// \param val is the hint (number of bytes or 0 for all bytes) +/// \return \b true if the smallest hint has changed bool FuncProto::setReturnBytesConsumed(int4 val) { - int4 oldVal = returnBytesConsumed; - if (oldVal == 0 || val < oldVal) + if (val == 0) + return false; + if (returnBytesConsumed == 0 || val < returnBytesConsumed) { returnBytesConsumed = val; - return (oldVal != val); + return true; + } + return false; } /// \brief Assuming \b this prototype is locked, calculate the \e extrapop diff --git a/Ghidra/Features/Decompiler/src/decompile/datatests/multiret.xml b/Ghidra/Features/Decompiler/src/decompile/datatests/multiret.xml new file mode 100644 index 0000000000..b7e67d337b --- /dev/null +++ b/Ghidra/Features/Decompiler/src/decompile/datatests/multiret.xml @@ -0,0 +1,30 @@ + + + + +4883ec1864488b042528000000488944 +240831c083fe01744f83fe02742a897c +2404488d7c2404e8dc0f00008b442404 +488b542408644833142528000000753d +4883c418c3000000488d7c2404b8e903 +00006689442404e8ac0f00008b442404 +ebce000000000000488d7c2404c64424 +0461e8910f00008b442404ebb3e88e0f +0000 + + + +Stack20 = CONCAT31.*0x61 +Stack20 = CONCAT22.*0x3e9 +Stack20 = param_1 + From 0b1cc5ea0e59212b23df318664014fdef1ddcb10 Mon Sep 17 00:00:00 2001 From: Ryan Kurtz Date: Wed, 31 Mar 2021 08:51:43 -0400 Subject: [PATCH 19/22] GP-824: Explicitly permitting illegal reflective access for improved JDK 16 compatibility --- Ghidra/Features/Base/.launch/Ghidra.launch | 4 +++- Ghidra/RuntimeScripts/Common/support/launch.properties | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Ghidra/Features/Base/.launch/Ghidra.launch b/Ghidra/Features/Base/.launch/Ghidra.launch index 643b32cce5..bc3f9b7d7e 100644 --- a/Ghidra/Features/Base/.launch/Ghidra.launch +++ b/Ghidra/Features/Base/.launch/Ghidra.launch @@ -17,6 +17,7 @@ + @@ -26,7 +27,8 @@ + - + diff --git a/Ghidra/RuntimeScripts/Common/support/launch.properties b/Ghidra/RuntimeScripts/Common/support/launch.properties index 5212b28141..d25a029940 100644 --- a/Ghidra/RuntimeScripts/Common/support/launch.properties +++ b/Ghidra/RuntimeScripts/Common/support/launch.properties @@ -77,6 +77,9 @@ VMARGS_MACOS=-Dapple.laf.useScreenMenuBar=false # Prevent log4j from using the Jansi DLL on Windows. VMARGS_WINDOWS=-Dlog4j.skipJansi=true +# Permit "illegal reflective accesses" to enable compatibility with some 3rd party jars +VMARGS=--illegal-access=permit + # Persistent cache directory used by the application. This directory will be used to store # persistent application caches for all users. The default location for Mac/Linux is the same as # specified by java.io.tmpdir property. The default location for Windows corresponds to the From e881a47d92ec243da44c934c824908aafae31b71 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Thu, 1 Apr 2021 12:04:33 -0400 Subject: [PATCH 20/22] GP-587: Updated documentation --- .../java/ghidra/app/plugin/assembler/Assembler.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java index e0bc082847..6e9728ecbc 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/app/plugin/assembler/Assembler.java @@ -38,15 +38,15 @@ public interface Assembler { * *

* This method is only valid if the assembler is bound to a program. An instance may optionally - * implement this method without a program binding. In that case, the returned instruction block - * will refer to pseudo instructions. + * implement this method without a program binding. In that case, the returned iterator will + * refer to pseudo instructions. * *

* NOTE: There must be an active transaction on the bound program for this method to succeed. * * @param at the location where the resulting instructions should be placed * @param listing a new-line separated or array sequence of instructions - * @return the block of resulting instructions + * @return an iterator over the resulting instructions * @throws AssemblySyntaxException a textual instruction is non well-formed * @throws AssemblySemanticException a well-formed instruction cannot be assembled * @throws MemoryAccessException there is an issue writing the result to program memory @@ -196,7 +196,7 @@ public interface Assembler { throws MemoryAccessException; /** - * Place an instruction into the bound program. + * Place instruction bytes into the bound program. * *

* This method is not valid without a program binding. Also, this method must be called during a @@ -204,7 +204,7 @@ public interface Assembler { * * @param insbytes the instruction data * @param at the location of the start of the instruction - * @return the new {@link Instruction} code unit + * @return an iterator over the disassembled instructions * @throws MemoryAccessException there is an issue writing the result to program memory */ public InstructionIterator patchProgram(byte[] insbytes, Address at) From 72cdb5242e0d9a654e1b1404de3ba79372642d14 Mon Sep 17 00:00:00 2001 From: Paul Sorensen Date: Thu, 1 Apr 2021 17:54:27 -0400 Subject: [PATCH 21/22] Fix string splitting exception `string` isn't a type, the author intended to use `str.split()` --- GhidraBuild/IDAPro/Python/7xx/python/idaxml.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py b/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py index 5a88be0742..d73212ac84 100644 --- a/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py +++ b/GhidraBuild/IDAPro/Python/7xx/python/idaxml.py @@ -2395,7 +2395,7 @@ class XmlImporter(IdaXml): # overlayed addresses not currently handled return BADADDR elif ':' in addrstr: - [segstr, offset_str] = string.split(addrstr,':') + [segstr, offset_str] = str.split(addrstr,':') offset = int(offset_str,16) if self.is_int(segstr) == True: sgmt = int(segstr,16) @@ -3239,7 +3239,7 @@ class XmlImporter(IdaXml): idc.warning(msg) return elif ':' in addrstr: - [seg_str, offset_str] = string.split(addrstr,':') + [seg_str, offset_str] = str.split(addrstr,':') offset = int(offset_str, 16) if self.is_int(seg_str): base = int(seg_str, 16) From 359925c9b3e7602e71fb569e44c2c121e9b98c94 Mon Sep 17 00:00:00 2001 From: Ryan Kurtz Date: Fri, 2 Apr 2021 13:08:05 -0400 Subject: [PATCH 22/22] GP-832: Certify --- Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh index 23008d2685..1e3520b6f5 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/docmain.hh @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.