Merge remote-tracking branch 'origin/GT-3085_SleighCollisionWarning'

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

@@ -797,6 +797,19 @@ int4 ConstructTpl::fillinBuild(vector<int4> &check,AddrSpace *const_space)
   return 0;
 }
 
+bool ConstructTpl::buildOnly(void) const
+
+{
+  vector<OpTpl *>::const_iterator iter;
+  OpTpl *op;
+  for(iter=vec.begin();iter!=vec.end();++iter) {
+    op = *iter;
+    if (op->getOpcode() != BUILD)
+      return false;
+  }
+  return true;
+}
+
 void ConstructTpl::changeHandleIndex(const vector<int4> &handmap)
 
 {

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

@@ -178,6 +178,7 @@ public:
   bool addOpList(const vector<OpTpl *> &oplist);
   void setResult(HandleTpl *t) { result = t; }
   int4 fillinBuild(vector<int4> &check,AddrSpace *const_space);
+  bool buildOnly(void) const;
   void changeHandleIndex(const vector<int4> &handmap);
   void setInput(VarnodeTpl *vn,int4 index,int4 slot);
   void setOutput(VarnodeTpl *vn,int4 index);

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

@@ -1463,6 +1463,7 @@ SleighCompile::SleighCompile(void)
   warnunnecessarypcode = false;
   warndeadtemps = false;
   lenientconflicterrors = true;
+  warnalllocalcollisions = false;
   warnallnops = false;
   root = (SubtableSymbol *)0;
 }
@@ -1624,6 +1625,74 @@ void SleighCompile::checkConsistency(void)
   }
 }
 
+int4 SleighCompile::findCollision(map<uintb,int4> &local2Operand,const vector<uintb> &locals,int operand)
+
+{
+  for(int4 i=0;i<locals.size();++i) {
+    pair<map<uintb,int4>::iterator,bool> res;
+    res = local2Operand.insert(pair<uintb,int4>(locals[i],operand));
+    if (!res.second) {
+      int4 oldIndex = (*res.first).second;
+      if (oldIndex != operand)
+	return oldIndex;
+    }
+  }
+  return -1;
+}
+
+bool SleighCompile::checkLocalExports(Constructor *ct)
+
+{
+  if (ct->getTempl() == (ConstructTpl *)0)
+    return true;		// No template, collisions impossible
+  if (ct->getTempl()->buildOnly())
+    return true;		// Operand exports aren't manipulated, so no collision is possible
+  if (ct->getNumOperands() < 2)
+    return true;		// Collision can only happen with multiple operands
+  bool noCollisions = true;
+  map<uintb,int4> collect;
+  for(int4 i=0;i<ct->getNumOperands();++i) {
+    vector<uintb> newCollect;
+    ct->getOperand(i)->collectLocalValues(newCollect);
+    if (newCollect.empty()) continue;
+    int4 collideOperand = findCollision(collect, newCollect, i);
+    if (collideOperand >= 0) {
+      noCollisions = false;
+      if (warnalllocalcollisions) {
+	cerr << "Possible collision with symbols ";
+	cerr << ct->getOperand(collideOperand)->getName();
+	cerr << " and " << ct->getOperand(i)->getName();
+	cerr << " in constructor starting at line " << dec << ct->getLineno() << endl;
+      }
+      break;	// Don't continue
+    }
+  }
+  return noCollisions;
+}
+
+void SleighCompile::checkLocalCollisions(void)
+
+{
+  int4 collisionCount = 0;
+  SubtableSymbol *sym = root; // Start with the instruction table
+  int4 i = -1;
+  for(;;) {
+    int4 numconst = sym->getNumConstructors();
+    for(int4 j=0;j<numconst;++j) {
+      if (!checkLocalExports(sym->getConstructor(j)))
+	collisionCount += 1;
+    }
+    i+=1;
+    if (i>=tables.size()) break;
+    sym = tables[i];
+  }
+  if (collisionCount > 0) {
+    cerr << "WARNING: " << dec << collisionCount << " constructors with local collisions between operands" << endl;
+    if (!warnalllocalcollisions)
+      cerr << "Use -c switch to list each individually" << endl;
+  }
+}
+
 void SleighCompile::checkNops(void)
 
 {
@@ -1705,6 +1774,8 @@ void SleighCompile::process(void)
   if (errors>0) return;
   checkConsistency();
   if (errors>0) return;
+  checkLocalCollisions();
+  if (errors>0) return;
   buildPatterns();
   if (errors>0) return;
   buildDecisionTrees();
@@ -2694,7 +2765,8 @@ static void findSlaSpecs(vector<string> &res, const string &dir, const string &s
   }
 }
 
-static void initCompiler(SleighCompile &compiler, map<string,string> &defines, bool enableUnnecessaryPcodeWarning, bool disableLenientConflict,
+static void initCompiler(SleighCompile &compiler, map<string,string> &defines, bool enableUnnecessaryPcodeWarning,
+			 bool disableLenientConflict, bool enableAllCollisionWarning,
 			 bool enableAllNopWarning,bool enableDeadTempWarning,bool enforceLocalKeyWord)
 
 {
@@ -2702,21 +2774,18 @@ static void initCompiler(SleighCompile &compiler, map<string,string> &defines, b
   for (iter = defines.begin(); iter != defines.end(); iter++) {
     compiler.setPreprocValue((*iter).first, (*iter).second);
   }
-  if (enableUnnecessaryPcodeWarning) {
+  if (enableUnnecessaryPcodeWarning)
     compiler.setUnnecessaryPcodeWarning(true);
-  }
-  if (disableLenientConflict) {
+  if (disableLenientConflict)
     compiler.setLenientConflict(false);
-  }
-  if (enableAllNopWarning) {
+  if (enableAllCollisionWarning)
+    compiler.setLocalCollisionWarning( true );
+  if (enableAllNopWarning)
     compiler.setAllNopWarning( true );
-  }
-  if (enableDeadTempWarning) {
+  if (enableDeadTempWarning)
     compiler.setDeadTempWarning(true);
-  }
-  if (enforceLocalKeyWord) {
-	  compiler.setEnforceLocalKeyWord(true);
-  }
+  if (enforceLocalKeyWord)
+    compiler.setEnforceLocalKeyWord(true);
 }
 
 static void segvHandler(int sig) {
@@ -2743,6 +2812,7 @@ int main(int argc,char **argv)
     cerr << "   -n              print warnings for all NOP constructors" << endl;
     cerr << "   -t              print warnings for dead temporaries" << endl;
     cerr << "   -e              enforce use of 'local' keyword for temporaries" << endl;
+    cerr << "   -c              print warnings for all constructors with colliding operands" << endl;
     cerr << "   -DNAME=VALUE    defines a preprocessor macro NAME with value VALUE" << endl;
     exit(2);
   }
@@ -2752,6 +2822,7 @@ int main(int argc,char **argv)
   map<string,string> defines;
   bool enableUnnecessaryPcodeWarning = false;
   bool disableLenientConflict = false;
+  bool enableAllCollisionWarning = false;
   bool enableAllNopWarning = false;
   bool enableDeadTempWarning = false;
   bool enforceLocalKeyWord = false;
@@ -2778,6 +2849,8 @@ int main(int argc,char **argv)
       enableUnnecessaryPcodeWarning = true;
     else if (argv[i][1] == 'l')
       disableLenientConflict = true;
+    else if (argv[i][1] == 'c')
+      enableAllCollisionWarning = true;
     else if (argv[i][1] == 'n')
       enableAllNopWarning = true;
     else if (argv[1][1] == 't')
@@ -2815,7 +2888,8 @@ int main(int argc,char **argv)
       sla.replace(slaspec.length() - slaspecExtLen, slaspecExtLen, SLAEXT);
       SleighCompile compiler;
       initCompiler(compiler, defines, enableUnnecessaryPcodeWarning, 
-		   disableLenientConflict, enableAllNopWarning, enableDeadTempWarning, enforceLocalKeyWord);
+		   disableLenientConflict, enableAllCollisionWarning, enableAllNopWarning,
+		   enableDeadTempWarning, enforceLocalKeyWord);
       retval = run_compilation(slaspec.c_str(),sla.c_str(),compiler);
       if (retval != 0) {
 	return retval; // stop on first error
@@ -2848,7 +2922,8 @@ int main(int argc,char **argv)
     
     SleighCompile compiler;
     initCompiler(compiler, defines, enableUnnecessaryPcodeWarning, 
-		 disableLenientConflict, enableAllNopWarning, enableDeadTempWarning, enforceLocalKeyWord);
+		 disableLenientConflict, enableAllCollisionWarning, enableAllNopWarning,
+		 enableDeadTempWarning, enforceLocalKeyWord);
     
     if (i < argc - 1) {
       string fileoutExamine(argv[i+1]);

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

@@ -195,6 +195,7 @@ private:
   bool warnunnecessarypcode;	// True if we warn of unnecessary ZEXT or SEXT
   bool warndeadtemps;		// True if we warn of temporaries that are written but not read
   bool lenientconflicterrors;	// True if we ignore most pattern conflict errors
+  bool warnalllocalcollisions;	// True if local export collisions generate individual warnings
   bool warnallnops;		// True if pcode NOPs generate individual warnings
   vector<string> noplist;	// List of individual NOP warnings
   int4 errors;
@@ -203,6 +204,9 @@ private:
   void buildDecisionTrees(void);
   void buildPatterns(void);
   void checkConsistency(void);
+  static int4 findCollision(map<uintb,int4> &local2Operand,const vector<uintb> &locals,int operand);
+  bool checkLocalExports(Constructor *ct);
+  void checkLocalCollisions(void);
   void checkNops(void);
   string checkSymbols(SymbolScope *scope);
   void addSymbol(SleighSymbol *sym);
@@ -224,6 +228,7 @@ public:
   void setDeadTempWarning(bool val) { warndeadtemps = val; }
   void setEnforceLocalKeyWord(bool val) { pcode.setEnforceLocalKey(val); }
   void setLenientConflict(bool val) { lenientconflicterrors = val; }
+  void setLocalCollisionWarning(bool val) { warnalllocalcollisions = val; }
   void setAllNopWarning(bool val) { warnallnops = val; }
   void process(void);
 

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

@@ -728,6 +728,13 @@ void VarnodeSymbol::getFixedHandle(FixedHandle &hand,ParserWalker &walker) const
   hand.size = fix.size;
 }
 
+void VarnodeSymbol::collectLocalValues(vector<uintb> &results) const
+
+{
+  if (fix.space->getType() == IPTR_INTERNAL)
+    results.push_back(fix.offset);
+}
+
 void VarnodeSymbol::saveXml(ostream &s) const
 
 {
@@ -1034,6 +1041,13 @@ void OperandSymbol::print(ostream &s,ParserWalker &walker) const
   walker.popOperand();
 }
 
+void OperandSymbol::collectLocalValues(vector<uintb> &results) const
+
+{
+  if (triple != (TripleSymbol *)0)
+    triple->collectLocalValues(results);
+}
+
 void OperandSymbol::saveXml(ostream &s) const
 
 {
@@ -1542,6 +1556,29 @@ void Constructor::markSubtableOperands(vector<int4> &check) const
   }
 }
 
+void Constructor::collectLocalExports(vector<uintb> &results) const
+
+{
+  if (templ == (ConstructTpl *)0) return;
+  HandleTpl *handle = templ->getResult();
+  if (handle == (HandleTpl *)0) return;
+  if (handle->getSpace().isConstSpace()) return;	// Even if the value is dynamic, the pointed to value won't get used
+  if (handle->getPtrSpace().getType() != ConstTpl::real) {
+    if (handle->getTempSpace().isUniqueSpace())
+      results.push_back(handle->getTempOffset().getReal());
+    return;
+  }
+  if (handle->getSpace().isUniqueSpace()) {
+    results.push_back(handle->getPtrOffset().getReal());
+    return;
+  }
+  if (handle->getSpace().getType() == ConstTpl::handle) {
+    int4 handleIndex = handle->getSpace().getHandleIndex();
+    OperandSymbol *opSym = getOperand(handleIndex);
+    opSym->collectLocalValues(results);
+  }
+}
+
 bool Constructor::isRecursive(void) const
 
 { // Does this constructor cause recursion with its table
@@ -1862,6 +1899,13 @@ SubtableSymbol::~SubtableSymbol(void)
     delete *iter;
 }
 
+void SubtableSymbol::collectLocalValues(vector<uintb> &results) const
+
+{
+  for(int4 i=0;i<construct.size();++i)
+    construct[i]->collectLocalExports(results);
+}
+
 void SubtableSymbol::saveXml(ostream &s) const
 
 {

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

@@ -152,6 +152,7 @@ public:
   virtual void getFixedHandle(FixedHandle &hand,ParserWalker &walker) const=0;
   virtual int4 getSize(void) const { return 0; }	// Size out of context
   virtual void print(ostream &s,ParserWalker &walker) const=0;
+  virtual void collectLocalValues(vector<uintb> &results) const {}
 };
   
 class FamilySymbol : public TripleSymbol {
@@ -254,6 +255,7 @@ public:
   virtual int4 getSize(void) const { return fix.size; }
   virtual void print(ostream &s,ParserWalker &walker) const {
     s << getName(); }
+  virtual void collectLocalValues(vector<uintb> &results) const;
   virtual symbol_type getType(void) const { return varnode_symbol; }
   virtual void saveXml(ostream &s) const;
   virtual void saveXmlHeader(ostream &s) const;
@@ -348,6 +350,7 @@ public:
   virtual void getFixedHandle(FixedHandle &hnd,ParserWalker &walker) const;
   virtual int4 getSize(void) const;
   virtual void print(ostream &s,ParserWalker &walker) const;
+  virtual void collectLocalValues(vector<uintb> &results) const;
   virtual symbol_type getType(void) const { return operand_symbol; }
   virtual void saveXml(ostream &s) const;
   virtual void saveXmlHeader(ostream &s) const;
@@ -513,6 +516,7 @@ public:
       (*iter)->apply(walker);
   }
   void markSubtableOperands(vector<int4> &check) const;
+  void collectLocalExports(vector<uintb> &results) const;
   void setError(bool val) const { inerror = val; }
   bool isError(void) const { return inerror; }
   bool isRecursive(void) const;
@@ -578,6 +582,7 @@ public:
   virtual int4 getSize(void) const { return -1; }
   virtual void print(ostream &s,ParserWalker &walker) const {
     throw SleighError("Cannot use subtable in expression"); }
+  virtual void collectLocalValues(vector<uintb> &results) const;
   virtual symbol_type getType(void) const { return subtable_symbol; }
   virtual void saveXml(ostream &s) const;
   virtual void saveXmlHeader(ostream &s) const;