GP-4226 Detect BRANCHIND used as a RETURN

Ghidra/Features/Decompiler/certification.manifest

@@ -4,7 +4,6 @@
 ##MODULE IP: Modified Nuvola Icons - LGPL 2.1
 ##MODULE IP: Oxygen Icons - LGPL 3.0
 ##MODULE IP: Tango Icons - Public Domain
-.gitignore||GHIDRA||||END|
 Module.manifest||GHIDRA||||END|
 data/decompiler.theme.properties||GHIDRA||||END|
 src/decompile/.cproject||GHIDRA||||END|
@@ -62,6 +61,7 @@ src/decompile/datatests/skipnext2.xml||GHIDRA||||END|
 src/decompile/datatests/stackreturn.xml||GHIDRA||||END|
 src/decompile/datatests/statuscmp.xml||GHIDRA||||END|
 src/decompile/datatests/switchind.xml||GHIDRA||||END|
+src/decompile/datatests/switchreturn.xml||GHIDRA||||END|
 src/decompile/datatests/threedim.xml||GHIDRA||||END|
 src/decompile/datatests/twodim.xml||GHIDRA||||END|
 src/decompile/datatests/union_datatype.xml||GHIDRA||||END|

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

@@ -711,20 +711,26 @@ bool FlowInfo::setupCallindSpecs(PcodeOp *op,FuncCallSpecs *fc)
 }
 
 /// \param op is the BRANCHIND operation to convert
-/// \param failuremode is a code indicating the type of failure when trying to recover the jump table
+/// \param mode indicates the type of failure when trying to recover the jump table
-void FlowInfo::truncateIndirectJump(PcodeOp *op,int4 failuremode)
+void FlowInfo::truncateIndirectJump(PcodeOp *op,JumpTable::RecoveryMode mode)
 
 {
-  data.opSetOpcode(op,CPUI_CALLIND); // Turn jump into call
+  if (mode == JumpTable::fail_return) {
-  setupCallindSpecs(op,(FuncCallSpecs *)0);
+    data.opSetOpcode(op,CPUI_RETURN);	// Turn jump into return
-  if (failuremode != 2)					// Unless the switch was a thunk mechanism
+    data.warning("Treating indirect jump as return",op->getAddr());
-    data.getCallSpecs(op)->setBadJumpTable(true);	// Consider using special name for switch variable
+  }
+  else {
+    data.opSetOpcode(op,CPUI_CALLIND); // Turn jump into call
+    setupCallindSpecs(op,(FuncCallSpecs *)0);
+    if (mode != JumpTable::fail_thunk)			// Unless the switch was a thunk mechanism
+      data.getCallSpecs(op)->setBadJumpTable(true);	// Consider using special name for switch variable
 
-  // Create an artificial return
+    // Create an artificial return
-  PcodeOp *truncop = artificialHalt(op->getAddr(),0);
+    PcodeOp *truncop = artificialHalt(op->getAddr(),0);
-  data.opDeadInsertAfter(truncop,op);
+    data.opDeadInsertAfter(truncop,op);
 
-  data.warning("Treating indirect jump as call",op->getAddr());
+    data.warning("Treating indirect jump as call",op->getAddr());
+  }
 }
 
 /// \brief Test if the given p-code op is a member of an array
@@ -1404,16 +1410,16 @@ void FlowInfo::recoverJumpTables(vector<JumpTable *> &newTables,vector<PcodeOp *
 
   for(int4 i=0;i<tablelist.size();++i) {
     op = tablelist[i];
-    int4 failuremode;
+    JumpTable::RecoveryMode mode;
-    JumpTable *jt = data.recoverJumpTable(partial,op,this,failuremode); // Recover it
+    JumpTable *jt = data.recoverJumpTable(partial,op,this,mode); // Recover it
     if (jt == (JumpTable *)0) { // Could not recover jumptable
-      if ((failuremode == 3) && (tablelist.size() > 1) && (!isInArray(notreached,op))) {
+      if ((mode == JumpTable::fail_noflow) && (tablelist.size() > 1) && (!isInArray(notreached,op))) {
 	// If the indirect op was not reachable with current flow AND there is more flow to generate,
 	//     AND we haven't tried to recover this table before
 	notreached.push_back(op); // Save this op so we can try to recovery table again later
       }
       else if (!isFlowForInline())	// Unless this flow is being inlined for something else
-	truncateIndirectJump(op,failuremode); // Treat the indirect jump as a call
+	truncateIndirectJump(op,mode); // Treat the indirect jump as a call
     }
     newTables.push_back(jt);
   }

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

@@ -137,7 +137,7 @@ private:
   void checkMultistageJumptables(void);
   void recoverJumpTables(vector<JumpTable *> &newTables,vector<PcodeOp *> &notreached);
   void deleteCallSpec(FuncCallSpecs *fc);		///< Remove the given call site from the list for \b this function
-  void truncateIndirectJump(PcodeOp *op,int4 failuremode);  	///< Treat indirect jump as indirect call that never returns
+  void truncateIndirectJump(PcodeOp *op,JumpTable::RecoveryMode mode);  ///< Treat indirect jump as CALLIND/RETURN
   static bool isInArray(vector<PcodeOp *> &array,PcodeOp *op);
 public:
   FlowInfo(Funcdata &d,PcodeOpBank &o,BlockGraph &b,vector<FuncCallSpecs *> &q);	///< Constructor

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

@@ -118,7 +118,7 @@ class Funcdata {
   void pushMultiequals(BlockBasic *bb);		///< Push MULTIEQUAL Varnodes of the given block into the output block
   void clearBlocks(void);			///< Clear all basic blocks
   void structureReset(void);			///< Calculate initial basic block structures (after a control-flow change)
-  int4 stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowInfo *flow);
+  JumpTable::RecoveryMode stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowInfo *flow);
   void switchOverJumpTables(const FlowInfo &flow);	///< Convert jump-table addresses to basic block indices
   void clearJumpTables(void);			///< Clear any jump-table information
 
@@ -423,6 +423,7 @@ public:
   Varnode *findLinkedVarnode(SymbolEntry *entry) const;	///< Find a Varnode matching the given Symbol mapping
   void findLinkedVarnodes(SymbolEntry *entry,vector<Varnode *> &res) const;	///< Find Varnodes that map to the given SymbolEntry
   void buildDynamicSymbol(Varnode *vn);				///< Build a \e dynamic Symbol associated with the given Varnode
+  bool testForReturnAddress(Varnode *vn);	///< Test if the given Varnode is (derived from) the return address
   bool attemptDynamicMapping(SymbolEntry *entry,DynamicHash &dhash);
   bool attemptDynamicMappingLate(SymbolEntry *entry,DynamicHash &dhash);
   Merge &getMerge(void) { return covermerge; }			///< Get the Merge object for \b this function
@@ -519,7 +520,7 @@ public:
   JumpTable *linkJumpTable(PcodeOp *op);		///< Link jump-table with a given BRANCHIND
   JumpTable *findJumpTable(const PcodeOp *op) const;	///< Find a jump-table associated with a given BRANCHIND
   JumpTable *installJumpTable(const Address &addr);	///< Install a new jump-table for the given Address
-  JumpTable *recoverJumpTable(Funcdata &partial,PcodeOp *op,FlowInfo *flow,int4 &failuremode);
+  JumpTable *recoverJumpTable(Funcdata &partial,PcodeOp *op,FlowInfo *flow,JumpTable::RecoveryMode &mode);
   bool earlyJumpTableFail(PcodeOp *op);	///< Try to determine, early, if jump-table analysis will fail
   int4 numJumpTables(void) const { return jumpvec.size(); }	///< Get the number of jump-tables for \b this function
   JumpTable *getJumpTable(int4 i) { return jumpvec[i]; }	///< Get the i-th jump-table

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

@@ -477,18 +477,15 @@ JumpTable *Funcdata::installJumpTable(const Address &addr)
 /// A partial function (copy) is built using the flow info. Simplification is performed on the
 /// partial function (using the "jumptable" strategy), then destination addresses of the
 /// branch are recovered by examining the simplified data-flow. The jump-table object
-/// is populated with the recovered addresses.  An integer value is returned:
+/// is populated with the recovered addresses.  A code indicating success or the type of
-///   - 0 = success
+/// failure is returned.
-///   - 1 = normal could-not-recover failure
-///   - 2 = \b likely \b thunk failure
-///   - 3 = no legal flows to the BRANCHIND failure
 ///
 /// \param partial is a function object for caching analysis
 /// \param jt is the jump-table object to populate
 /// \param op is the BRANCHIND p-code op to analyze
 /// \param flow is the existing flow information
 /// \return the success/failure code
-int4 Funcdata::stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowInfo *flow)
+JumpTable::RecoveryMode Funcdata::stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowInfo *flow)
 
 {
   if (!partial.isJumptableRecoveryOn()) {
@@ -514,7 +511,7 @@ int4 Funcdata::stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowIn
     catch(LowlevelError &err) {
       glb->allacts.setCurrent(oldactname);
       warning(err.explain,op->getAddr());
-      return 1;
+      return JumpTable::fail_normal;
     }
   }
   PcodeOp *partop = partial.findOp(op->getSeqNum());
@@ -522,7 +519,11 @@ int4 Funcdata::stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowIn
   if (partop==(PcodeOp *)0 || partop->code() != CPUI_BRANCHIND || partop->getAddr() != op->getAddr())
     throw LowlevelError("Error recovering jumptable: Bad partial clone");
   if (partop->isDead())	// Indirectop we were trying to recover was eliminated as dead code (unreachable)
-    return 0;			// Return jumptable as
+    return JumpTable::success;			// Return jumptable as
+
+  // Test if the branch target is copied from the return address.
+  if (testForReturnAddress(partop->getIn(0)))
+    return JumpTable::fail_return;		// Return special failure code.  Switch would not recover anyway.
 
   try {
     jt->setLoadCollect(flow->doesJumpRecord());
@@ -533,16 +534,16 @@ int4 Funcdata::stageJumpTable(Funcdata &partial,JumpTable *jt,PcodeOp *op,FlowIn
       jt->recoverAddresses(&partial); // Analyze partial to recover jumptable addresses
   }
   catch(JumptableNotReachableError &err) {	// Thrown by recoverAddresses
-    return 3;
+    return JumpTable::fail_noflow;
   }
   catch(JumptableThunkError &err) {		// Thrown by recoverAddresses
-    return 2;
+    return JumpTable::fail_thunk;
   }
   catch(LowlevelError &err) {
     warning(err.explain,op->getAddr());
-    return 1;
+    return JumpTable::fail_normal;
   }
-  return 0;
+  return JumpTable::success;
 }
 
 /// Backtrack from the BRANCHIND, looking for ops that might affect the destination.
@@ -633,22 +634,22 @@ bool Funcdata::earlyJumpTableFail(PcodeOp *op)
 /// \param partial is the Funcdata copy to perform analysis on if necessary
 /// \param op is the given BRANCHIND PcodeOp
 /// \param flow is current flow information for \b this function
-/// \param failuremode will hold the final success/failure code (0=success)
+/// \param mode will hold the final success/failure code
 /// \return the recovered JumpTable or NULL if there was no success
-JumpTable *Funcdata::recoverJumpTable(Funcdata &partial,PcodeOp *op,FlowInfo *flow,int4 &failuremode)
+JumpTable *Funcdata::recoverJumpTable(Funcdata &partial,PcodeOp *op,FlowInfo *flow,JumpTable::RecoveryMode &mode)
 
 {
   JumpTable *jt;
 
-  failuremode = 0;
+  mode = JumpTable::success;
   jt = linkJumpTable(op);		// Search for pre-existing jumptable
   if (jt != (JumpTable *)0) {
     if (!jt->isOverride()) {
       if (jt->getStage() != 1)
 	return jt;		// Previously calculated jumptable (NOT an override and NOT incomplete)
     }
-    failuremode = stageJumpTable(partial,jt,op,flow); // Recover based on override information
+    mode = stageJumpTable(partial,jt,op,flow); // Recover based on override information
-    if (failuremode != 0)
+    if (mode != JumpTable::success)
       return (JumpTable *)0;
     jt->setIndirectOp(op);	// Relink table back to original op
     return jt;
@@ -659,8 +660,8 @@ JumpTable *Funcdata::recoverJumpTable(Funcdata &partial,PcodeOp *op,FlowInfo *fl
   if (earlyJumpTableFail(op))
     return (JumpTable *)0;
   JumpTable trialjt(glb);
-  failuremode = stageJumpTable(partial,&trialjt,op,flow);
+  mode = stageJumpTable(partial,&trialjt,op,flow);
-  if (failuremode != 0)
+  if (mode != JumpTable::success)
     return (JumpTable *)0;
   //  if (trialjt.is_twostage())
   //    warning("Jumptable maybe incomplete. Second-stage recovery not implemented",trialjt.Opaddress());

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

@@ -1282,6 +1282,40 @@ bool Funcdata::attemptDynamicMappingLate(SymbolEntry *entry,DynamicHash &dhash)
   return true;
 }
 
+/// Follow the Varnode back to see if it comes from the return address for \b this function.
+/// If so, return \b true.  The return address can flow through COPY, INDIRECT, and AND operations.
+/// If there are any other operations in the flow path, or if a standard storage location for the
+/// return address was not defined, return \b false.
+/// \param vn is the given Varnode to trace
+/// \return \b true if flow is from the return address
+bool Funcdata::testForReturnAddress(Varnode *vn)
+
+{
+  VarnodeData &retaddr(glb->defaultReturnAddr);
+  if (retaddr.space == (AddrSpace *)0)
+    return false;			// No standard storage location to compare to
+  while(vn->isWritten()) {
+    PcodeOp *op = vn->getDef();
+    OpCode opc = op->code();
+    if (opc == CPUI_INDIRECT || opc == CPUI_COPY) {
+      vn = op->getIn(0);
+    }
+    else if (opc == CPUI_INT_AND) {
+      // We only want to allow "alignment" style masking
+      if (!op->getIn(1)->isConstant())
+	return false;
+      vn = op->getIn(0);
+    }
+    else
+      return false;
+  }
+  if (vn->getSpace() != retaddr.space || vn->getOffset() != retaddr.offset || vn->getSize() != retaddr.size)
+    return false;
+  if (!vn->isInput())
+    return false;
+  return true;
+}
+
 /// \brief Replace all read references to the first Varnode with a second Varnode
 ///
 /// \param vn is the first Varnode (being replaced)

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

@@ -525,6 +525,16 @@ public:
 /// It knows how to map from specific switch variable values to the destination
 /// \e case block and how to label the value.
 class JumpTable {
+public:
+  /// \brief Recovery status for a specific JumpTable
+  enum RecoveryMode {
+    success = 0,			///< JumpTable is fully recovered
+    fail_normal = 1,		///< Normal failure to recover
+    fail_thunk = 2,		///< Likely \b thunk
+    fail_noflow = 3,		///< No legal flow to BRANCHIND
+    fail_return = 4  		///< Likely \b return operation
+  };
+private:
   /// \brief An address table index and its corresponding out-edge
   struct IndexPair {
     int4 blockPosition;				///< Out-edge index for the basic-block

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

@@ -0,0 +1,26 @@
+<decompilertest>
+<binaryimage arch="MIPS:LE:32:default:default">
+<!--
+    Example of a return instruction occuring through a non-standard register.
+    The decompiler should process initially as a jumptable, but should discover
+    it is actually branching through the return value.  It should convert the
+    indirect jump into a return operation.
+-->
+<bytechunk space="ram" offset="0x10000">
+fa630b620018084000650b9700ef0065
+</bytechunk>
+<symbol space="ram" offset="0x10000" name="switchreturn"/>
+<symbol space="ram" offset="0x10020" name="testcall"/>
+</binaryimage>
+<script>
+  <com>set context ISA_MODE 1 [ram,0x10000] [ram,0x20000]</com>
+  <com>set context RELP 1 [ram,0x10000] [ram,0x20000]</com>
+  <com>lo fu switchreturn</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>quit</com>
+</script>
+<stringmatch name="Switch return #1" min="1" max="1">Treating indirect jump as return</stringmatch>
+<stringmatch name="Switch return #2" min="0" max="0">Could not recover</stringmatch>
+<stringmatch name="Switch return #3" min="0" max="0">\(\*UNRECOVERED_JUMPTABLE\)</stringmatch>
+</decompilertest>