2392-Updated-ruleBlockIfNoExit-to-better-select-bran.patch

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: cmorin6 <cedric.morin6@gmail.com>
Date: Tue, 20 Oct 2020 20:30:39 +0200
Subject: [PATCH] 2392: Updated ruleBlockIfNoExit to better select branch if
 both are noexit.

---
 .../Decompiler/src/decompile/cpp/block.cc     | 46 ++++++++++++++++
 .../Decompiler/src/decompile/cpp/block.hh     |  7 +++
 .../src/decompile/cpp/blockaction.cc          | 54 ++++++++++++++++---
 .../src/decompile/cpp/blockaction.hh          |  1 +
 .../Decompiler/src/decompile/cpp/op.hh        |  2 +
 5 files changed, 104 insertions(+), 6 deletions(-)

diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc
index f536f78e12..a40e6c3548 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc
@@ -1343,6 +1343,20 @@ void BlockGraph::finalizePrinting(Funcdata &data) const
     (*iter)->finalizePrinting(data);
 }
 
+int4 BlockGraph::getInnerBlockDepth(void)
+
+{
+  int4 depth;
+  int4 maxDepth = 0;
+  for(int4 i=0;i<list.size();++i){
+    depth = list[i]->getBlockDepth();
+    if(depth>maxDepth){
+	maxDepth=depth;
+    }
+  }
+  return maxDepth;
+}
+
 void BlockGraph::encodeBody(Encoder &encoder) const
 
 {
@@ -2591,6 +2605,12 @@ bool BlockBasic::isDoNothing(void) const
   return hasOnlyMarkers();
 }
 
+int4 BlockBasic::getOpSize(void)
+
+{
+  return op.size();
+}
+
 /// In terms of machine instructions, a basic block always covers a range of addresses,
 /// from its first instruction to its last. This method establishes that range.
 /// \param beg is the address of the first instruction in the block
@@ -2935,6 +2955,13 @@ FlowBlock *BlockList::getSplitPoint(void)
   return getBlock(getSize()-1)->getSplitPoint();
 }
 
+int4 BlockList::getBlockDepth(void)
+
+{
+  // list join block together but don't increase block depth
+  return getInnerBlockDepth();
+}
+
 void BlockList::printHeader(ostream &s) const
 
 {
@@ -3019,6 +3046,13 @@ void BlockCondition::encodeHeader(Encoder &encoder) const
   encoder.writeString(ATTRIB_OPCODE, nm);
 }
 
+int4 BlockCondition::getBlockDepth(void)
+
+{
+  // conditions join block together but don't increase block depth
+  return getInnerBlockDepth();
+}
+
 void BlockIf::markUnstructured(void)
 
 {
@@ -3615,6 +3649,18 @@ FlowBlock *BlockSwitch::nextFlowAfter(const FlowBlock *bl) const
   return getParent()->nextFlowAfter(this);
 }
 
+int4 BlockSwitch::getBlockDepth(void){
+  int4 i;
+  int4 maxDepth=0;
+  for(i=0;i<caseblocks.size();++i){
+      int4 depth=caseblocks[i].block->getBlockDepth();
+      if(depth>maxDepth){
+	  maxDepth=depth;
+      }
+  }
+  return maxDepth+2; // +1 for switch block and +1 for case/default block
+}
+
 /// \param bt is the block_type
 /// \return a new instance of the specialized FlowBlock
 FlowBlock *BlockMap::resolveBlock(FlowBlock::block_type bt)
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh
index 1b3146ed2b..3dd8bdc5ee 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh
@@ -253,6 +253,7 @@ public:
   /// \param data is the function to finalize
   virtual void finalizePrinting(Funcdata &data) const {}
 
+  virtual int4 getBlockDepth(void) {return 0;}		///< Return the depth in code block of \b this
   virtual void encodeHeader(Encoder &encoder) const;	///< Encode basic information as attributes
   virtual void decodeHeader(Decoder &decoder);		///< Decode basic information from element attributes
 
@@ -385,6 +386,8 @@ public:
   virtual FlowBlock *nextFlowAfter(const FlowBlock *bl) const;
   virtual void finalTransform(Funcdata &data);
   virtual void finalizePrinting(Funcdata &data) const;
+  virtual int4 getInnerBlockDepth();					///< Return max depth of child blocks
+  virtual int4 getBlockDepth() {return getInnerBlockDepth()+1;}
   virtual void encodeBody(Encoder &encoder) const;
   virtual void decodeBody(Decoder &decoder);
   void decode(Decoder &decoder);				///< Decode \b this BlockGraph from a stream
@@ -490,6 +493,7 @@ public:
   list<PcodeOp *>::const_iterator beginOp(void) const { return op.begin(); }	///< Return an iterator to the beginning of the PcodeOps
   list<PcodeOp *>::const_iterator endOp(void) const { return op.end(); }	///< Return an iterator to the end of the PcodeOps
   bool emptyOp(void) const { return op.empty(); }		///< Return \b true if \b block contains no operations
+  int4 getOpSize(void);					///< Number of PcodeOps contained in \b this block
   bool noInterveningStatement(void) const;
   PcodeOp *findMultiequal(const vector<Varnode *> &varArray);		///< Find MULTIEQUAL with given inputs
   PcodeOp *earliestUse(Varnode *vn);
@@ -594,6 +598,7 @@ public:
   virtual PcodeOp *lastOp(void) const;
   virtual bool negateCondition(bool toporbottom);
   virtual FlowBlock *getSplitPoint(void);
+  virtual int4 getBlockDepth(void);
 };
 
 /// \brief Two conditional blocks combined into one conditional using BOOL_AND or BOOL_OR
@@ -623,6 +628,7 @@ public:
   virtual bool isComplex(void) const { return getBlock(0)->isComplex(); }
   virtual FlowBlock *nextFlowAfter(const FlowBlock *bl) const;
   virtual void encodeHeader(Encoder &encoder) const;
+  virtual int4 getBlockDepth(void);
 };
 
 /// \brief A basic "if" block
@@ -785,6 +791,7 @@ public:
   virtual void emit(PrintLanguage *lng) const { lng->emitBlockSwitch(this); }
   virtual FlowBlock *nextFlowAfter(const FlowBlock *bl) const;
   virtual void finalizePrinting(Funcdata &data) const;
+  virtual int4 getBlockDepth(void);
 };
 
 /// \brief Helper class for resolving cross-references while deserializing BlockGraph objects
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.cc
index 56fcd00b48..f8cb2c670a 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.cc
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.cc
@@ -1483,6 +1483,7 @@ bool CollapseStructure::ruleBlockIfNoExit(FlowBlock *bl)
 {
   FlowBlock *clauseblock;
   int4 i;
+  int4 bestIndex=-1;
 
   if (bl->sizeOut() != 2) return false; // Must be binary condition
   if (bl->isSwitchOut()) return false;
@@ -1500,15 +1501,56 @@ bool CollapseStructure::ruleBlockIfNoExit(FlowBlock *bl)
     //      bl->setGotoBranch(i);
     //      return true;
     //    }
+    if (bestIndex==-1){
+	bestIndex=i;
+    }else{ // both match
+	bestIndex = selectBestNoExit(bl->getOut(0),bl->getOut(1));
+    }
+  }
+  if(bestIndex==-1) return false; // no match
+  clauseblock = bl->getOut(bestIndex);
+  if (bestIndex==0) {			// clause must be true out of bl
+    if (bl->negateCondition(true))
+      dataflow_changecount += 1;
+  }
+  graph.newBlockIf(bl,clauseblock);
+  return true;
+}
 
-    if (i==0) {			// clause must be true out of bl
-      if (bl->negateCondition(true))
-	dataflow_changecount += 1;
+/// Select the best of two NoExit branch to be collapsed by ruleBlockIfNoExit.
+/// \param clause0 is the first NoExit branch
+/// \param clause1 is the second NoExit branch
+/// \return the index of the selected branch (0 or 1)
+int4 CollapseStructure::selectBestNoExit(FlowBlock *clause0,FlowBlock *clause1)
+
+{
+  // select lowest block depth
+  int4 depth0 = clause0->getBlockDepth();
+  int4 depth1 = clause1->getBlockDepth();
+  if (depth0<depth1)return 0;
+  if (depth1<depth0)return 1;
+
+  // same depth, prefer non return
+  bool isRet0 = clause0->lastOp()!=(PcodeOp *)0 && clause0->lastOp()->isStandardReturn();
+  bool isRet1 = clause1->lastOp()!=(PcodeOp *)0 && clause1->lastOp()->isStandardReturn();
+  if(isRet0 && !isRet1) return 1;
+  if(isRet1 && !isRet0) return 0;
+
+  // prefer block containing only return op
+  if(isRet0){ // both are return
+    FlowBlock* fb;
+    if(clause0->getType()==FlowBlock::t_copy){
+      fb = ((BlockCopy*)clause0)->subBlock(0);
+      if(fb->getType()==FlowBlock::t_basic && ((BlockBasic*)fb)->getOpSize()==1) return 0;
+    }
+    if(clause1->getType()==FlowBlock::t_copy){
+      fb = ((BlockCopy*)clause1)->subBlock(0);
+      if(fb->getType()==FlowBlock::t_basic && ((BlockBasic*)fb)->getOpSize()==1) return 1;
     }
-    graph.newBlockIf(bl,clauseblock);
-    return true;
   }
-  return false;
+
+  // fall back to previous behavior
+  return 0;
 }
 
 /// Try to find a while/do structure, starting with a given FlowBlock.
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.hh
index 4cef5f4498..c062e5b211 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.hh
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/blockaction.hh
@@ -219,6 +219,7 @@ class CollapseStructure {
   bool ruleCaseFallthru(FlowBlock *bl);		///< Attempt to one switch case falling through to another
   int4 collapseInternal(FlowBlock *targetbl);	///< The main collapsing loop
   void collapseConditions(void);		///< Simplify conditionals
+  int4 selectBestNoExit(FlowBlock *clause0,FlowBlock *clause1);
 public:
   CollapseStructure(BlockGraph &g);		///< Construct given a control-flow graph
   int4 getChangeCount(void) const { return dataflow_changecount; }	///< Get number of data-flow changes
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/op.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/op.hh
index a2d5c984ee..f7d5c3822e 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/op.hh
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/op.hh
@@ -187,6 +187,8 @@ public:
   bool isCallOrBranch(void) const { return ((flags&(PcodeOp::branch|PcodeOp::call))!=0); }
   /// \brief Return \b true if this op breaks fall-thru flow
   bool isFlowBreak(void) const { return ((flags&(PcodeOp::branch|PcodeOp::returns))!=0); }
+  /// \brief Return \b true if this op will be decompiled as "return"
+  bool isStandardReturn(void) const { return ((flags&PcodeOp::returns)!=0 && getHaltType()==0); }
   /// \brief Return \b true if this op flips the true/false meaning of its control-flow branching
   bool isBooleanFlip(void) const { return ((flags&PcodeOp::boolean_flip)!=0); }
   /// \brief Return \b true if the fall-thru branch is taken when the boolean input is true
-- 
2.45.1