From a7e7421be3f0186102136f72eaf639a13cf5b6be Mon Sep 17 00:00:00 2001 From: dragonmacher <48328597+dragonmacher@users.noreply.github.com> Date: Fri, 24 Jul 2020 17:12:35 -0400 Subject: [PATCH] GT-3647 - Find Data Types - fixed the data type finding to correctly find types when the decompiler wraps lines --- .../app/decompiler/ClangVariableDecl.java | 15 ++++---- .../finder/AnonymousVariableAccessDR.java | 38 +++++++++++++++---- .../DecompilerDataTypeReferenceFinder.java | 8 +++- .../datatype/finder/DecompilerReference.java | 32 +++++++++++++++- .../datatype/finder/DecompilerVariable.java | 6 +++ .../datatype/finder/VariableAccessDR.java | 10 +++-- 6 files changed, 89 insertions(+), 20 deletions(-) diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangVariableDecl.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangVariableDecl.java index ee41f0c1ad..0fada8ad27 100644 --- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangVariableDecl.java +++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/decompiler/ClangVariableDecl.java @@ -27,6 +27,7 @@ import ghidra.util.Msg; import ghidra.util.xml.SpecXmlUtils; import ghidra.xml.XmlElement; import ghidra.xml.XmlPullParser; + /** * * @@ -45,21 +46,21 @@ public class ClangVariableDecl extends ClangTokenGroup { } public DataType getDataType() { - return datatype; + return datatype; } - + public HighVariable getHighVariable() { return typevar; } - + @Override - public void restoreFromXML(XmlPullParser parser,PcodeFactory pfactory) { - XmlElement node = parser.peek(); - super.restoreFromXML(parser,pfactory); + public void restoreFromXML(XmlPullParser parser, PcodeFactory pfactory) { + XmlElement node = parser.peek(); + super.restoreFromXML(parser, pfactory); long symref = SpecXmlUtils.decodeLong(node.getAttribute(ClangXML.SYMREF)); HighSymbol sym = pfactory.getSymbol(symref); if (sym == null) { - Msg.error(this, "Invalid symbol reference: " + symref); + Msg.error(this, "Invalid symbol reference: " + symref + " in " + Parent()); return; } typevar = sym.getHighVariable(); diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/AnonymousVariableAccessDR.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/AnonymousVariableAccessDR.java index 129bc89662..c7da04ea26 100644 --- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/AnonymousVariableAccessDR.java +++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/AnonymousVariableAccessDR.java @@ -45,7 +45,7 @@ import ghidra.program.model.data.DataType; * being accessed anonymously, since there is no variable of Foo declared * in the current function. */ -public class AnonymousVariableAccessDR extends DecompilerReference { +public class AnonymousVariableAccessDR extends VariableAccessDR { protected AnonymousVariableAccessDR(ClangLine line, ClangFieldToken token) { super(line, token); @@ -54,16 +54,40 @@ public class AnonymousVariableAccessDR extends DecompilerReference { @Override public void accumulateMatches(DataType dt, String fieldName, List results) { + // + // This class is backed by a ClangFieldToken. That class's data type is the composite + // that contains the field being accessed. A variable being accessed has 2 types being + // touched: the aforementioned composite and the type of the field itself. + // + // This can match in one of two cases: + // 1) the client seeks to match a given field inside of the containing composite, or + // 2) the client seeks to match only the type, which means that the field type itself must match + // + ClangFieldToken field = (ClangFieldToken) sourceToken; - DataType fieldDt = field.getDataType(); - if (!isEqual(dt, fieldDt)) { + DataType compositeType = field.getDataType(); + DataType fieldDt = DecompilerReference.getFieldDataType(field); + + boolean matchesComposite = isEqual(dt, compositeType); + boolean matchesField = isEqual(dt, fieldDt); + boolean noMatch = !(matchesComposite || matchesField); + if (noMatch) { return; } - if (field.getText().equals(fieldName)) { - results.add(new DataTypeReference(fieldDt, fieldName, getFunction(), getAddress(), - getContext())); + if (fieldName == null) { + // case 2; no field name to check + if (matchesField) { + results.add(createReference(variable)); + } + return; + } + + // case 1; check the field name and the composite type + if (matchesComposite && field.getText().equals(fieldName)) { + results.add( + new DataTypeReference(compositeType, fieldName, getFunction(), getAddress(), + getContext())); } } - } diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java index f597491948..da8a90c900 100644 --- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java +++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerDataTypeReferenceFinder.java @@ -461,8 +461,14 @@ public class DecompilerDataTypeReferenceFinder implements DataTypeReferenceFinde return false; // should not happen } + // Note: the field's type is that of the parent structure, not the field. We want the + // field's type, so we must retrieve that. + DataType fieldDt = DecompilerReference.getFieldDataType(field); + + // unusual code: getDataType() on the variable may return the type of the field being + // accessed. Contrastingly, getDataType() on the field may return the + // type of the parent structure. DataType variableDt = variable.getDataType(); - DataType fieldDt = field.getDataType(); return !DecompilerReference.isEqual(variableDt, fieldDt); } diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerReference.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerReference.java index d2d511ba1c..076ee9c9c7 100644 --- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerReference.java +++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerReference.java @@ -18,8 +18,7 @@ package ghidra.app.extension.datatype.finder; import java.util.Arrays; import java.util.List; -import ghidra.app.decompiler.ClangLine; -import ghidra.app.decompiler.ClangToken; +import ghidra.app.decompiler.*; import ghidra.app.plugin.core.navigation.locationreferences.ReferenceUtils; import ghidra.app.services.DataTypeReference; import ghidra.program.model.address.Address; @@ -103,12 +102,26 @@ public abstract class DecompilerReference { public static boolean isEquivalent(DataType dt1, DataType dt2) { DataType base1 = getBaseType(dt1); DataType base2 = getBaseType(dt2); + + if (base1 == null || base2 == null) { + // this should not happen, but we have seen sometimes that ClangVariableDecl + // cannot find its HighSymbol from which to get a datatype + return false; + } + return base1.isEquivalent(base2); } public static boolean isEqual(DataType dt1, DataType dt2) { DataType base1 = getBaseType(dt1); DataType base2 = getBaseType(dt2); + + if (base1 == null || base2 == null) { + // this should not happen, but we have seen sometimes that ClangVariableDecl + // cannot find its HighSymbol from which to get a datatype + return false; + } + return base1.equals(base2); } @@ -129,6 +142,21 @@ public abstract class DecompilerReference { return dt; } + public static DataType getFieldDataType(ClangFieldToken field) { + DataType fieldDt = field.getDataType(); + fieldDt = DecompilerReference.getBaseType(fieldDt); + if (fieldDt instanceof Structure) { + Structure parent = (Structure) fieldDt; + int offset = field.getOffset(); + int n = parent.getLength(); + if (offset >= 0 && offset < n) { + DataTypeComponent dtc = parent.getComponentAt(field.getOffset()); + fieldDt = dtc.getDataType(); + } + } + return fieldDt; + } + @Override public String toString() { diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerVariable.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerVariable.java index 576b92fd30..95f0f5464d 100644 --- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerVariable.java +++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/DecompilerVariable.java @@ -51,6 +51,12 @@ public abstract class DecompilerVariable { return ((ClangTypeToken) variable).getDataType(); } +// not sure if we need this; the type returned here is the structure and not the +// field's type +// if (variable instanceof ClangFieldToken) { +// return ((ClangFieldToken) variable).getDataType(); +// } + // Note: this is the icky part of the API. How to know from where to get the data type? HighVariable highVariable = variable.getHighVariable(); if (highVariable != null) { diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/VariableAccessDR.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/VariableAccessDR.java index 8710ace2f7..5f6ab9bbfc 100644 --- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/VariableAccessDR.java +++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/extension/datatype/finder/VariableAccessDR.java @@ -39,6 +39,10 @@ public class VariableAccessDR extends DecompilerReference { super(line, null /* This class does not always have a 'type' token */); } + protected VariableAccessDR(ClangLine line, ClangFieldToken token) { + super(line, token); + } + void setVariable(ClangVariableToken token, List casts) { if (variable != null) { throw new AssertException("Decompiler variable is already set for this access"); @@ -184,7 +188,7 @@ public class VariableAccessDR extends DecompilerReference { return matches; } - private DataTypeReference createReference(DecompilerVariable var) { + protected DataTypeReference createReference(DecompilerVariable var) { DataType dataType = var.getDataType(); String context = getContext(var); @@ -263,11 +267,11 @@ public class VariableAccessDR extends DecompilerReference { //@formatter:off return "{\n" + + "\tline " + getContext() + ",\n" + + "\tfunction: " + getFunction() + "\n" + "\tvariable: " + StringUtilities.toStringWithIndent(variable) + ",\n" + "\tdata type: " + getDataType() + ",\n"+ subFieldsString + - "\tline " + getContext() + ",\n" + - "\tfunction: " + getFunction() + "\n" + "}"; //@formatter:on }