From 576afa4088db817bfe604e97d0ea4b18febdfffb Mon Sep 17 00:00:00 2001 From: caheckman <48068198+caheckman@users.noreply.github.com> Date: Wed, 14 Aug 2019 17:05:02 -0400 Subject: [PATCH] print both colliding operands, command line option --- .../src/decompile/cpp/slgh_compile.cc | 133 ++++++++++++------ .../src/decompile/cpp/slgh_compile.hh | 6 +- .../src/decompile/cpp/slghsymbol.cc | 14 +- .../src/decompile/cpp/slghsymbol.hh | 10 +- 4 files changed, 106 insertions(+), 57 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc index dfb28cf95a..e7c34bec6a 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.cc @@ -788,35 +788,6 @@ void ConsistencyChecker::printOpError(OpTpl *op,Constructor *ct,int4 err1,int4 e cerr << " operator" << endl << " " << msg << endl; } -bool ConsistencyChecker::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; - set collect; - ct->getOperand(0)->collectLocalValues(collect); - for(int4 i=1;igetNumOperands();++i) { - set newCollect; - ct->getOperand(i)->collectLocalValues(newCollect); - if (newCollect.empty()) continue; - int4 newSize = collect.size() + newCollect.size(); - collect.insert(newCollect.begin(),newCollect.end()); - if (newSize != collect.size()) { - noCollisions = false; - cerr << "Possible local export collision with symbol "; - cerr << ct->getOperand(i)->getName(); - cerr << " in constructor starting at line " << dec << ct->getLineno() << endl; - break; // Don't continue - } - } - return noCollisions; -} - bool ConsistencyChecker::checkConstructorSection(Constructor *ct,ConstructTpl *cttpl) { // Check all the OpTpl s within the given section for consistency, return true if all tests pass @@ -895,7 +866,6 @@ bool ConsistencyChecker::checkSubtable(SubtableSymbol *sym) for(int4 i=0;igetConstructor(i); - checkLocalExports(ct); if (!checkConstructorSection(ct,ct->getTempl())) testresult = false; int4 numsection = ct->getNumSections(); @@ -1493,6 +1463,7 @@ SleighCompile::SleighCompile(void) warnunnecessarypcode = false; warndeadtemps = false; lenientconflicterrors = true; + warnalllocalcollisions = false; warnallnops = false; root = (SubtableSymbol *)0; } @@ -1650,6 +1621,74 @@ void SleighCompile::checkConsistency(void) } } +int4 SleighCompile::findCollision(map &local2Operand,const vector &locals,int operand) + +{ + for(int4 i=0;i::iterator,bool> res; + res = local2Operand.insert(pair(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 collect; + for(int4 i=0;igetNumOperands();++i) { + vector 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;jgetConstructor(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) { @@ -1731,6 +1770,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(); @@ -2720,7 +2761,8 @@ static void findSlaSpecs(vector &res, const string &dir, const string &s } } -static void initCompiler(SleighCompile &compiler, map &defines, bool enableUnnecessaryPcodeWarning, bool disableLenientConflict, +static void initCompiler(SleighCompile &compiler, map &defines, bool enableUnnecessaryPcodeWarning, + bool disableLenientConflict, bool enableAllCollisionWarning, bool enableAllNopWarning,bool enableDeadTempWarning,bool enforceLocalKeyWord) { @@ -2728,21 +2770,18 @@ static void initCompiler(SleighCompile &compiler, map &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) { @@ -2769,6 +2808,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); } @@ -2778,6 +2818,7 @@ int main(int argc,char **argv) map defines; bool enableUnnecessaryPcodeWarning = false; bool disableLenientConflict = false; + bool enableAllCollisionWarning = false; bool enableAllNopWarning = false; bool enableDeadTempWarning = false; bool enforceLocalKeyWord = false; @@ -2804,6 +2845,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') @@ -2841,7 +2884,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 @@ -2874,7 +2918,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]); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.hh index d71cfe5023..dd12825809 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/slgh_compile.hh @@ -105,7 +105,6 @@ class ConsistencyChecker { int4 recoverSize(const ConstTpl &sizeconst,Constructor *ct); bool checkOpMisuse(OpTpl *op,Constructor *ct); bool sizeRestriction(OpTpl *op,Constructor *ct); - bool checkLocalExports(Constructor *ct); bool checkConstructorSection(Constructor *ct,ConstructTpl *cttpl); bool checkVarnodeTruncation(Constructor *ct,int4 slot,OpTpl *op,VarnodeTpl *vn,bool isbigendian); bool checkSectionTruncations(Constructor *ct,ConstructTpl *cttpl,bool isbigendian); @@ -196,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 noplist; // List of individual NOP warnings int4 errors; @@ -204,6 +204,9 @@ private: void buildDecisionTrees(void); void buildPatterns(void); void checkConsistency(void); + static int4 findCollision(map &local2Operand,const vector &locals,int operand); + bool checkLocalExports(Constructor *ct); + void checkLocalCollisions(void); void checkNops(void); string checkSymbols(SymbolScope *scope); void addSymbol(SleighSymbol *sym); @@ -225,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); diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc index 02873b9742..c5bfa420a4 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/slghsymbol.cc @@ -728,11 +728,11 @@ void VarnodeSymbol::getFixedHandle(FixedHandle &hand,ParserWalker &walker) const hand.size = fix.size; } -void VarnodeSymbol::collectLocalValues(set &results) const +void VarnodeSymbol::collectLocalValues(vector &results) const { if (fix.space->getType() == IPTR_INTERNAL) - results.insert(fix.offset); + results.push_back(fix.offset); } void VarnodeSymbol::saveXml(ostream &s) const @@ -1041,7 +1041,7 @@ void OperandSymbol::print(ostream &s,ParserWalker &walker) const walker.popOperand(); } -void OperandSymbol::collectLocalValues(set &results) const +void OperandSymbol::collectLocalValues(vector &results) const { if (triple != (TripleSymbol *)0) @@ -1556,7 +1556,7 @@ void Constructor::markSubtableOperands(vector &check) const } } -void Constructor::collectLocalExports(set &results) const +void Constructor::collectLocalExports(vector &results) const { if (templ == (ConstructTpl *)0) return; @@ -1565,11 +1565,11 @@ void Constructor::collectLocalExports(set &results) const 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.insert(handle->getTempOffset().getReal()); + results.push_back(handle->getTempOffset().getReal()); return; } if (handle->getSpace().isUniqueSpace()) { - results.insert(handle->getPtrOffset().getReal()); + results.push_back(handle->getPtrOffset().getReal()); return; } if (handle->getSpace().getType() == ConstTpl::handle) { @@ -1899,7 +1899,7 @@ SubtableSymbol::~SubtableSymbol(void) delete *iter; } -void SubtableSymbol::collectLocalValues(set &results) const +void SubtableSymbol::collectLocalValues(vector &results) const { for(int4 i=0;i &results) const {} + virtual void collectLocalValues(vector &results) const {} }; class FamilySymbol : public TripleSymbol { @@ -255,7 +255,7 @@ public: virtual int4 getSize(void) const { return fix.size; } virtual void print(ostream &s,ParserWalker &walker) const { s << getName(); } - virtual void collectLocalValues(set &results) const; + virtual void collectLocalValues(vector &results) const; virtual symbol_type getType(void) const { return varnode_symbol; } virtual void saveXml(ostream &s) const; virtual void saveXmlHeader(ostream &s) const; @@ -350,7 +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(set &results) const; + virtual void collectLocalValues(vector &results) const; virtual symbol_type getType(void) const { return operand_symbol; } virtual void saveXml(ostream &s) const; virtual void saveXmlHeader(ostream &s) const; @@ -516,7 +516,7 @@ public: (*iter)->apply(walker); } void markSubtableOperands(vector &check) const; - void collectLocalExports(set &results) const; + void collectLocalExports(vector &results) const; void setError(bool val) const { inerror = val; } bool isError(void) const { return inerror; } bool isRecursive(void) const; @@ -582,7 +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(set &results) const; + virtual void collectLocalValues(vector &results) const; virtual symbol_type getType(void) const { return subtable_symbol; } virtual void saveXml(ostream &s) const; virtual void saveXmlHeader(ostream &s) const;