Merge remote-tracking branch 'origin/GT-3577_AliasBlock'

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

@@ -96,6 +96,7 @@ Architecture::Architecture(void)
   infer_pointers = true;
   funcptr_align = 0;
   flowoptions = 0;
+  alias_block_level = 2;	// Block structs and arrays by default
   defaultfp = (ProtoModel *)0;
   defaultReturnAddr.space = (AddrSpace *)0;
   evalfp_current = (ProtoModel *)0;

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

@@ -130,6 +130,7 @@ public:
   vector<AddrSpace *> inferPtrSpaces;	///< Set of address spaces in which a pointer constant is inferable
   int4 funcptr_align;		///< How many bits of alignment a function ptr has
   uint4 flowoptions;            ///< options passed to flow following engine
+  int4 alias_block_level;	///< Aliases blocked by 0=none, 1=struct, 2=array, 3=all
   vector<Rule *> extra_pool_rules; ///< Extra rules that go in the main pool (cpu specific, experimental)
 
   Database *symboltab;		///< Memory map of global variables and functions

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

@@ -482,7 +482,7 @@ int4 ParamListStandard::characterizeAsParam(const Address &loc,int4 size) const
     const ParamEntry *testEntry = (*iterpair.first).getParamEntry();
     if (testEntry->getMinSize() <= size && testEntry->justifiedContain(loc, size)==0)
       return 1;
-    if (testEntry->containedBy(loc, size))
+    if (testEntry->isExclusion() && testEntry->containedBy(loc, size))
       res = 2;
     ++iterpair.first;
   }
@@ -490,7 +490,7 @@ int4 ParamListStandard::characterizeAsParam(const Address &loc,int4 size) const
     iterpair.second = resolver->find_end(loc.getOffset() + (size-1));
     while(iterpair.first != iterpair.second) {
       const ParamEntry *testEntry = (*iterpair.first).getParamEntry();
-      if (testEntry->containedBy(loc, size)) {
+      if (testEntry->isExclusion() && testEntry->containedBy(loc, size)) {
 	res = 2;
 	break;
       }
@@ -953,9 +953,12 @@ bool ParamListStandard::getBiggestContainedParam(const Address &loc,int4 size,Va
   ParamEntryResolver *resolver = resolverMap[index];
   if (resolver == (ParamEntryResolver *)0)
     return false;
+  Address endLoc = loc + (size-1);
+  if (endLoc.getOffset() < loc.getOffset())
+    return false;	// Assume there is no parameter if we see wrapping
   const ParamEntry *maxEntry = (const ParamEntry *)0;
   ParamEntryResolver::const_iterator iter = resolver->find_begin(loc.getOffset());
-  ParamEntryResolver::const_iterator enditer = resolver->find_end(loc.getOffset() + (size-1));
+  ParamEntryResolver::const_iterator enditer = resolver->find_end(endLoc.getOffset());
   while(iter != enditer) {
     const ParamEntry *testEntry = (*iter).getParamEntry();
     ++iter;
@@ -966,9 +969,9 @@ bool ParamListStandard::getBiggestContainedParam(const Address &loc,int4 size,Va
 	maxEntry = testEntry;
     }
   }
+  if (maxEntry != (const ParamEntry *)0) {
     if (!maxEntry->isExclusion())
       return false;
-  if (maxEntry != (const ParamEntry *)0) {
     res.space = maxEntry->getSpace();
     res.offset = maxEntry->getBase();
     res.size = maxEntry->getSize();

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

@@ -1043,17 +1043,20 @@ void Heritage::guard(const Address &addr,int4 size,vector<Varnode *> &read,vecto
 /// pulls out the potential parameter.
 /// \param fc is the call site potentially taking a parameter
 /// \param addr is the starting address of the range
+/// \param transAddr is the start of the same range from the callee's stack perspective
 /// \param size is the size of the range in bytes
-void Heritage::guardCallOverlappingInput(FuncCallSpecs *fc,const Address &addr,int4 size)
+void Heritage::guardCallOverlappingInput(FuncCallSpecs *fc,const Address &addr,const Address &transAddr,int4 size)
 
 {
   VarnodeData vData;
 
-  if (fc->getBiggestContainedInputParam(addr, size, vData)) {
+  if (fc->getBiggestContainedInputParam(transAddr, size, vData)) {
     ParamActive *active = fc->getActiveInput();
-    Address taddr(vData.space,vData.offset);
+    Address truncAddr(vData.space,vData.offset);
-    if (active->whichTrial(taddr, size) < 0) { // If not already a trial
+    if (active->whichTrial(truncAddr, size) < 0) { // If not already a trial
-      int4 truncateAmount = addr.justifiedContain(size, taddr, vData.size, false);
+      int4 truncateAmount = transAddr.justifiedContain(size, truncAddr, vData.size, false);
+      int4 diff = (int4)(truncAddr.getOffset() - transAddr.getOffset());
+      truncAddr = addr + diff;		// Convert truncated Address to caller's perspective
       PcodeOp *op = fc->getOp();
       PcodeOp *subpieceOp = fd->newOp(2,op->getAddr());
       fd->opSetOpcode(subpieceOp, CPUI_SUBPIECE);
@@ -1061,9 +1064,9 @@ void Heritage::guardCallOverlappingInput(FuncCallSpecs *fc,const Address &addr,i
       wholeVn->setActiveHeritage();
       fd->opSetInput(subpieceOp,wholeVn,0);
       fd->opSetInput(subpieceOp,fd->newConstant(4,truncateAmount),1);
-      Varnode *vn = fd->newVarnodeOut(vData.size, taddr, subpieceOp);
+      Varnode *vn = fd->newVarnodeOut(vData.size, truncAddr, subpieceOp);
       fd->opInsertBefore(subpieceOp,op);
-      active->registerTrial(taddr, vData.size);
+      active->registerTrial(truncAddr, vData.size);
       fd->opInsertInput(op, vn, op->numInput());
     }
   }
@@ -1121,21 +1124,21 @@ void Heritage::guardCalls(uint4 flags,const Address &addr,int4 size,vector<Varno
 	  tryregister = false;
 	}
       }
-      Address taddr(spc,off);
+      Address transAddr(spc,off);	// Address relative to callee's stack
       if (tryregister) {
-	int4 inputCharacter = fc->characterizeAsInputParam(taddr,size);
+	int4 inputCharacter = fc->characterizeAsInputParam(transAddr,size);
 	if (inputCharacter == 1) {		// Call could be using this range as an input parameter
 	  ParamActive *active = fc->getActiveInput();
-	  if (active->whichTrial(taddr,size)<0) { // If not already a trial
+	  if (active->whichTrial(transAddr,size)<0) { // If not already a trial
 	    PcodeOp *op = fc->getOp();
-	    active->registerTrial(taddr,size);
+	    active->registerTrial(transAddr,size);
 	    Varnode *vn = fd->newVarnode(size,addr);
 	    vn->setActiveHeritage();
 	    fd->opInsertInput(op,vn,op->numInput());
 	  }
 	}
 	else if (inputCharacter == 2)		// Call may be using part of this range as an input parameter
-	  guardCallOverlappingInput(fc, addr, size);
+	  guardCallOverlappingInput(fc, addr, transAddr, size);
       }
     }
     // We do not guard the call if the effect is "unaffected" or "reload"

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

@@ -246,7 +246,7 @@ class Heritage {
   void reprocessFreeStores(AddrSpace *spc,vector<PcodeOp *> &freeStores);
   void guard(const Address &addr,int4 size,vector<Varnode *> &read,vector<Varnode *> &write,vector<Varnode *> &inputvars);
   void guardInput(const Address &addr,int4 size,vector<Varnode *> &input);
-  void guardCallOverlappingInput(FuncCallSpecs *fc,const Address &addr,int4 size);
+  void guardCallOverlappingInput(FuncCallSpecs *fc,const Address &addr,const Address &transAddr,int4 size);
   void guardCalls(uint4 flags,const Address &addr,int4 size,vector<Varnode *> &write);
   void guardStores(const Address &addr,int4 size,vector<Varnode *> &write);
   void guardLoads(uint4 flags,const Address &addr,int4 size,vector<Varnode *> &write);

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

@@ -79,6 +79,7 @@ OptionDatabase::OptionDatabase(Architecture *g)
   registerOption(new OptionSetLanguage());
   registerOption(new OptionJumpLoad());
   registerOption(new OptionToggleRule());
+  registerOption(new OptionAliasBlock());
 }
 
 OptionDatabase::~OptionDatabase(void)
@@ -784,3 +785,34 @@ string OptionToggleRule::apply(Architecture *glb,const string &p1,const string &
   }
   return res;
 }
+
+/// \class OptionAliasBlock
+/// \brief Set how locked data-types on the stack affect alias heuristics
+///
+/// Stack analysis uses the following simple heuristic: a pointer is unlikely to reference (alias)
+/// a stack location if there is a locked data-type between the pointer base and the location.
+/// This option determines what kind of locked data-types \b block aliases in this way.
+///   - none - no data-types will block an alias
+///   - struct - only structure data-types will block an alias
+///   - array - array data-types (and structure data-types) will block an alias
+///   - all - all locked data-types will block an alias
+string OptionAliasBlock::apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const
+
+{
+  if (p1.size() == 0)
+    throw ParseError("Must specify alias block level");
+  int4 oldVal = glb->alias_block_level;
+  if (p1 == "none")
+    glb->alias_block_level = 0;
+  else if (p1 == "struct")
+    glb->alias_block_level = 1;
+  else if (p1 == "array")
+    glb->alias_block_level = 2;		// The default. Let structs and arrays block aliases
+  else if (p1 == "all")
+    glb->alias_block_level = 3;
+  else
+    throw ParseError("Unknown alias block level: "+p1);
+  if (oldVal == glb->alias_block_level)
+    return "Alias block level unchanged";
+  return "Alias block level set to " + p1;
+}

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

@@ -258,4 +258,10 @@ public:
   virtual string apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const;
 };
 
+class OptionAliasBlock : public ArchOption {
+public:
+  OptionAliasBlock(void) { name = "aliasblock"; }	///< Constructor
+  virtual string apply(Architecture *glb,const string &p1,const string &p2,const string &p3) const;
+};
+
 #endif

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

@@ -1084,6 +1084,7 @@ void ScopeLocal::markUnaliased(const vector<uintb> &alias)
   if (rangemap == (EntryMap *)0) return;
   list<SymbolEntry>::iterator iter,enditer;
 
+  int4 alias_block_level = glb->alias_block_level;
   bool aliason = false;
   uintb curalias=0;
   int4 i=0;
@@ -1107,8 +1108,17 @@ void ScopeLocal::markUnaliased(const vector<uintb> &alias)
 	aliason = false;
       if (!aliason)
 	symbol->getScope()->setAttribute(symbol,Varnode::nolocalalias);
-      if (symbol->isTypeLocked())
+      if (symbol->isTypeLocked() && alias_block_level != 0) {
-	aliason = false;
+	if (alias_block_level == 3)
+	  aliason = false;		// For this level, all locked data-types block aliases
+	else {
+	  type_metatype meta = symbol->getType()->getMetatype();
+	  if (meta == TYPE_STRUCT)
+	    aliason = false;		// Only structures block aliases
+	  else if (meta == TYPE_ARRAY && alias_block_level > 1)
+	    aliason = false;		// Only arrays (and structures) block aliases
+	}
+      }
     }
   }
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/DecompileOptions.java

@@ -100,6 +100,38 @@ public class DecompileOptions {
 	private final static boolean INPLACEOP_OPTIONDEFAULT = false;
 	private boolean inplaceTokens;
 
+	private final static String ALIASBLOCK_OPTIONSTRING = "Analysis.Alias Blocking";
+	private final static String ALIASBLOCK_OPTIONDESCRIPTION =
+		"Specify which data-types prevent a pointer alias from reaching across them on the stack.";
+
+	public enum AliasBlockEnum {
+
+		None("none", "None"),
+		Struct("struct", "Structures"),
+		Array("array", "Arrays and Structures"),
+		All("all", "All Data-types");
+
+		private String label;
+		private String optionString;
+
+		private AliasBlockEnum(String optString, String label) {
+			this.label = label;
+			this.optionString = optString;
+		}
+
+		public String getOptionString() {
+			return optionString;
+		}
+
+		@Override
+		public String toString() {
+			return label;
+		}
+	}
+
+	private final static AliasBlockEnum ALIASBLOCK_OPTIONDEFAULT = AliasBlockEnum.Array;
+	private AliasBlockEnum aliasBlock;
+
 	private final static String CONVENTION_OPTIONSTRING = "Display.Print calling convention name";
 	private final static String CONVENTION_OPTIONDESCRIPTION =
 		"If set, the names of callling conventions (which differ " +
@@ -204,12 +236,20 @@ public class DecompileOptions {
 
 	public enum IntegerFormatEnum {
 
-		Hexadecimal("Force Hexadecimal"), Decimal("Force Decimal"), BestFit("Best Fit");
+		Hexadecimal("hex", "Force Hexadecimal"),
+		Decimal("dec", "Force Decimal"),
+		BestFit("best", "Best Fit");
 
 		private String label;
+		private String optionString;
 
-		private IntegerFormatEnum(String label) {
+		private IntegerFormatEnum(String optString, String label) {
 			this.label = label;
+			this.optionString = optString;
+		}
+
+		public String getOptionString() {
+			return optionString;
 		}
 
 		@Override
@@ -301,6 +341,7 @@ public class DecompileOptions {
 		inferconstptr = INFERCONSTPTR_OPTIONDEFAULT;
 		nullToken = NULLTOKEN_OPTIONDEFAULT;
 		inplaceTokens = INPLACEOP_OPTIONDEFAULT;
+		aliasBlock = ALIASBLOCK_OPTIONDEFAULT;
 		conventionPrint = CONVENTION_OPTIONDEFAULT;
 		noCastPrint = NOCAST_OPTIONDEFAULT;
 		maxwidth = MAXWIDTH_OPTIONDEFAULT;
@@ -360,6 +401,7 @@ public class DecompileOptions {
 		inferconstptr = opt.getBoolean(INFERCONSTPTR_OPTIONSTRING, INFERCONSTPTR_OPTIONDEFAULT);
 		nullToken = opt.getBoolean(NULLTOKEN_OPTIONSTRING, NULLTOKEN_OPTIONDEFAULT);
 		inplaceTokens = opt.getBoolean(INPLACEOP_OPTIONSTRING, INPLACEOP_OPTIONDEFAULT);
+		aliasBlock = opt.getEnum(ALIASBLOCK_OPTIONSTRING, ALIASBLOCK_OPTIONDEFAULT);
 		conventionPrint = opt.getBoolean(CONVENTION_OPTIONSTRING, CONVENTION_OPTIONDEFAULT);
 		noCastPrint = opt.getBoolean(NOCAST_OPTIONSTRING, NOCAST_OPTIONDEFAULT);
 		maxwidth = opt.getInt(MAXWIDTH_OPTIONSTRING, MAXWIDTH_OPTIONDEFAULT);
@@ -473,6 +515,8 @@ public class DecompileOptions {
 			NULLTOKEN_OPTIONDESCRIPTION);
 		opt.registerOption(INPLACEOP_OPTIONSTRING, INPLACEOP_OPTIONDEFAULT, help,
 			INPLACEOP_OPTIONDESCRIPTION);
+		opt.registerOption(ALIASBLOCK_OPTIONSTRING, ALIASBLOCK_OPTIONDEFAULT, help,
+			ALIASBLOCK_OPTIONDESCRIPTION);
 		opt.registerOption(CONVENTION_OPTIONSTRING, CONVENTION_OPTIONDEFAULT, help,
 			CONVENTION_OPTIONDESCRIPTION);
 		opt.registerOption(NOCAST_OPTIONSTRING, NOCAST_OPTIONDEFAULT, help,
@@ -589,6 +633,7 @@ public class DecompileOptions {
 		appendOption(buf, "inferconstptr", inferconstptr ? "on" : "off", "", "");
 		appendOption(buf, "nullprinting", nullToken ? "on" : "off", "", "");
 		appendOption(buf, "inplaceops", inplaceTokens ? "on" : "off", "", "");
+		appendOption(buf, "aliasblock", aliasBlock.getOptionString(), "", "");
 		appendOption(buf, "conventionprinting", conventionPrint ? "on" : "off", "", "");
 		appendOption(buf, "nocastprinting", noCastPrint ? "on" : "off", "", "");
 		appendOption(buf, "maxlinewidth", Integer.toString(maxwidth), "", "");
@@ -605,14 +650,7 @@ public class DecompileOptions {
 		appendOption(buf, "commentheader", "header", commentHeadInclude ? "on" : "off", "");
 		appendOption(buf, "commentheader", "warningheader", commentWARNInclude ? "on" : "off", "");
 
-		String curformat = "best";
+		appendOption(buf, "integerformat", integerFormat.getOptionString(), "", "");
-		if (IntegerFormatEnum.Hexadecimal.equals(integerFormat)) {
-			curformat = "hex";
-		}
-		else if (IntegerFormatEnum.Decimal.equals(integerFormat)) {
-			curformat = "dec";
-		}
-		appendOption(buf, "integerformat", curformat, "", "");
 
 		appendOption(buf, "setlanguage", displayLanguage.toString(), "", "");
 

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/CommitLocalsAction.java

@@ -26,9 +26,9 @@ public class CommitLocalsAction extends AbstractDecompilerAction {
 
 	public CommitLocalsAction() {
 		super("Commit Locals");
-		setPopupMenuData(new MenuData(new String[] { "Commit Locals" }, "Commit"));
+		setPopupMenuData(new MenuData(new String[] { "Commit Local Names" }, "Commit"));
 		setDescription(
-			"Save Local variable definitions to Program, locking them into their current type definitions");
+			"Save Local variable names from Decompiler window to Program");
 	}
 
 	@Override
@@ -42,10 +42,10 @@ public class CommitLocalsAction extends AbstractDecompilerAction {
 	@Override
 	protected void decompilerActionPerformed(DecompilerActionContext context) {
 		Program program = context.getProgram();
-		int transaction = program.startTransaction("Commit Params/Return");
+		int transaction = program.startTransaction("Commit Local Names");
 		try {
 			HighFunction hfunc = context.getHighFunction();
-			HighFunctionDBUtil.commitLocalsToDatabase(hfunc, SourceType.USER_DEFINED);
+			HighFunctionDBUtil.commitLocalNamesToDatabase(hfunc, SourceType.USER_DEFINED);
 		}
 		finally {
 			program.endTransaction(transaction, true);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunctionDBUtil.java

@@ -202,7 +202,7 @@ public class HighFunctionDBUtil {
 	 * @param highFunction is the decompiler's model of the function
 	 * @param source is the desired SourceType for the commit
 	 */
-	public static void commitLocalsToDatabase(HighFunction highFunction, SourceType source) {
+	public static void commitLocalNamesToDatabase(HighFunction highFunction, SourceType source) {
 
 		Function function = highFunction.getFunction();
 
@@ -216,19 +216,7 @@ public class HighFunctionDBUtil {
 			}
 			String name = sym.getName();
 			try {
-				Variable var =
+				HighFunctionDBUtil.updateDBVariable(sym, null, null, source);
-					clearConflictingLocalVariables(function, sym.getStorage(), sym.getPCAddress());
-				if (var == null) {
-					var = createLocalVariable(function, sym.getDataType(), sym.getStorage(),
-						sym.getPCAddress(), source);
-					if (name != null) {
-						var.setName(name, source);
-					}
-				}
-				else {
-					var.setDataType(sym.getDataType(), sym.getStorage(), false, source);
-					var.setName(name, source);
-				}
 			}
 			catch (UsrException e) {
 				Msg.error(HighFunctionDBUtil.class, "Local variable commit failed for " +