Refactor buildXrefs, parallel versions of ConsistencyCheck

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

@@ -28,13 +28,12 @@ SleighBase::SleighBase(void)
 
 /// Assuming the symbol table is populated, iterate through the table collecting
 /// registers (for the map), user-op names, and context fields.
-void SleighBase::buildXrefs(void)
+void SleighBase::buildXrefs(vector<string> &errorPairs)
 
 {
   SymbolScope *glb = symtab.getGlobalScope();
   SymbolTree::const_iterator iter;
   SleighSymbol *sym;
-  int4 errors = 0;
   ostringstream s;
 
   for(iter=glb->begin();iter!=glb->end();++iter) {
@@ -43,9 +42,8 @@ void SleighBase::buildXrefs(void)
       pair<VarnodeData,string> ins(((VarnodeSymbol *)sym)->getFixedVarnode(),sym->getName());
       pair<map<VarnodeData,string>::iterator,bool> res = varnode_xref.insert(ins);
       if (!res.second) {
-	s << "Duplicate (offset,size) pair for registers: ";
+	errorPairs.push_back(sym->getName());
-	s << sym->getName() << " and " << (*(res.first)).second << '\n';
+	errorPairs.push_back((*(res.first)).second);
-	errors += 1;
       }
     }
     else if (sym->getType() == SleighSymbol::userop_symbol) {
@@ -62,8 +60,6 @@ void SleighBase::buildXrefs(void)
       registerContext(csym->getName(),startbit,endbit);
     }
   }
-  if (errors > 0)
-    throw SleighError(s.str());
 }
 
 /// If \b this SleighBase is being reused with a new program, the context
@@ -239,5 +235,8 @@ void SleighBase::restoreXml(const Element *el)
   iter++;
   symtab.restoreXml(*iter,this);
   root = (SubtableSymbol *)symtab.getGlobalScope()->findSymbol("instruction");
-  buildXrefs();
+  vector<string> errorPairs;
+  buildXrefs(errorPairs);
+  if (!errorPairs.empty())
+    throw SleighError("Duplicate register pairs");
 }

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

@@ -36,7 +36,7 @@ protected:
   uint4 maxdelayslotbytes;	///< Maximum number of bytes in a delay-slot directive
   uint4 unique_allocatemask;	///< Bits that are guaranteed to be zero in the unique allocation scheme
   uint4 numSections;		///< Number of \e named sections
-  void buildXrefs(void);	///< Build register map. Collect user-ops and context-fields.
+  void buildXrefs(vector<string> &errorPairs);	///< Build register map. Collect user-ops and context-fields.
   void reregisterContext(void);	///< Reregister context fields for a new executable
   void restoreXml(const Element *el);	///< Read a SLEIGH specification from XML
 public:

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

@@ -186,7 +186,7 @@ SubtableSymbol *WithBlock::getCurrentSubtable(const list<WithBlock> &stack)
 ConsistencyChecker::ConsistencyChecker(SleighCompile *sleigh,SubtableSymbol *rt,bool un,bool warndead)
 
 {
-  slgh = sleigh;
+  compiler = sleigh;
   root_symbol = rt;
   unnecessarypcode = 0;
   readnowrite = 0;
@@ -195,16 +195,6 @@ ConsistencyChecker::ConsistencyChecker(SleighCompile *sleigh,SubtableSymbol *rt,
   printdeadwarning = warndead;
 }
 
-void ConsistencyChecker::reportError(const Location *loc, const string &msg)
-{
-  slgh->reportError(loc, msg);
-}
-
-void ConsistencyChecker::reportWarning(const Location *loc, const string &msg)
-{
-  slgh->reportWarning(loc, msg);
-}
-
 int4 ConsistencyChecker::recoverSize(const ConstTpl &sizeconst,Constructor *ct)
 
 {
@@ -245,7 +235,7 @@ void ConsistencyChecker::dealWithUnnecessaryExt(OpTpl *op,Constructor *ct)
     ostringstream msg;
     msg << "Unnecessary ";
     printOpName(msg,op);
-    reportWarning(slgh->getLocation(ct), msg.str());
+    compiler->reportWarning(compiler->getLocation(ct), msg.str());
   }
   op->setOpcode(CPUI_COPY);	// Equivalent to copy
   unnecessarypcode += 1;
@@ -258,7 +248,7 @@ void ConsistencyChecker::dealWithUnnecessaryTrunc(OpTpl *op,Constructor *ct)
     ostringstream msg;
     msg << "Unnecessary ";
     printOpName(msg,op);
-    reportWarning(slgh->getLocation(ct), msg.str());
+    compiler->reportWarning(compiler->getLocation(ct), msg.str());
   }
   op->setOpcode(CPUI_COPY);	// Equivalent to copy
   op->removeInput(1);
@@ -273,7 +263,7 @@ bool ConsistencyChecker::checkOpMisuse(OpTpl *op,Constructor *ct)
     {
       VarnodeTpl *vn = op->getIn(1);
       if (vn->getSpace().isConstSpace() && vn->getOffset().isZero()) {
-	reportError(slgh->getLocation(ct), "Unsigned comparison with zero is always false");
+	compiler->reportError(compiler->getLocation(ct), "Unsigned comparison with zero is always false");
       }
     }
     break;
@@ -800,7 +790,7 @@ void ConsistencyChecker::printOpError(OpTpl *op,Constructor *ct,int4 err1,int4 e
   printOpName(msgBuilder,op);
   msgBuilder << " operator" << endl << "  " << msg;
 
-  reportError(slgh->getLocation(ct), msgBuilder.str());
+  compiler->reportError(compiler->getLocation(ct), msgBuilder.str());
 }
 
 bool ConsistencyChecker::checkConstructorSection(Constructor *ct,ConstructTpl *cttpl)
@@ -896,7 +886,7 @@ bool ConsistencyChecker::checkSubtable(SubtableSymbol *sym)
 	ostringstream msg;
 	msg << "Table '" << sym->getName() << "' exports inconsistently; ";
 	msg << "Constructor starting at line " << dec << ct->getLineno() << " is first inconsistency";
-	reportError(slgh->getLocation(ct), msg.str());
+	compiler->reportError(compiler->getLocation(ct), msg.str());
 	testresult = false;
       }
       seennonemptyexport = true;
@@ -907,7 +897,7 @@ bool ConsistencyChecker::checkSubtable(SubtableSymbol *sym)
 	ostringstream msg;
 	msg << "Table '" << sym->getName() << "' has inconsistent export size; ";
 	msg << "Constructor starting at line " << dec << ct->getLineno() << " is first conflict";
-	reportError(slgh->getLocation(ct), msg.str());
+	compiler->reportError(compiler->getLocation(ct), msg.str());
 	testresult = false;
       }
     }
@@ -916,7 +906,7 @@ bool ConsistencyChecker::checkSubtable(SubtableSymbol *sym)
 	ostringstream msg;
 	msg << "Table '" << sym->getName() << "' exports inconsistently; ";
 	msg << "Constructor starting at line " << dec << ct->getLineno() << " is first inconsistency";
-	reportError(slgh->getLocation(ct), msg.str());
+	compiler->reportError(compiler->getLocation(ct), msg.str());
 	testresult = false;
       }
       seenemptyexport = true;
@@ -924,7 +914,7 @@ bool ConsistencyChecker::checkSubtable(SubtableSymbol *sym)
   }
   if (seennonemptyexport) {
     if (tablesize == 0) {
-      reportWarning(slgh->getLocation(sym), "Table '" + sym->getName() + "' exports size 0");
+      compiler->reportWarning(compiler->getLocation(sym), "Table '" + sym->getName() + "' exports size 0");
     }
     sizemap[sym] = tablesize;	// Remember recovered size
   }
@@ -1234,11 +1224,11 @@ void ConsistencyChecker::checkUnusedTemps(Constructor *ct,const map<uintb,Optimi
     const OptimizeRecord &currec( (*iter).second );
     if (currec.readcount == 0) {
       if (printdeadwarning)
-	reportWarning(slgh->getLocation(ct), "Temporary is written but not read");
+	compiler->reportWarning(compiler->getLocation(ct), "Temporary is written but not read");
       writenoread += 1;
     }
     else if (currec.writecount == 0) {
-      reportError(slgh->getLocation(ct), "Temporary is read but not written");
+      compiler->reportError(compiler->getLocation(ct), "Temporary is read but not written");
       readnowrite += 1;
     }
     ++iter;
@@ -1883,10 +1873,15 @@ void SleighCompile::process(void)
   if (errors>0) return;
   buildDecisionTrees();
   if (errors>0) return;
-  try {
+  vector<string> errorPairs;
-    buildXrefs();		// Make sure we can build crossrefs properly
+  buildXrefs(errorPairs);		// Make sure we can build crossrefs properly
-  } catch(SleighError &err) {
+  if (!errorPairs.empty()) {
-    reportError(err.explain);
+    for(int4 i=0;i<errorPairs.size();i+=2) {
+      ostringstream s;
+      s << "Duplicate (offset,size) pair for registers: ";
+      s << errorPairs[i] << " and " << errorPairs[i+1] << endl;
+      reportError(s.str());
+    }
     return;
   }
   checkUniqueAllocation();

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

@@ -93,7 +93,7 @@ class ConsistencyChecker {
     OptimizeRecord(void) {
       writeop = -1; readop = -1; inslot=-1; writecount=0; readcount=0; writesection=-2; readsection=-2; opttype=-1; }
   };
-  SleighCompile *slgh;
+  SleighCompile *compiler;
   int4 unnecessarypcode;
   int4 readnowrite;
   int4 writenoread;
@@ -102,8 +102,6 @@ class ConsistencyChecker {
   SubtableSymbol *root_symbol;
   vector<SubtableSymbol *> postorder;
   map<SubtableSymbol *,int4> sizemap; // Sizes associated with tables
-  void reportError(const Location* loc, const string &msg);
-  void reportWarning(const Location* loc, const string &msg);
   OperandSymbol *getOperandSymbol(int4 slot,OpTpl *op,Constructor *ct);
   void printOpName(ostream &s,OpTpl *op);
   void printOpError(OpTpl *op,Constructor *ct,int4 err1,int4 err2,const string &message);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcode/utils/MessageFormattingUtils.java

@@ -1,3 +1,18 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
 package ghidra.pcode.utils;
 
 import ghidra.sleigh.grammar.Location;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcodeCPort/sleighbase/SleighBase.java

@@ -16,9 +16,9 @@
 package ghidra.pcodeCPort.sleighbase;
 
 import java.io.PrintStream;
+import java.util.ArrayList;
 
 import generic.stl.*;
-import ghidra.pcode.utils.MessageFormattingUtils;
 import ghidra.pcodeCPort.context.SleighError;
 import ghidra.pcodeCPort.pcoderaw.VarnodeData;
 import ghidra.pcodeCPort.slghpatexpress.ContextField;
@@ -27,8 +27,6 @@ import ghidra.pcodeCPort.space.AddrSpace;
 import ghidra.pcodeCPort.space.spacetype;
 import ghidra.pcodeCPort.translate.Translate;
 import ghidra.pcodeCPort.utils.XmlUtils;
-import ghidra.sleigh.grammar.Location;
-import ghidra.util.Msg;
 
 public abstract class SleighBase extends Translate implements NamedSymbolProvider {
 
@@ -69,39 +67,17 @@ public abstract class SleighBase extends Translate implements NamedSymbolProvide
 		return (root != null);
 	}
 
-	public void reportError(Location location, String msg) {
+	protected void buildXrefs(ArrayList<SleighSymbol> errorPairs) {
-		Msg.error(this, MessageFormattingUtils.format(location, msg));
-	}
-
-	public void reportError(Location location, String msg, Throwable t) {
-		Msg.error(this, MessageFormattingUtils.format(location, msg), t);
-	}
-
-	public void reportWarning(Location location, String msg) {
-		Msg.warn(this, MessageFormattingUtils.format(location, msg));
-	}
-
-	protected void buildXrefs() {
 		SymbolScope glb = symtab.getGlobalScope();
-		int errors = 0;
 		glb.begin();
 		IteratorSTL<SleighSymbol> iter;
-		StringBuffer buffer = new StringBuffer();
 		for (iter = glb.begin(); !iter.isEnd(); iter.increment()) {
 			SleighSymbol sym = iter.get();
 			if (sym.getType() == symbol_type.varnode_symbol) {
 				Pair<IteratorSTL<VarnodeSymbol>, Boolean> res = varnode_xref.insert((VarnodeSymbol) sym);
 				if (!res.second) {
-
+					errorPairs.add(sym);
-					String msg = String.format("Duplicate (offset,size) pair for registers: %s (%s) and %s (%s)",
+					errorPairs.add(res.first.get());
-							sym.getName(), sym.getLocation(), res.first.get().getName(), res.first.get().getLocation());
-
-					buffer.append(msg + "\n");
-
-					reportError(sym.getLocation(), msg);
-					reportError(res.first.get().getLocation(), msg);
-
-					errors += 1;
 				}
 			} else if (sym.getType() == symbol_type.userop_symbol) {
 				int index = ((UserOpSymbol) sym).getIndex();
@@ -117,9 +93,6 @@ public abstract class SleighBase extends Translate implements NamedSymbolProvide
 				registerContext(csym.getName(), startbit, endbit);
 			}
 		}
-		if (errors > 0) {
-			throw new SleighError(buffer.toString(), null);
-		}
 	}
 
 	protected void reregisterContext() {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcodeCPort/slgh_compile/ConsistencyChecker.java

@@ -25,8 +25,6 @@ import ghidra.pcodeCPort.semantics.ConstTpl.const_type;
 import ghidra.pcodeCPort.semantics.ConstTpl.v_field;
 import ghidra.pcodeCPort.slghsymbol.*;
 import ghidra.pcodeCPort.space.AddrSpace;
-import ghidra.sleigh.grammar.Location;
-import ghidra.util.Msg;
 
 class ConsistencyChecker {
 
@@ -35,6 +33,7 @@ class ConsistencyChecker {
 	int writenoread;
 	boolean printextwarning;
 	boolean printdeadwarning;
+	SleighCompile compiler;
 	SubtableSymbol root_symbol;
 	VectorSTL<SubtableSymbol> postorder = new VectorSTL<>();
 
@@ -480,30 +479,6 @@ class ConsistencyChecker {
 		}
 	}
 
-	private void reportError(Location location, CharSequence message) {
-		if (location == null) {
-			Msg.error(this, message);
-			return;
-		}
-		StringBuilder sb = new StringBuilder();
-		sb.append(location).append(": ").append(message);
-
-		Msg.error(this, sb.toString());
-	}
-
-	private void reportWarning(Location location, CharSequence message) {
-		if (location == null) {
-			Msg.warn(this, message);
-			return;
-		}
-
-		StringBuilder sb = new StringBuilder();
-
-		sb.append(location).append(": ").append(message);
-
-		Msg.warn(this, sb.toString());
-	}
-
 	void printOpError(OpTpl op, Constructor ct, int err1, int err2, String message) {
 		SubtableSymbol sym = ct.getParent();
 		OperandSymbol op1, op2;
@@ -537,7 +512,7 @@ class ConsistencyChecker {
 
 		sb.append("\n  ").append(message);
 
-		reportError(op.location, sb);
+		compiler.reportError(op.location, sb.toString());
 
 	}
 
@@ -576,7 +551,7 @@ class ConsistencyChecker {
 	}
 
 	private void handle(String msg, Constructor ct) {
-		reportError(ct.location, " Unsigned comparison with " + msg + " in constructor");
+		compiler.reportError(ct.location, " Unsigned comparison with " + msg + " in constructor");
 	}
 
 	private void handleZero(String trueOrFalse, Constructor ct) {
@@ -740,7 +715,7 @@ class ConsistencyChecker {
 			HandleTpl exportres = ct.getTempl().getResult();
 			if (exportres != null) {
 				if (seenemptyexport && (!seennonemptyexport)) {
-					reportError(ct.location, String.format(
+					compiler.reportError(ct.location, String.format(
 						"Table '%s' exports inconsistently; Constructor at %s is first inconsitency",
 						sym.getName(), ct.location));
 					testresult = false;
@@ -751,7 +726,7 @@ class ConsistencyChecker {
 					tablesize = exsize;
 				}
 				if ((exsize != 0) && (exsize != tablesize)) {
-					reportError(ct.location, String.format(
+					compiler.reportError(ct.location, String.format(
 						"Table '%s' has inconsistent export size; Constructor at %s is first conflict",
 						sym.getName(), ct.location));
 					testresult = false;
@@ -759,7 +734,7 @@ class ConsistencyChecker {
 			}
 			else {
 				if (seennonemptyexport && (!seenemptyexport)) {
-					reportError(ct.location, String.format(
+					compiler.reportError(ct.location, String.format(
 						"Table '%s' exports inconsistently; Constructor at %s is first inconsitency",
 						sym.getName(), ct.location));
 					testresult = false;
@@ -769,7 +744,8 @@ class ConsistencyChecker {
 		}
 		if (seennonemptyexport) {
 			if (tablesize == 0) {
-				reportWarning(sym.location, "Table '" + sym.getName() + "' exports size 0");
+				compiler.reportWarning(sym.location,
+					"Table '" + sym.getName() + "' exports size 0");
 
 			}
 			sizemap.put(sym, tablesize);	// Remember recovered size
@@ -785,7 +761,7 @@ class ConsistencyChecker {
 	// input size is the same as the output size
 	void dealWithUnnecessaryExt(OpTpl op, Constructor ct) {
 		if (printextwarning) {
-			reportWarning(op.location, "Unnecessary '" + getOpName(op) + "'");
+			compiler.reportWarning(op.location, "Unnecessary '" + getOpName(op) + "'");
 		}
 		op.setOpcode(OpCode.CPUI_COPY); // Equivalent to copy
 		unnecessarypcode += 1;
@@ -793,7 +769,7 @@ class ConsistencyChecker {
 
 	void dealWithUnnecessaryTrunc(OpTpl op, Constructor ct) {
 		if (printextwarning) {
-			reportWarning(op.location, "Unnecessary '" + getOpName(op) + "'");
+			compiler.reportWarning(op.location, "Unnecessary '" + getOpName(op) + "'");
 		}
 		op.setOpcode(OpCode.CPUI_COPY); // Equivalent to copy
 		op.removeInput(1);
@@ -1144,12 +1120,12 @@ class ConsistencyChecker {
 			OptimizeRecord currec = pair.second;
 			if (currec.readcount == 0) {
 				if (printdeadwarning) {
-					reportWarning(ct.location, "Temporary is written but not read");
+					compiler.reportWarning(ct.location, "Temporary is written but not read");
 				}
 				writenoread += 1;
 			}
 			else if (currec.writecount == 0) {
-				reportError(ct.location, "Temporary is read but not written");
+				compiler.reportError(ct.location, "Temporary is read but not written");
 				readnowrite += 1;
 			}
 			iter.increment();
@@ -1175,7 +1151,8 @@ class ConsistencyChecker {
 		checkUnusedTemps(ct, recs);
 	}
 
-	ConsistencyChecker(SubtableSymbol rt, boolean unnecessary, boolean warndead) {
+	ConsistencyChecker(SleighCompile cp, SubtableSymbol rt, boolean unnecessary, boolean warndead) {
+		compiler = cp;
 		root_symbol = rt;
 		unnecessarypcode = 0;
 		readnowrite = 0;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcodeCPort/slgh_compile/SleighCompile.java

@@ -499,7 +499,7 @@ public class SleighCompile extends SleighBase {
 	void checkConsistency() {
 		entry("checkConsistency");
 		ConsistencyChecker checker =
-			new ConsistencyChecker(root, warnunnecessarypcode, warndeadtemps);
+			new ConsistencyChecker(this, root, warnunnecessarypcode, warndeadtemps);
 
 		if (!checker.test()) {
 			errors += 1;
@@ -650,26 +650,23 @@ public class SleighCompile extends SleighBase {
 	}
 
 
-	@Override
 	public void reportError(Location location, String msg) {
 		entry("reportError", location, msg);
-		super.reportError(location, msg);
+		Msg.error(this, MessageFormattingUtils.format(location, msg));
 
 		errors += 1;
 	}
 
-	@Override
 	public void reportError(Location location, String msg, Throwable t) {
 		entry("reportError", location, msg);
-		super.reportError(location, msg, t);
+		Msg.error(this, MessageFormattingUtils.format(location, msg), t);
 
 		errors += 1;
 	}
 
-	@Override
 	public void reportWarning(Location location, String msg) {
 		entry("reportWarning", location, msg);
-		super.reportWarning(location, msg);
+		Msg.warn(this, MessageFormattingUtils.format(location, msg));
 
 		warnings += 1;
 	}
@@ -757,11 +754,19 @@ public class SleighCompile extends SleighBase {
 		if (errors > 0) {
 			return;
 		}
-		try {
+		ArrayList<SleighSymbol> errorPairs = new ArrayList<SleighSymbol>();
-			buildXrefs(); // Make sure we can build crossrefs properly
+		buildXrefs(errorPairs);			// Make sure we can build crossrefs properly
-		}
+		if (!errorPairs.isEmpty()) {
-		catch (SleighError err) {
+			for (int i = 0; i < errorPairs.size(); i += 2) {
-			Msg.error(this, err.location + ": " + err.getMessage(), err);
+				SleighSymbol sym1 = errorPairs.get(i);
+				SleighSymbol sym2 = errorPairs.get(i + 1);
+				String msg =
+					String.format("Duplicate (offset,size) pair for registers: %s (%s) and %s (%s)",
+						sym1.getName(), sym1.getLocation(), sym2.getName(), sym2.getLocation());
+
+				reportError(sym1.getLocation(), msg);
+				reportError(sym2.getLocation(), msg);
+			}
 			errors += 1;
 			return;
 		}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcodeCPort/slghpatexpress/TokenPattern.java

@@ -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.