Merge remote-tracking branch 'origin/GP-5073_InliningRecursionBug'

(Closes #5824)

Ghidra/Features/Decompiler/certification.manifest

@@ -38,6 +38,7 @@ src/decompile/datatests/ifswitch.xml||GHIDRA||||END|
 src/decompile/datatests/impliedfield.xml||GHIDRA||||END|
 src/decompile/datatests/indproto.xml||GHIDRA||||END|
 src/decompile/datatests/injectoverride.xml||GHIDRA||||END|
+src/decompile/datatests/inline.xml||GHIDRA||||END|
 src/decompile/datatests/longdouble.xml||GHIDRA||||END|
 src/decompile/datatests/loopcomment.xml||GHIDRA||||END|
 src/decompile/datatests/lzcount.xml||GHIDRA||||END|

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

@@ -1120,12 +1120,6 @@ void FlowInfo::inlineEZClone(const FlowInfo &inlineflow,const Address &calladdr)
 bool FlowInfo::testHardInlineRestrictions(Funcdata *inlinefd,PcodeOp *op,Address &retaddr)
 
 {
-  if (inline_recursion->find( inlinefd->getAddress() ) != inline_recursion->end()) {
-    // This function has already been included with current inlining
-    inline_head->warning("Could not inline here",op->getAddr());
-    return false;
-  }
-  
   if (!inlinefd->getFuncProto().isNoReturn()) {
     list<PcodeOp *>::iterator iter = op->getInsertIter();
     ++iter;
@@ -1142,8 +1136,6 @@ bool FlowInfo::testHardInlineRestrictions(Funcdata *inlinefd,PcodeOp *op,Address
     // If the inlining "jumps back" this starts a new basic block
     data.opMarkStartBasic(nextop);
   }
-
-  inline_recursion->insert(inlinefd->getAddress());
   return true;
 }
 
@@ -1243,11 +1235,31 @@ bool FlowInfo::inlineSubFunction(FuncCallSpecs *fc)
 {
   Funcdata *fd = fc->getFuncdata();
   if (fd == (Funcdata *)0) return false;
-  PcodeOp *op = fc->getOp();
-  Address retaddr;
 
-  if (!data.inlineFlow( fd, *this, op))
+  if (inline_head == (Funcdata *)0) {
+    // This is the top level of inlining
+    inline_head = &data;	// Set up head of inlining
+    inline_recursion = &inline_base;
+  }
+  inline_recursion->insert(data.getAddress()); // Insert current function
+  if (inline_recursion->find( fd->getAddress() ) != inline_recursion->end()) {
+    // This function has already been included with current inlining
+    inline_head->warning("Could not inline here",fc->getOp()->getAddr());
     return false;
+  }
+
+  int4 res = data.inlineFlow( fd, *this, fc->getOp());
+  if (res < 0)
+    return false;
+  else if (res == 0) {	// easy model
+    // Remove inlined function from list so it can be inlined again, even if it also inlines
+    inline_recursion->erase(fd->getAddress());
+  }
+  else if (res == 1) {	// hard model
+    // Add inlined function to recursion list, even if it contains no inlined calls,
+    // to prevent parent from inlining it twice
+    inline_recursion->insert(fd->getAddress());
+  }
 
   // Changing CALL to JUMP may make some original code unreachable
   setPossibleUnreachable();
@@ -1306,17 +1318,6 @@ void FlowInfo::deleteCallSpec(FuncCallSpecs *fc)
 void FlowInfo::injectPcode(void)
 
 {
-  if (inline_head == (Funcdata *)0) {
-    // This is the top level of inlining
-    inline_head = &data;	// Set up head of inlining
-    inline_recursion = &inline_base;
-    inline_recursion->insert(data.getAddress()); // Insert ourselves
-    //    inline_head = (Funcdata *)0;
-  }
-  else {
-    inline_recursion->insert(data.getAddress()); // Insert ourselves
-  }
-
   for(int4 i=0;i<injectlist.size();++i) {
     PcodeOp *op = injectlist[i];
     if (op == (PcodeOp *)0) continue;

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

@@ -192,7 +192,7 @@ public:
 
   void followFlow(const Address &baddr,const Address &eadddr);
   void truncatedFlow(const Funcdata *fd,const FlowInfo *flow);
-  bool inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop);
+  int4 inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop);
   void overrideFlow(const Address &addr,uint4 type);
   void doLiveInject(InjectPayload *payload,const Address &addr,BlockBasic *bl,list<PcodeOp *>::iterator pos);
   

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

@@ -804,8 +804,8 @@ void Funcdata::truncatedFlow(const Funcdata *fd,const FlowInfo *flow)
 /// \param inlinefd is the function to in-line
 /// \param flow is the flow object being injected
 /// \param callop is the site of the injection
-/// \return \b true if the injection was successful
-bool Funcdata::inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop)
+/// \return 0 for a successful inlining with the easy model, 1 for the hard model, -1 if inlining was not successful
+int4 Funcdata::inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop)
 
 {
   inlinefd->getArch()->clearAnalysis(inlinefd);
@@ -821,7 +821,9 @@ bool Funcdata::inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop)
   inlineflow.forwardRecursion(flow);
   inlineflow.generateOps();
 
+  int4 res;
   if (inlineflow.checkEZModel()) {
+    res = 0;
     // With an EZ clone there are no jumptables to clone
     list<PcodeOp *>::const_iterator oiter = obank.endDead();
     --oiter;			// There is at least one op
@@ -843,7 +845,8 @@ bool Funcdata::inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop)
   else {
     Address retaddr;
     if (!flow.testHardInlineRestrictions(inlinefd,callop,retaddr))
-      return false;
+      return -1;
+    res = 1;
     vector<JumpTable *>::const_iterator jiter; // Clone any jumptables from inline piece
     for(jiter=inlinefd->jumpvec.begin();jiter!=inlinefd->jumpvec.end();++jiter) {
       JumpTable *jtclone = new JumpTable(*jiter);
@@ -862,7 +865,7 @@ bool Funcdata::inlineFlow(Funcdata *inlinefd,FlowInfo &flow,PcodeOp *callop)
 
   obank.setUniqId( inlinefd->obank.getUniqId() );
   
-  return true;
+  return res;
 }
 
 /// \brief Find the primary branch operation for an instruction

Ghidra/Features/Decompiler/src/decompile/datatests/inline.xml

@@ -0,0 +1,63 @@
+<decompilertest>
+<binaryimage arch="x86:LE:64:default:gcc">
+<!--
+  Functions that inline other functions
+-->
+<bytechunk space="ram" offset="0x100000" readonly="true">
+8d4732c3e8f7ffffff89c7e8f0ffffff
+c339f77c01c34883ec084889d7e8de0f
+00004883c408c35389fbba88001000be
+0a000000e8d8ffffffba8c001000be64
+00000089dfe8c7ffffff5bc34883ec08
+85f6751389f8c1e81f01c7d1ffe80b00
+00004883c408c38d7c7f01ebf089f883
+ff0174134883ec0889fe83e601e8caff
+ffff4883c408c3c3
+</bytechunk>
+<bytechunk space="ram" offset="0x100088" readonly="true">
+  54454e0048554e4452454400
+</bytechunk>
+<symbol space="ram" offset="0x100000" name="add50"/>
+<symbol space="ram" offset="0x100004" name="add100"/>
+<symbol space="ram" offset="0x100011" name="compare"/>
+<symbol space="ram" offset="0x100027" name="twohard"/>
+<symbol space="ram" offset="0x101000" name="puts"/>
+<symbol space="ram" offset="0x10004c" name="collatz1"/>
+<symbol space="ram" offset="0x10006d" name="collatz"/>
+</binaryimage>
+<script>
+  <com>parse line extern int4 add50(int4 a);</com>
+  <com>parse line extern int4 add100(int4 a);</com>
+  <com>parse line extern void compare(int4 a,int4 b,char *resp);</com>
+  <com>parse line extern void twohard(int4 x);</com>
+  <com>parse line extern void puts(char *msg);</com>
+  <com>parse line extern int4 collatz1(int4 a,int4 b);</com>
+  <com>parse line extern int4 collatz(int4 val);</com>
+  <com>option inline add50</com>
+  <com>option inline compare</com>
+  <com>option inline collatz1</com>
+  <com>option inline collatz</com>
+  <com>lo fu add100</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>lo fu twohard</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>lo fu collatz</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>quit</com>
+</script>
+<stringmatch name="Inlining #1" min="1" max="1">return a \+ 100;</stringmatch>
+<stringmatch name="Inlining #2" min="1" max="1">if \(x &lt; 10\)</stringmatch>
+<stringmatch name="Inlining #3" min="1" max="1">puts\("TEN"\);</stringmatch>
+<stringmatch name="Inlining #4" min="1" max="1">compare\(x,100,"HUNDRED"\);</stringmatch>
+<stringmatch name="Inlining #5" min="1" max="1">if \(\(val &amp; 1U\)</stringmatch>
+<stringmatch name="Inlining #6" min="1" max="1">val = val / 2;</stringmatch>
+<stringmatch name="Inlining #7" min="1" max="1">val = val \* 3 \+ 1;</stringmatch>
+<stringmatch name="Inlining #8" min="1" max="1">= collatz\(val\);</stringmatch>
+<stringmatch name="Inlining #9" min="2" max="2">WARNING: Could not inline here</stringmatch>
+<stringmatch name="Inlining #10" min="1" max="1">WARNING: Inlined function: add50</stringmatch>
+<stringmatch name="Inlining #11" min="1" max="1">WARNING: Inlined function: compare</stringmatch>
+<stringmatch name="Inlining #12" min="1" max="1">WARNING: Inlined function: collatz1</stringmatch>
+</decompilertest>