Merge remote-tracking branch 'origin/GP-1277_LostDeindirects' into patch

(Closes #3104)

Author: ghidra1

Ghidra/Features/Decompiler/certification.manifest

@@ -11,12 +11,14 @@ src/decompile/cpp/Doxyfile||GHIDRA|||Most of this file is autogenerated by doxyg
 src/decompile/cpp/Makefile||GHIDRA||||END|
 src/decompile/datatests/convert.xml||GHIDRA||||END|
 src/decompile/datatests/deadvolatile.xml||GHIDRA||||END|
+src/decompile/datatests/deindirect.xml||GHIDRA||||END|
 src/decompile/datatests/floatprint.xml||GHIDRA||||END|
 src/decompile/datatests/forloop1.xml||GHIDRA||||END|
 src/decompile/datatests/forloop_loaditer.xml||GHIDRA||||END|
 src/decompile/datatests/forloop_thruspecial.xml||GHIDRA||||END|
 src/decompile/datatests/forloop_varused.xml||GHIDRA||||END|
 src/decompile/datatests/forloop_withskip.xml||GHIDRA||||END|
+src/decompile/datatests/indproto.xml||GHIDRA||||END|
 src/decompile/datatests/loopcomment.xml||GHIDRA||||END|
 src/decompile/datatests/multiret.xml||GHIDRA||||END|
 src/decompile/datatests/namespace.xml||GHIDRA||||END|

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

@@ -4581,6 +4581,8 @@ void FuncCallSpecs::deindirect(Funcdata &data,Funcdata *newfd)
   if (isOverride())	// If we are overridden at the call-site
     return;		// Don't use the discovered function prototype
 
+  data.getOverride().insertIndirectOverride(op->getAddr(),entryaddress);
+
   // Try our best to merge existing prototype
   // with the one we have just been handed
   vector<Varnode *> newinput;
@@ -4592,7 +4594,6 @@ void FuncCallSpecs::deindirect(Funcdata &data,Funcdata *newfd)
     commitNewOutputs(data,newoutput);
   }
   else {
-    data.getOverride().insertIndirectOverride(op->getAddr(),entryaddress);
     data.setRestartPending(true);
   }
 }
@@ -4613,16 +4614,19 @@ void FuncCallSpecs::forceSet(Funcdata &data,const FuncProto &fp)
 {
   vector<Varnode *> newinput;
   Varnode *newoutput;
+
+  // Copy the recovered prototype into the override manager so that
+  // future restarts don't have to rediscover it
+  FuncProto *newproto = new FuncProto();
+  newproto->copy(fp);
+  data.getOverride().insertProtoOverride(op->getAddr(),newproto);
   if (lateRestriction(fp,newinput,newoutput)) {
     commitNewInputs(data,newinput);
     commitNewOutputs(data,newoutput);
   }
   else {
     // Too late to make restrictions to correct prototype
-    // Add a restart override with the forcing prototype
-    FuncProto *newproto = new FuncProto();
-    newproto->copy(fp);
-    data.getOverride().insertProtoOverride(op->getAddr(),newproto);
+    // Force a restart
     data.setRestartPending(true);
   }
   // Regardless of what happened, lock the prototype so it doesn't happen again

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

@@ -0,0 +1,36 @@
+<decompilertest>
+<binaryimage arch="x86:LE:64:default:gcc">
+  <!--
+      A contrived function with 2 indirect calls that share the same parameter set up.
+      The parameter setup initially gets associated with only one of the two indirect calls.
+      The indirects eventually collapse to direct calls and a prototype is associated with
+      both calls, but too late for one, forcing a restart.  The decompiler should be
+      able to collapse both indirects to direct calls AND associate the correct prototype.
+  -->
+<bytechunk space="ram" offset="0x10071a" readonly="true">
+                    554889e54883
+ec20897dec8975e88955e4488d0598ff
+ffff488945f88b45e48d50058b45e883
+c00389d689c79090909090837dec097f
+14488b45f8ffd0488d3d2c010000e81d
+feffffeb12488b45f8ffd0488d3d1d01
+0000e809feffff90c9c3
+</bytechunk>
+<bytechunk space="ram" offset="0x10088a" readonly="true">
+  4c657373004d6f726500
+</bytechunk>
+<symbol space="ram" offset="0x10071a" name="deindirect"/>
+<symbol space="ram" offset="0x1006ca" name="realfunc"/>
+<symbol space="ram" offset="0x100580" name="puts"/>
+</binaryimage>
+<script>
+  <com>parse line extern void deindirect(int4 a,int4 b,int4 c);</com>
+  <com>parse line extern void realfunc(int4 a,int4 b);</com>
+  <com>lo fu deindirect</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>quit</com>
+</script>
+<stringmatch name="Deindirect #1" min="0" max="0">Exceeded maximum restarts</stringmatch>
+<stringmatch name="Deindirect #2" min="2" max="2">realfunc\(b \+ 3,c \+ 5\)</stringmatch>
+</decompilertest>

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

@@ -0,0 +1,35 @@
+<decompilertest>
+<binaryimage arch="x86:LE:64:default:gcc">
+  <!--
+      A contrived function with 2 indirect calls that share the same parameter set up.
+      The parameter setup initially gets associated with only one of the two indirect calls.
+      A prototype data-type is eventually propagated to both indirect calls
+      but too late for one, forcing a restart.  The decompiler should be able to
+      associate the correct prototype with both indirect calls.
+  -->
+<bytechunk space="ram" offset="0x100771" readonly="true">
+  554889e54883ec10897dfc8975f848
+8955f08b45fc89c79090909090837df8
+647517488b45f0488b00ffd0488d3d53
+010000e838feffffeb16488b45f0488b
+4008ffd0488d3d40010000e820feffff
+90c9c3
+</bytechunk>
+<bytechunk space="ram" offset="0x1008f6" readonly="true">
+  5065656b0047657400
+</bytechunk>
+<symbol space="ram" offset="0x100771" name="indproto"/>
+<symbol space="ram" offset="0x1005e0" name="puts"/>
+</binaryimage>
+<script>
+  <com>parse line struct methods { void (*peek)(int4 a); void (*get)(int4 b); };</com>
+  <com>parse line extern void indproto(int4 a,int4 b,methods *ptr);</com>
+  <com>lo fu indproto</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>quit</com>
+</script>
+<stringmatch name="Indirect prototype #1" min="0" max="0">Exceeded maximum restarts</stringmatch>
+<stringmatch name="Indirect prototype #2" min="1" max="1">ptr-&gt;peek\)\(a\)</stringmatch>
+<stringmatch name="Indirect prototype #3" min="1" max="1">ptr-&gt;get\)\(a\)</stringmatch>
+</decompilertest>