Remove stack placeholders before guarding calls

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

@@ -130,6 +130,45 @@ FlowBlock *PriorityQueue::extract(void)
   return res;
 }
 
+/// Initialize heritage state information for a particular address space
+/// \param spc is the address space
+HeritageInfo::HeritageInfo(AddrSpace *spc)
+
+{
+  if (spc == (AddrSpace *)0) {
+    space = (AddrSpace *)0;
+    delay = 0;
+    deadcodedelay = 0;
+    hasCallPlaceholders = false;
+  }
+  else if (!spc->isHeritaged()) {
+    space = (AddrSpace *)0;
+    delay = spc->getDelay();
+    deadcodedelay = spc->getDeadcodeDelay();
+    hasCallPlaceholders = false;
+  }
+  else {
+    space = spc;
+    delay = spc->getDelay();
+    deadcodedelay = spc->getDeadcodeDelay();
+    hasCallPlaceholders = (spc->getType() == IPTR_SPACEBASE);
+  }
+  deadremoved = 0;
+  warningissued = false;
+  loadGuardSearch = false;
+}
+
+void HeritageInfo::reset(void)
+
+{
+  // Leave any override intact: deadcodedelay = delay;
+  deadremoved = 0;
+  if (space != (AddrSpace *)0)
+    hasCallPlaceholders = (space->getType() == IPTR_SPACEBASE);
+  warningissued = false;
+  loadGuardSearch = false;
+}
+
 /// Instantiate the heritage manager for a particular function.
 /// \param data is the function
 Heritage::Heritage(Funcdata *data)
@@ -1180,16 +1219,10 @@ void Heritage::guardCalls(uint4 flags,const Address &addr,int4 size,vector<Varno
       uintb off = addr.getOffset();
       bool tryregister = true;
       if (spc->getType() == IPTR_SPACEBASE) {
-	if (fc->getStackPlaceholderSlot() < 0) { // Any stack resolution is complete (or never started)
+	if (fc->getSpacebaseOffset() != FuncCallSpecs::offset_unknown)
-	  if (fc->getSpacebaseOffset() != FuncCallSpecs::offset_unknown)
+	  off = spc->wrapOffset(off - fc->getSpacebaseOffset());
-	    off = spc->wrapOffset(off - fc->getSpacebaseOffset());
+	else
-	  else
+	  tryregister = false; // Do not attempt to register this stack loc as a trial
-	    tryregister = false; // Do not attempt to register this stack loc as a trial
-	}
-	else {			// Stack has not been resolved, so we need to abort
-	  fc->abortSpacebaseRelative(*fd);
-	  tryregister = false;
-	}
       }
       Address transAddr(spc,off);	// Address relative to callee's stack
       if (tryregister) {
@@ -1695,6 +1728,19 @@ static void verify_dfs(const vector<FlowBlock *> &list,vector<vector<FlowBlock *
 }
 #endif
 
+/// Assuming we are just about to do heritage on an address space,
+/// clear any placeholder LOADs associated with it on CALLs.
+/// \param info is state for the specific address space
+void Heritage::clearStackPlaceholders(HeritageInfo *info)
+
+{
+  int4 numCalls = fd->numCalls();
+  for(int4 i=0;i<numCalls;++i) {
+    fd->getCallSpecs(i)->abortSpacebaseRelative(*fd);
+  }
+  info->hasCallPlaceholders = false;	// Mark that clear has taken place
+}
+
 /// \brief Perform one level of Varnode splitting to match a JoinRecord
 ///
 /// Split all the pieces in \b lastcombo, putting them into \b nextlev in order,
@@ -2288,16 +2334,9 @@ void Heritage::buildInfoList(void)
 {
   if (!infolist.empty()) return;
   const AddrSpaceManager *manage = fd->getArch();
-  infolist.resize(manage->numSpaces());
+  infolist.reserve(manage->numSpaces());
-  for(int4 i=0;i<manage->numSpaces();++i) {
+  for(int4 i=0;i<manage->numSpaces();++i)
-    AddrSpace *spc = manage->getSpace(i);
+    infolist.emplace_back(manage->getSpace(i));
-    if (spc == (AddrSpace *)0)
-      infolist[i].set((AddrSpace *)0,0,0);
-    else if (!spc->isHeritaged())
-      infolist[i].set((AddrSpace *)0,spc->getDelay(),spc->getDeadcodeDelay());
-    else
-      infolist[i].set(spc,spc->getDelay(),spc->getDeadcodeDelay());
-  }
 }
 
 /// From any address space that is active for this pass, free Varnodes are collected
@@ -2328,6 +2367,9 @@ void Heritage::heritage(void)
     info = &infolist[i];
     if (!info->isHeritaged()) continue;
     if (pass < info->delay) continue; // It is too soon to heritage this space
+    if (info->hasCallPlaceholders)
+      clearStackPlaceholders(info);
+
     if (!info->loadGuardSearch) {
       info->loadGuardSearch = true;
       if (discoverIndexedStackPointers(info->space,freeStores,true)) {

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

@@ -90,11 +90,11 @@ class HeritageInfo {
   int4 deadremoved;		///< >0 if Varnodes in this space have been eliminated
   bool loadGuardSearch;		///< \b true if the search for LOAD ops to guard has been performed
   bool warningissued;		///< \b true if warning issued previously
-  void set(AddrSpace *spc,int4 dl,int4 dcdl) {
+  bool hasCallPlaceholders;	///< \b true for the \e stack space, if stack placeholders have not been removed
-    space=spc; delay=dl; deadcodedelay=dcdl; deadremoved=0; warningissued=false; loadGuardSearch = false; } ///< Set all fields
   bool isHeritaged(void) const { return (space != (AddrSpace *)0); }	///< Return \b true if heritage is performed on this space
-  void reset(void) {
+  void reset(void);		///< Reset the state
-    deadremoved = 0; deadcodedelay = delay; warningissued = false; loadGuardSearch = false; }	///< Reset
+public:
+  HeritageInfo(AddrSpace *spc);	///< Constructor
 };
 
 /// \brief Description of a LOAD operation that needs to be guarded
@@ -222,6 +222,7 @@ class Heritage {
   /// \brief Get the heritage status for the given address space
   const HeritageInfo *getInfo(AddrSpace *spc) const { return &(infolist[spc->getIndex()]); }
 
+  void clearStackPlaceholders(HeritageInfo *info);	///< Clear remaining stack placeholder LOADs on any call
   void splitJoinLevel(vector<Varnode *> &lastcombo,vector<Varnode *> &nextlev,JoinRecord *joinrec);
   void splitJoinRead(Varnode *vn,JoinRecord *joinrec);
   void splitJoinWrite(Varnode *vn,JoinRecord *joinrec);