Merge remote-tracking branch 'origin/GP-621_CommentGroups'

Ghidra/Features/Decompiler/src/decompile/cpp/comment.cc

@@ -21,10 +21,8 @@
 /// \param ad is the Address of the instruction associated with the comment
 /// \param uq is used internally to sub-sort comments at the same address
 /// \param txt is the body of the comment
-Comment::Comment(uint4 tp,const Address &fad,
+Comment::Comment(uint4 tp,const Address &fad,const Address &ad,int4 uq,const string &txt) :
-			     const Address &ad,int4 uq,
+  type(tp), uniq(uq), funcaddr(fad), addr(ad), text(txt), emitted(false)
-			     const string &txt) :
-  type(tp), funcaddr(fad), addr(ad), uniq(uq), text(txt)
 {
 }
 
@@ -55,6 +53,7 @@ void Comment::saveXml(ostream &s) const
 void Comment::restoreXml(const Element *el,const AddrSpaceManager *manage)
 
 {
+  emitted = false;
   type = 0;
   type = Comment::encodeCommentType(el->getAttributeValue("type"));
   const List &list(el->getChildren());
@@ -344,6 +343,7 @@ void CommentSorter::setupFunctionList(uint4 tp,const Funcdata *fd,const CommentD
   while(iter != lastiter) {
     Comment *comm = *iter;
     if (findPosition(subsort, comm, fd)) {
+      comm->setEmitted(false);
       commmap[ subsort ] = comm;
       subsort.pos += 1;		// Advance the uniqueness counter
     }

Ghidra/Features/Decompiler/src/decompile/cpp/comment.hh

@@ -37,10 +37,11 @@ class Funcdata;
 class Comment {
   friend class CommentDatabaseInternal;
   uint4 type;			///< The properties associated with the comment
+  int4 uniq;			///< Sub-identifier for uniqueness
   Address funcaddr;		///< Address of the function containing the comment
   Address addr;			///< Address associated with the comment
-  int4 uniq;			///< Sub-identifier for uniqueness
   string text;			///< The body of the comment
+  mutable bool emitted;		///< \b true if this comment has already been emitted
 public:
   /// \brief Possible properties associated with a comment
   enum comment_type {
@@ -51,9 +52,10 @@ public:
     warning = 16,		///< The comment is auto-generated to alert the user
     warningheader = 32		///< The comment is auto-generated and should be in the header
   };
-  Comment(uint4 tp,const Address &fad,
+  Comment(uint4 tp,const Address &fad,const Address &ad,int4 uq,const string &txt);	///< Constructor
-	      const Address &ad,int4 uq,const string &txt);	///< Constructor
   Comment(void) {} 	///< Constructor for use with restoreXml
+  void setEmitted(bool val) const { emitted = val; }
+  bool isEmitted(void) const { return emitted; }
   uint4 getType(void) const { return type; }			///< Get the properties associated with the comment
   const Address &getFuncAddr(void) const { return funcaddr; }	///< Get the address of the function containing the comment
   const Address &getAddr(void) const { return addr; }		///< Get the address to which the instruction is attached

Ghidra/Features/Decompiler/src/decompile/cpp/printc.cc

@@ -2586,19 +2586,21 @@ void PrintC::emitBlockIf(const BlockIf *bl)
 
   pushMod();
   setMod(no_branch);
-  bl->getBlock(0)->emit(this);
+  FlowBlock *condBlock = bl->getBlock(0);
+  condBlock->emit(this);
   popMod();
+  emitCommentBlockTree(condBlock);
   emit->tagLine();
-  op = bl->getBlock(0)->lastOp();
+  op = condBlock->lastOp();
   emit->tagOp("if",EmitXml::keyword_color,op);
   emit->spaces(1);
   pushMod();
   setMod(only_branch);
-  bl->getBlock(0)->emit(this);
+  condBlock->emit(this);
   popMod();
   if (bl->getGotoTarget() != (FlowBlock *)0) {
     emit->spaces(1);
-    emitGotoStatement(bl->getBlock(0),bl->getGotoTarget(),bl->getGotoType());
+    emitGotoStatement(condBlock,bl->getGotoTarget(),bl->getGotoType());
     popMod();
     return;
   }
@@ -2644,8 +2646,10 @@ void PrintC::emitForLoop(const BlockWhileDo *bl)
   pushMod();
   unsetMod(no_branch|only_branch);
   emitAnyLabelStatement(bl);
+  FlowBlock *condBlock = bl->getBlock(0);
+  emitCommentBlockTree(condBlock);
   emit->tagLine();
-  op = bl->getBlock(0)->lastOp();
+  op = condBlock->lastOp();
   emit->tagOp("for",EmitXml::keyword_color,op);
   emit->spaces(1);
   int4 id1 = emit->openParen('(');
@@ -2659,7 +2663,7 @@ void PrintC::emitForLoop(const BlockWhileDo *bl)
   }
   emit->print(";");
   emit->spaces(1);
-  bl->getBlock(0)->emit(this);		// Emit the conditional statement
+  condBlock->emit(this);		// Emit the conditional statement
   emit->print(";");
   emit->spaces(1);
   op = bl->getIterateOp();		// Emit the iterator statement
@@ -2695,13 +2699,14 @@ void PrintC::emitBlockWhileDo(const BlockWhileDo *bl)
   pushMod();
   unsetMod(no_branch|only_branch);
   emitAnyLabelStatement(bl);
-  emit->tagLine();
+  FlowBlock *condBlock = bl->getBlock(0);
-  op = bl->getBlock(0)->lastOp();
+  op = condBlock->lastOp();
   if (bl->hasOverflowSyntax()) {
     // Print conditional block as
     //     while( true ) {
     //       conditionbody ...
     //       if (conditionalbranch) break;
+    emit->tagLine();
     emit->tagOp("while",EmitXml::keyword_color,op);
     int4 id1 = emit->openParen('(');
     emit->spaces(1);
@@ -2713,27 +2718,30 @@ void PrintC::emitBlockWhileDo(const BlockWhileDo *bl)
     emit->print("{");
     pushMod();
     setMod(no_branch);
-    bl->getBlock(0)->emit(this);
+    condBlock->emit(this);
     popMod();
+    emitCommentBlockTree(condBlock);
     emit->tagLine();
     emit->tagOp("if",EmitXml::keyword_color,op);
     emit->spaces(1);
     pushMod();
     setMod(only_branch);
-    bl->getBlock(0)->emit(this);
+    condBlock->emit(this);
     popMod();
     emit->spaces(1);
-    emitGotoStatement(bl->getBlock(0),(const FlowBlock *)0,FlowBlock::f_break_goto);
+    emitGotoStatement(condBlock,(const FlowBlock *)0,FlowBlock::f_break_goto);
   }
   else {
     // Print conditional block "normally" as
     //     while(condition) {
+    emitCommentBlockTree(condBlock);
+    emit->tagLine();
     emit->tagOp("while",EmitXml::keyword_color,op);
     emit->spaces(1);
     int4 id1 = emit->openParen('(');
     pushMod();
     setMod(comma_separate);
-    bl->getBlock(0)->emit(this);
+    condBlock->emit(this);
     popMod();
     emit->closeParen(')',id1);
     emit->spaces(1);
@@ -2924,11 +2932,38 @@ void PrintC::emitCommentGroup(const PcodeOp *inst)
   commsorter.setupOpList(inst);
   while(commsorter.hasNext()) {
     Comment *comm = commsorter.getNext();
+    if (comm->isEmitted()) continue;
     if ((instr_comment_type & comm->getType())==0) continue;
     emitLineComment(-1,comm);
   }
 }
 
+/// With the control-flow hierarchy, print any comments associated with basic blocks in
+/// the specified subtree.  Used where statements from multiple basic blocks are printed on
+/// one line and a normal comment would get printed in the middle of this line.
+/// \param bl is the root of the control-flow subtree
+void PrintC::emitCommentBlockTree(const FlowBlock *bl)
+
+{
+  if (bl == (const FlowBlock *)0) return;
+  FlowBlock::block_type btype = bl->getType();
+  if (btype == FlowBlock::t_copy) {
+    bl = bl->subBlock(0);
+    btype = bl->getType();
+  }
+  if (btype == FlowBlock::t_plain) return;
+  if (bl->getType() != FlowBlock::t_basic) {
+    const BlockGraph *rootbl = (const BlockGraph *)bl;
+    int4 size = rootbl->getSize();
+    for(int4 i=0;i<size;++i) {
+      emitCommentBlockTree(rootbl->subBlock(i));
+    }
+    return;
+  }
+  commsorter.setupBlockList(bl);
+  emitCommentGroup((const PcodeOp *)0);	// Emit any comments for the block
+}
+
 /// Collect all comment lines marked as \e header for the function and
 /// emit them with the appropriate delimiters.
 /// \param fd is the given function
@@ -2939,6 +2974,7 @@ void PrintC::emitCommentFuncHeader(const Funcdata *fd)
   commsorter.setupHeader(CommentSorter::header_basic);
   while(commsorter.hasNext()) {
     Comment *comm = commsorter.getNext();
+    if (comm->isEmitted()) continue;
     if ((head_comment_type & comm->getType())==0) continue;
     emitLineComment(0,comm);
     extralinebreak = true;
@@ -2950,6 +2986,7 @@ void PrintC::emitCommentFuncHeader(const Funcdata *fd)
     commsorter.setupHeader(CommentSorter::header_unplaced);
     while(commsorter.hasNext()) {
       Comment *comm = commsorter.getNext();
+      if (comm->isEmitted()) continue;
       if (!extralinebreak) {
 	Comment label(Comment::warningheader,fd->getAddress(),fd->getAddress(),0,
 		      "Comments that could not be placed in the function body:");

Ghidra/Features/Decompiler/src/decompile/cpp/printc.hh

@@ -156,6 +156,7 @@ protected:
   void emitLabelStatement(const FlowBlock *bl);		///< Emit any required label statement for a given basic block
   void emitAnyLabelStatement(const FlowBlock *bl);	///< Emit any required label statement for a given control-flow block
   void emitCommentGroup(const PcodeOp *inst);		///< Emit comments associated with a given statement
+  void emitCommentBlockTree(const FlowBlock *bl);	///< Emit any comments under the given control-flow subtree
   void emitCommentFuncHeader(const Funcdata *fd);	///< Emit comments in the given function's header
   void emitForLoop(const BlockWhileDo *bl);		///< Emit block as a \e for loop
   void opFunc(const PcodeOp *op);			///< Push a \e functional expression based on the given p-code op to the RPN stack

Ghidra/Features/Decompiler/src/decompile/cpp/printlanguage.cc

@@ -631,6 +631,7 @@ void PrintLanguage::emitLineComment(int4 indent,const Comment *comm)
     emit->tagComment(commentend.c_str(),EmitXml::comment_color,
 		      spc,off);
   emit->stopComment(id);
+  comm->setEmitted(true);
 }
 
 /// Tell the emitter whether to emit just the raw tokens or if

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/ClangLayoutController.java

@@ -18,7 +18,6 @@ package ghidra.app.decompiler.component;
 import java.awt.*;
 import java.math.BigInteger;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.regex.*;
 
@@ -183,10 +182,6 @@ public class ClangLayoutController implements LayoutModel, LayoutModelListener {
 		ClangFieldElement lineNumberFieldElement =
 			createLineNumberFieldElement(line, lineCount, paintLineNumbers);
 
-		if (isComment(tokens)) {
-			return createCommentField(tokens, lineNumberFieldElement, line.getIndent());
-		}
-
 		FieldElement[] elements = createFieldElementsForLine(tokens);
 
 		int indent = line.getIndent() * indentWidth;
@@ -196,66 +191,29 @@ public class ClangLayoutController implements LayoutModel, LayoutModelListener {
 			hlFactory);
 	}
 
-	private ClangTextField createCommentField(List<ClangToken> tokens,
-			ClangFieldElement lineNumberFieldElement, int indentCount) {
-
-		StringBuilder buffy = new StringBuilder();
-		for (ClangToken t : tokens) {
-			buffy.append(t.getText());
-		}
-
-		String text = buffy.toString();
-		ClangCommentToken token = getFirstCommentToken(tokens);
-		Color color = syntax_color[token.getSyntaxType()];
-		AttributedString prototype = new AttributedString("prototype", color, metrics);
-		Program program = decompilerPanel.getProgram();
-		FieldElement element = CommentUtils.parseTextForAnnotations(text, program, prototype, 0);
-
-		FieldElement[] elements = new FieldElement[] { element };
-		ClangCommentToken newCommentToken = ClangCommentToken.derive(token, text);
-		List<ClangToken> newTokens = Arrays.asList(newCommentToken);
-
-		int indent = indentCount * indentWidth;
-		int lineNumberWidth = lineNumberFieldElement.getStringWidth();
-		int updatedMaxWidth = maxWidth + lineNumberWidth;
-		return new ClangTextField(newTokens, elements, lineNumberFieldElement, indent,
-			updatedMaxWidth, hlFactory);
-	}
-
 	private FieldElement[] createFieldElementsForLine(List<ClangToken> tokens) {
 
-		ClangFieldElement[] elements = new ClangFieldElement[tokens.size()];
+		FieldElement[] elements = new FieldElement[tokens.size()];
 		int columnPosition = 0;
 		for (int i = 0; i < tokens.size(); ++i) {
 			ClangToken token = tokens.get(i);
-			AttributedString as =
+			Color color = syntax_color[token.getSyntaxType()];
-				new AttributedString(token.getText(), syntax_color[token.getSyntaxType()], metrics);
+			if (token instanceof ClangCommentToken) {
-			elements[i] = new ClangFieldElement(token, as, columnPosition);
+				AttributedString prototype = new AttributedString("prototype", color, metrics);
-			columnPosition += as.length();
+				Program program = decompilerPanel.getProgram();
+				elements[i] =
+					CommentUtils.parseTextForAnnotations(token.getText(), program, prototype, 0);
+				columnPosition += elements[i].length();
+			}
+			else {
+				AttributedString as = new AttributedString(token.getText(), color, metrics);
+				elements[i] = new ClangFieldElement(token, as, columnPosition);
+				columnPosition += as.length();
+			}
 		}
 		return elements;
 	}
 
-	private ClangCommentToken getFirstCommentToken(List<ClangToken> tokens) {
-		for (ClangToken t : tokens) {
-			if (t instanceof ClangCommentToken) {
-				return (ClangCommentToken) t;
-			}
-		}
-		return null;
-	}
-
-	private boolean isComment(List<ClangToken> tokens) {
-		for (ClangToken t : tokens) {
-			if (t instanceof ClangCommentToken) {
-				// for now, I believe all comments are on a line by themselves, so if we find
-				// a comment token, then these are all comments
-				return true;
-			}
-		}
-		return false;
-	}
-
 	private ClangFieldElement createLineNumberFieldElement(ClangLine line, int lineCount,
 			boolean paintLineNumbers) {
 

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/component/DecompilerUtils.java

@@ -435,6 +435,9 @@ public class DecompilerUtils {
 		FieldSelection fieldSelection = new FieldSelection();
 		for (ClangToken clangToken : tokens) {
 			ClangLine lineParent = clangToken.getLineParent();
+			if (lineParent == null) {
+				continue;
+			}
 			int lineNumber = lineParent.getLineNumber();
 			// lineNumber is one-based, we need zero-based
 			fieldSelection.addRange(lineNumber - 1, lineNumber);
@@ -629,6 +632,62 @@ public class DecompilerUtils {
 		return text.startsWith("goto");
 	}
 
+	/**
+	 * Within a token stream, after seeing an initial comment token, collect the contiguous
+	 * sequence of tokens that are part of the comment and group them into a single
+	 * ClangCommentToken.  This makes post processing on the full comment string easier.
+	 * A single comment string can contain white space that manifests as ClangSyntaxTokens
+	 * with white space as text. 
+	 * @param alltoks is the token stream
+	 * @param i is the position of the initial comment token
+	 * @param first is the initial comment token
+	 * @param current is the ClangLine object currently being scanned
+	 * @param builder is used to collect the full comment string
+	 * @return the position of the first token after the comment string
+	 */
+	private static int consumeCommentTokens(List<ClangNode> alltoks, int i, ClangCommentToken first,
+			ClangLine current, StringBuilder builder) {
+		builder.setLength(0);
+		builder.append(first.getText());
+		i += 1;
+		while (i < alltoks.size()) {
+			ClangToken tok = (ClangToken) alltoks.get(i);
+			if (tok instanceof ClangCommentToken) {
+				if (first.getSyntaxType() != tok.getSyntaxType()) {
+					break;
+				}
+				builder.append(tok.getText());
+			}
+			else if (tok instanceof ClangSyntaxToken) {
+				// Comments can have blank space tokens embedded in them
+				String val = tok.getText();
+				if (val.isBlank()) {
+					builder.append(val);
+				}
+				else {
+					break;
+				}
+			}
+			else {
+				break;
+			}
+			i += 1;
+		}
+		ClangCommentToken commentToken = ClangCommentToken.derive(first, builder.toString());
+		commentToken.setLineParent(current);
+		current.addToken(commentToken);
+
+		return i;
+	}
+
+	/**
+	 * A token hierarchy is flattened and then split into individual lines at the
+	 * ClangBreak tokens.  An array of the lines, each as a ClangLine object that owns
+	 * its respective tokens, is returned.  Sequences of comment tokens are collapsed into
+	 * a single ClangCommentToken.
+	 * @param group is the token hierarchy
+	 * @return the array of ClangLine objects
+	 */
 	public static ArrayList<ClangLine> toLines(ClangTokenGroup group) {
 
 		List<ClangNode> alltoks = new ArrayList<>();
@@ -650,13 +709,21 @@ public class DecompilerUtils {
 		else {
 			current = new ClangLine(lineNumber++, 0); // otherwise use zero indent
 		}
+
+		StringBuilder commentBuilder = new StringBuilder();
 		for (; i < alltoks.size(); ++i) {
+
 			ClangToken tok = (ClangToken) alltoks.get(i);
 			if (tok instanceof ClangBreak) {
 				lines.add(current);
 				brk = (ClangBreak) tok;
 				current = new ClangLine(lineNumber++, brk.getIndent());
 			}
+			else if (tok instanceof ClangCommentToken) {
+				i = consumeCommentTokens(alltoks, i, (ClangCommentToken) tok, current,
+					commentBuilder);
+				i -= 1;
+			}
 			else {
 				tok.setLineParent(current);
 				current.addToken(tok);