From 2d8cf82af2228b54fd41cd31824664c40a315fd7 Mon Sep 17 00:00:00 2001 From: Joel Anderson Date: Sat, 2 Oct 2021 22:37:04 -0400 Subject: [PATCH] align comments with code in decompiler Add a configuration option for the decompiler to align comments with code instead of using the fixed indentation setting. --- .../Decompiler/src/decompile/cpp/options.cc | 12 +++ .../Decompiler/src/decompile/cpp/options.hh | 7 ++ .../src/decompile/cpp/printlanguage.cc | 19 ++++- .../src/decompile/cpp/printlanguage.hh | 2 + .../DecompilePlugin/DecompilerOptions.html | 9 +++ .../app/decompiler/DecompileOptions.java | 31 ++++++++ .../plugin/core/decompile/DecompilerTest.java | 77 ++++++++++++++++++- .../ghidra/program/model/pcode/ElementId.java | 3 + 8 files changed, 156 insertions(+), 4 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/options.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/options.cc index 0ee439e4473..562c544c232 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/options.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/options.cc @@ -55,6 +55,7 @@ ElementId ELEM_SETLANGUAGE = ElementId("setlanguage",207); ElementId ELEM_STRUCTALIGN = ElementId("structalign",208); ElementId ELEM_TOGGLERULE = ElementId("togglerule",209); ElementId ELEM_WARNING = ElementId("warning",210); +ElementId ELEM_COMMENTINDENTALIGN = ElementId("commentindentalign",271); /// If the parameter is "on" return \b true, if "off" return \b false. /// Any other value causes an exception. @@ -109,6 +110,7 @@ OptionDatabase::OptionDatabase(Architecture *g) registerOption(new OptionMaxLineWidth()); registerOption(new OptionIndentIncrement()); registerOption(new OptionCommentIndent()); + registerOption(new OptionCommentIndentAlign()); registerOption(new OptionCommentStyle()); registerOption(new OptionCommentHeader()); registerOption(new OptionCommentInstruction()); @@ -524,6 +526,16 @@ string OptionCommentIndent::apply(Architecture *glb,const string &p1,const strin return "Comment indent set to "+p1; } +/// \class OptionCommentIndentAlign +/// \brief Toggle whether to align the comment with the current code rather or use a fixed indentation. +string OptionCommentIndentAlign::apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const + +{ + bool val = onOrOff(p1); + glb->print->setLineCommentIndentAlign(val); + return "Comment indent alignment turned "+p1; +} + /// \class OptionCommentStyle /// \brief Set the style of comment emitted by the decompiler /// diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/options.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/options.hh index 474ee70715b..29ac2dac613 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/options.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/options.hh @@ -61,6 +61,7 @@ extern ElementId ELEM_SETLANGUAGE; ///< Marshaling element \ extern ElementId ELEM_STRUCTALIGN; ///< Marshaling element \ extern ElementId ELEM_TOGGLERULE; ///< Marshaling element \ extern ElementId ELEM_WARNING; ///< Marshaling element \ +extern ElementId ELEM_COMMENTINDENTALIGN; ///< Marshaling element \ /// \brief Base class for options classes that affect the configuration of the Architecture object /// @@ -212,6 +213,12 @@ public: virtual string apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const; }; +class OptionCommentIndentAlign : public ArchOption { +public: + OptionCommentIndentAlign(void) { name = "commentindentalign"; } ///< Constructor + virtual string apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const; +}; + class OptionCommentStyle : public ArchOption { public: OptionCommentStyle(void) { name = "commentstyle"; } ///< Constructor diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.cc index a693042ded2..7dfb4400c37 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.cc @@ -87,6 +87,13 @@ void PrintLanguage::setLineCommentIndent(int4 val) line_commentindent = val; } +/// \param val is whether to align comments with code or use a fixed indentation +void PrintLanguage::setLineCommentIndentAlign(bool val) + +{ + line_commentindentalign = val; +} + /// By default, comments are indicated in the high-level language by preceding /// them with a specific sequence of delimiter characters, and optionally /// by ending the comment with another set of delimiter characters. @@ -573,6 +580,7 @@ void PrintLanguage::resetDefaultsInternal(void) mods = 0; head_comment_type = Comment::header | Comment::warningheader; line_commentindent = 20; + line_commentindentalign = false; namespc_strategy = MINIMAL_NAMESPACES; instr_comment_type = Comment::user2 | Comment::warning; } @@ -587,9 +595,14 @@ void PrintLanguage::emitLineComment(int4 indent,const Comment *comm) const string &text( comm->getText() ); const AddrSpace *spc = comm->getAddr().getSpace(); uintb off = comm->getAddr().getOffset(); - if (indent <0) - indent = line_commentindent; // User specified default indent - emit->tagLine(indent); + if (line_commentindentalign) { + emit->tagLine(); + } + else { + if (indent <0) + indent = line_commentindent; // User specified default indent + emit->tagLine(indent); + } int4 id = emit->startComment(); // The comment delimeters should not be printed as // comment tags, so that they won't get filled diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.hh index 497e968c705..95261c9f5a0 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.hh @@ -248,6 +248,7 @@ private: vector nodepend; ///< Data-flow nodes waiting to be pushed onto the RPN stack int4 pending; ///< Number of data-flow nodes waiting to be pushed int4 line_commentindent; ///< Number of characters a comment line should be indented + bool line_commentindentalign; ///< Whether to align comment lines with code or use a fixed indentation string commentstart; ///< Delimiter characters for the start of a comment string commentend; ///< Delimiter characters (if any) for the end of a comment protected: @@ -428,6 +429,7 @@ public: void setMaxLineSize(int4 mls) { emit->setMaxLineSize(mls); } ///< Set the maximum number of characters per line void setIndentIncrement(int4 inc) { emit->setIndentIncrement(inc); } ///< Set the number of characters to indent per level of code nesting void setLineCommentIndent(int4 val); ///< Set the number of characters to indent comment lines + void setLineCommentIndentAlign(bool val); ///< Set whether to align comment lines with code lines or not void setCommentDelimeter(const string &start,const string &stop, bool usecommentfill); ///< Establish comment delimiters for the language uint4 getInstructionComment(void) const { return instr_comment_type; } ///< Get the type of comments suitable within the body of a function diff --git a/Ghidra/Features/Decompiler/src/main/help/help/topics/DecompilePlugin/DecompilerOptions.html b/Ghidra/Features/Decompiler/src/main/help/help/topics/DecompilePlugin/DecompilerOptions.html index 4a93dfe117c..ceb3c3fffdd 100644 --- a/Ghidra/Features/Decompiler/src/main/help/help/topics/DecompilePlugin/DecompilerOptions.html +++ b/Ghidra/Features/Decompiler/src/main/help/help/topics/DecompilePlugin/DecompilerOptions.html @@ -399,6 +399,15 @@

+Align comments with code +
+
+

+ Aligns comment lines with the current indentation level of the decompiler output, instead of using a + fixed amount of spaces. When checked, the comment line indent level option is ignored. +

+
+
Comment style
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 de840a6629c..2db700a58a8 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 @@ -179,6 +179,13 @@ public String toString() { private final static int COMMENTINDENT_OPTIONDEFAULT = 20; // Must match PrintLanguage::resetDefaultsInternal private int commentindent; + private final static String COMMENTINDENTALIGN_OPTIONSTRING = "Display.Comment lines aligned with code"; + private final static String COMMENTINDENTALIGN_OPTIONDESCRIPTION = + "Align each comment with the indentation of the code immediately " + + "following it, instead of using the comment line indent level"; + private final static boolean COMMENTINDENTALIGN_OPTIONDEFAULT = false; // Must match PrintLanguage::resetDefaultsInternal + private boolean commentindentAlign; + private final static String COMMENTSTYLE_OPTIONSTRING = "Display.Comment style"; private final static String COMMENTSTYLE_OPTIONDESCRIPTION = "Choice between either the C style comments /* */ or C++ style // "; @@ -391,6 +398,7 @@ public DecompileOptions() { maxwidth = MAXWIDTH_OPTIONDEFAULT; indentwidth = INDENTWIDTH_OPTIONDEFAULT; commentindent = COMMENTINDENT_OPTIONDEFAULT; + commentindentAlign = COMMENTINDENTALIGN_OPTIONDEFAULT; commentStyle = COMMENTSTYLE_OPTIONDEFAULT; commentPREInclude = COMMENTPRE_OPTIONDEFAULT; commentPLATEInclude = COMMENTPLATE_OPTIONDEFAULT; @@ -443,6 +451,7 @@ public void grabFromToolAndProgram(Plugin ownerPlugin, ToolOptions opt, Program maxwidth = opt.getInt(MAXWIDTH_OPTIONSTRING, MAXWIDTH_OPTIONDEFAULT); indentwidth = opt.getInt(INDENTWIDTH_OPTIONSTRING, INDENTWIDTH_OPTIONDEFAULT); commentindent = opt.getInt(COMMENTINDENT_OPTIONSTRING, COMMENTINDENT_OPTIONDEFAULT); + commentindentAlign = opt.getBoolean(COMMENTINDENTALIGN_OPTIONSTRING, COMMENTINDENTALIGN_OPTIONDEFAULT); commentStyle = opt.getEnum(COMMENTSTYLE_OPTIONSTRING, COMMENTSTYLE_OPTIONDEFAULT); commentEOLInclude = opt.getBoolean(COMMENTEOL_OPTIONSTRING, COMMENTEOL_OPTIONDEFAULT); commentPREInclude = opt.getBoolean(COMMENTPRE_OPTIONSTRING, COMMENTPRE_OPTIONDEFAULT); @@ -559,6 +568,9 @@ public void registerOptions(Plugin ownerPlugin, ToolOptions opt, Program program opt.registerOption(COMMENTINDENT_OPTIONSTRING, COMMENTINDENT_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayCommentIndent"), COMMENTINDENT_OPTIONDESCRIPTION); + opt.registerOption(COMMENTINDENTALIGN_OPTIONSTRING, COMMENTINDENTALIGN_OPTIONDEFAULT, + new HelpLocation(HelpTopics.DECOMPILER, "DisplayCommentIndentAlign"), + COMMENTINDENTALIGN_OPTIONDESCRIPTION); opt.registerOption(COMMENTSTYLE_OPTIONSTRING, COMMENTSTYLE_OPTIONDEFAULT, new HelpLocation(HelpTopics.DECOMPILER, "DisplayCommentStyle"), COMMENTSTYLE_OPTIONDESCRIPTION); @@ -723,6 +735,9 @@ public void encode(Encoder encoder, DecompInterface iface) throws IOException { if (commentindent != COMMENTINDENT_OPTIONDEFAULT) { appendOption(encoder, ELEM_COMMENTINDENT, Integer.toString(commentindent), "", ""); } + if (commentindentAlign != COMMENTINDENTALIGN_OPTIONDEFAULT) { + appendOption(encoder, ELEM_COMMENTINDENTALIGN, commentindentAlign ? "on" : "off", "", ""); + } if (commentStyle != COMMENTSTYLE_OPTIONDEFAULT) { String curstyle = CommentStyleEnum.CPPStyle.equals(commentStyle) ? "cplusplus" : "c"; appendOption(encoder, ELEM_COMMENTSTYLE, curstyle, "", ""); @@ -1006,6 +1021,22 @@ public void setMaxInstructions(int num) { maxIntructionsPer = num; } + public int getCommentIndent() { + return commentindent; + } + + public void setCommentIndent(int indent) { + commentindent = indent; + } + + public boolean isCommentIndentAlign() { + return commentindentAlign; + } + + public void setCommentIndentAlign(boolean align) { + commentindentAlign = align; + } + public CommentStyleEnum getCommentStyle() { return commentStyle; } diff --git a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerTest.java b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerTest.java index 1044a1a4e82..cb1668d84f3 100644 --- a/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerTest.java +++ b/Ghidra/Features/Decompiler/src/test.slow/java/ghidra/app/plugin/core/decompile/DecompilerTest.java @@ -15,10 +15,14 @@ */ package ghidra.app.plugin.core.decompile; +import java.util.Optional; + import org.junit.*; import ghidra.app.decompiler.*; import ghidra.program.model.address.Address; +import ghidra.program.model.address.AddressSpace; +import ghidra.program.model.listing.CodeUnit; import ghidra.program.model.listing.Function; import ghidra.program.model.listing.Program; import ghidra.test.AbstractGhidraHeadedIntegrationTest; @@ -28,13 +32,14 @@ public class DecompilerTest extends AbstractGhidraHeadedIntegrationTest { private Program prog; private DecompInterface decompiler; + private long returnBytesOffset = 0x0; @Before public void setUp() throws Exception { ToyProgramBuilder builder = new ToyProgramBuilder("notepad_decompiler", true); builder.createMemory("test", "0x0", 2); - builder.addBytesReturn(0x0); + builder.addBytesReturn(returnBytesOffset); builder.createFunction("0x0"); prog = builder.getProgram(); @@ -58,4 +63,74 @@ public void testDecompileInterfaceReturnsAFunction() throws Exception { String decompilation = decompResults.getDecompiledFunction().getC(); Assert.assertNotNull(decompilation); } + + @Test + public void testAlignedCommentIndentation() throws Exception { + int indent = 20; + DecompileOptions options = new DecompileOptions(); + options.setCommentIndent(indent); + options.setCommentIndentAlign(true); + options.setPRECommentIncluded(true); + decompiler.setOptions(options); + + AddressSpace space = prog.getAddressFactory().getDefaultAddressSpace(); + + // add a comment to the program listing + Address returnBytesAddr = space.getAddress(returnBytesOffset); + int transaction = prog.startTransaction("add comment for indentation test"); + String comment = "aligned-comment-indentation-test"; + prog.getListing().getCodeUnitAt(returnBytesAddr).setComment(CodeUnit.PRE_COMMENT, comment); + prog.endTransaction(transaction, true); + + Address addr = space.getAddress(0x0); + Function func = prog.getListing().getFunctionAt(addr); + DecompileResults decompResults = decompiler.decompileFunction(func, + DecompileOptions.SUGGESTED_DECOMPILE_TIMEOUT_SECS, TaskMonitor.DUMMY); + String decompilation = decompResults.getDecompiledFunction().getC(); + Assert.assertNotNull(decompilation); + + Optional commentLineCheck = decompilation.lines().filter(line -> line.contains(comment)).findFirst(); + Optional returnLineCheck = decompilation.lines().filter(line -> line.endsWith("return;")).findFirst(); + Assert.assertTrue(commentLineCheck.isPresent()); + Assert.assertTrue(returnLineCheck.isPresent()); + + String commentLine = commentLineCheck.get(); + String returnLine = returnLineCheck.get(); + + Assert.assertFalse(commentLine.startsWith(" ".repeat(indent))); + + int commentIndentation = commentLine.indexOf(commentLine.stripLeading()); + int returnIndentation = returnLine.indexOf(returnLine.stripLeading()); + Assert.assertEquals(commentIndentation, returnIndentation); + } + + @Test + public void testFixedCommentIndentation() throws Exception { + int indent = 20; + DecompileOptions options = new DecompileOptions(); + options.setCommentIndent(indent); + options.setCommentIndentAlign(false); + options.setPRECommentIncluded(true); + decompiler.setOptions(options); + + AddressSpace space = prog.getAddressFactory().getDefaultAddressSpace(); + + // add a comment to the program listing + Address returnBytesAddr = space.getAddress(returnBytesOffset); + int transaction = prog.startTransaction("add comment for indentation test"); + String comment = "fixed-comment-indentation-test"; + prog.getListing().getCodeUnitAt(returnBytesAddr).setComment(CodeUnit.PRE_COMMENT, comment); + prog.endTransaction(transaction, true); + + Address addr = space.getAddress(0x0); + Function func = prog.getListing().getFunctionAt(addr); + DecompileResults decompResults = decompiler.decompileFunction(func, + DecompileOptions.SUGGESTED_DECOMPILE_TIMEOUT_SECS, TaskMonitor.DUMMY); + String decompilation = decompResults.getDecompiledFunction().getC(); + Assert.assertNotNull(decompilation); + + Optional commentLine = decompilation.lines().filter(line -> line.contains(comment)).findFirst(); + Assert.assertTrue(commentLine.isPresent()); + Assert.assertTrue(commentLine.get().startsWith(" ".repeat(indent))); + } } diff --git a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/ElementId.java b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/ElementId.java index 82a38f6b260..4fd17c02038 100644 --- a/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/ElementId.java +++ b/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/ElementId.java @@ -421,5 +421,8 @@ public record ElementId(String name, int id) { public static final ElementId ELEM_COMMAND_GETUSEROPNAME = new ElementId("command_getuseropname", COMMAND_GETUSEROPNAME); + // option to allow comments to align with code + public static final ElementId ELEM_COMMENTINDENTALIGN = new ElementId("commentindentalign", 271); + public static final ElementId ELEM_UNKNOWN = new ElementId("XMLunknown", 270); }