diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc index e61204390b..622c9c2f88 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc @@ -2116,10 +2116,9 @@ int4 ActionRestructureVarnode::apply(Funcdata &data) { ScopeLocal *l1 = data.getScopeLocal(); - bool aliasyes = data.isJumptableRecoveryOn() ? false : (numpass != 0); + bool aliasyes = (numpass != 0); // Alias calculations are not reliable on the first pass l1->restructureVarnode(aliasyes); - // Note the alias calculation, may not be very good on the first pass - if (data.syncVarnodesWithSymbols(l1,false)) + if (data.syncVarnodesWithSymbols(l1,false,aliasyes)) count += 1; numpass += 1; @@ -2144,7 +2143,7 @@ int4 ActionRestructureHigh::apply(Funcdata &data) #endif l1->restructureHigh(); - if (data.syncVarnodesWithSymbols(l1,true)) + if (data.syncVarnodesWithSymbols(l1,true,true)) count += 1; #ifdef OPACTION_DEBUG diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh index baf13a159c..102e3f52e6 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh @@ -378,7 +378,7 @@ public: bool checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,uint4 fl,const ParamTrial &trial) const; bool onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial,uint4 mainFlags) const; bool ancestorOpUse(int4 maxlevel,const Varnode *invn,const PcodeOp *op,ParamTrial &trial,int4 offset,uint4 mainFlags) const; - bool syncVarnodesWithSymbols(const ScopeLocal *lm,bool typesyes); + bool syncVarnodesWithSymbols(const ScopeLocal *lm,bool updateDatatypes,bool unmappedAliasCheck); Datatype *checkSymbolType(Varnode *vn); ///< Check for any delayed symbol data-type information on the given Varnode void transferVarnodeProperties(Varnode *vn,Varnode *newVn,int4 lsbOffset); bool fillinReadOnly(Varnode *vn); ///< Replace the given Varnode with its (constant) value in the load image diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc index e6b282ac74..7166bfd371 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc @@ -807,9 +807,9 @@ void Funcdata::calcNZMask(void) /// The caller can elect to update data-type information as well, where Varnodes /// and their associated HighVariables have their data-type finalized based symbols. /// \param lm is the Symbol scope within which to search for mapped Varnodes -/// \param typesyes is \b true if the caller wants to update data-types +/// \param updataDatatypes is \b true if the caller wants to update data-types /// \return \b true if any Varnode was updated -bool Funcdata::syncVarnodesWithSymbols(const ScopeLocal *lm,bool typesyes) +bool Funcdata::syncVarnodesWithSymbols(const ScopeLocal *lm,bool updateDatatypes,bool unmappedAliasCheck) { bool updateoccurred = false; @@ -827,7 +827,7 @@ bool Funcdata::syncVarnodesWithSymbols(const ScopeLocal *lm,bool typesyes) if (entry != (SymbolEntry *)0) { fl = entry->getAllFlags(); if (entry->getSize() >= vnexemplar->getSize()) { - if (typesyes) { + if (updateDatatypes) { ct = entry->getSizedType(vnexemplar->getAddr(), vnexemplar->getSize()); if (ct != (Datatype *)0 && ct->getMetatype() == TYPE_UNKNOWN) ct = (Datatype *)0; @@ -849,6 +849,10 @@ bool Funcdata::syncVarnodesWithSymbols(const ScopeLocal *lm,bool typesyes) // kind of symbol, if we are in scope fl = Varnode::mapped | Varnode::addrtied; } + else if (unmappedAliasCheck) { + // If the varnode is not in scope, check if we should treat as unaliased + fl = lm->isUnmappedUnaliased(vnexemplar) ? Varnode::nolocalalias : 0; + } else fl = 0; } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc index dd3b411738..abeb01fdea 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc @@ -2498,7 +2498,7 @@ void Heritage::heritage(void) else if (prev==2) { // If completely contained in range from previous pass if (vn->isHeritageKnown()) continue; // Don't heritage if we don't have to if (vn->hasNoDescend()) continue; - if ((!needwarning)&&(info->deadremoved>0)) { + if ((!needwarning)&&(info->deadremoved>0)&&!fd->isJumptableRecoveryOn()) { needwarning = true; bumpDeadcodeDelay(vn); warnvn = vn; @@ -2507,7 +2507,7 @@ void Heritage::heritage(void) } else { // Partially contained in old range, but may contain new stuff disjoint.add((*liter).first,(*liter).second.size,pass,prev); - if ((!needwarning)&&(info->deadremoved>0)) { + if ((!needwarning)&&(info->deadremoved>0)&&!fd->isJumptableRecoveryOn()) { // TODO: We should check if this varnode is tiled by previously heritaged ranges if (vn->isHeritageKnown()) continue; // Assume that it is tiled and produced by merging // In most cases, a truly new overlapping read will hit the bumpDeadcodeDelay either here or in prev==2 diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc index b7d80e282c..c3bd621496 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc @@ -275,7 +275,8 @@ ScopeLocal::ScopeLocal(uint8 id,AddrSpace *spc,Funcdata *fd,Architecture *g) : S { space = spc; - deepestParamOffset = ~((uintb)0); + minParamOffset = ~((uintb)0); + maxParamOffset = 0; rangeLocked = false; stackGrowsNegative = true; restrictScope(fd); @@ -344,7 +345,8 @@ void ScopeLocal::resetLocalWindow(void) { stackGrowsNegative = fd->getFuncProto().isStackGrowsNegative(); - deepestParamOffset = stackGrowsNegative ? ~((uintb)0) : 0; + minParamOffset = ~(uintb)0; + maxParamOffset = 0; if (rangeLocked) return; @@ -395,6 +397,22 @@ void ScopeLocal::decodeWrappingAttributes(Decoder &decoder) space = decoder.readSpace(ATTRIB_MAIN); } +/// Currently we treat all unmapped Varnodes as not having an alias, unless the Varnode is on the stack +/// and the location is also used to pass parameters. This should not be called until the second pass, in +/// order to give markNotMapped a chance to be called. +/// Return \b true if the Varnode can be treated as having no aliases. +/// \param vn is the given Varnode +/// \return \b true if there are no aliases +bool ScopeLocal::isUnmappedUnaliased(Varnode *vn) const + +{ + if (vn->getSpace() != space) return false; // Must be in mapped local (stack) space + if (maxParamOffset < minParamOffset) return false; // If no min/max, then we have no know stack parameters + if (vn->getOffset() < minParamOffset || vn->getOffset() > maxParamOffset) + return true; + return false; +} + /// The given range can no longer hold a \e mapped local variable. This indicates the range /// is being used for temporary storage. /// \param spc is the address space holding the given range @@ -412,14 +430,10 @@ void ScopeLocal::markNotMapped(AddrSpace *spc,uintb first,int4 sz,bool parameter else if (last > spc->getHighest()) last = spc->getHighest(); if (parameter) { // Everything above parameter - if (stackGrowsNegative) { - if (first < deepestParamOffset) - deepestParamOffset = first; - } - else { - if (first > deepestParamOffset) - deepestParamOffset = first; - } + if (first < minParamOffset) + minParamOffset = first; + if (last > maxParamOffset) + maxParamOffset = last; } Address addr(space,first); // Remove any symbols under range @@ -463,7 +477,8 @@ string ScopeLocal::buildVariableName(const Address &addr, start = -start; } else { - if (deepestParamOffset + 1 > 1 && stackGrowsNegative == (addr.getOffset() < deepestParamOffset)) { + if ((minParamOffset < maxParamOffset) && + (stackGrowsNegative ? (addr.getOffset() < minParamOffset) : (addr.getOffset() > maxParamOffset))) { s << 'Y'; // Indicate unusual region of stack } } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh index 36de5f6921..5267b0a3a9 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.hh @@ -203,7 +203,8 @@ class ScopeLocal : public ScopeInternal { list nameRecommend; ///< Symbol name recommendations for specific addresses list dynRecommend; ///< Symbol name recommendations for dynamic locations list typeRecommend; ///< Data-types for specific storage locations - uintb deepestParamOffset; ///< Deepest position of a parameter passed (to a called function) on the stack + uintb minParamOffset; ///< Minimum offset of parameter passed (to a called function) on the stack + uintb maxParamOffset; ///< Maximum offset of parameter passed (to a called function) on the stack bool stackGrowsNegative; ///< Marked \b true if the stack is considered to \e grow towards smaller offsets bool rangeLocked; ///< True if the subset of addresses \e mapped to \b this scope has been locked bool adjustFit(RangeHint &a) const; ///< Make the given RangeHint fit in the current Symbol map @@ -226,6 +227,8 @@ public: /// \return \b true is the Varnode can be used as unaffected storage bool isUnaffectedStorage(Varnode *vn) const { return (vn->getSpace() == space); } + bool isUnmappedUnaliased(Varnode *vn) const; ///< Check if a given unmapped Varnode should be treated as unaliased. + void markNotMapped(AddrSpace *spc,uintb first,int4 sz,bool param); ///< Mark a specific address range is not mapped // Routines that are specific to one address space