From 9f5cfa5170917a77cde40488cebbe17a62571024 Mon Sep 17 00:00:00 2001 From: ghizard <50744617+ghizard@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:28:53 -0400 Subject: [PATCH] GP-3649 - DemangledObjects - separate lref/rref from pointerLeverls; fix MDMang function pointers indirection --- ...ctDemangledFunctionDefinitionDataType.java | 30 +++++++++- .../app/util/demangler/DemangledDataType.java | 47 ++++++++++++---- .../demangler/gnu/GnuDemanglerParser.java | 26 ++++----- .../demangler/GnuDemanglerParserTest.java | 56 +++++++++++-------- .../MicrosoftDemanglerAnalyzerTest.java | 24 ++++++++ .../main/java/mdemangler/MDMangGhidra.java | 33 ++++++----- 6 files changed, 153 insertions(+), 63 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/AbstractDemangledFunctionDefinitionDataType.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/AbstractDemangledFunctionDefinitionDataType.java index 3a099ce7c6..79d2127583 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/AbstractDemangledFunctionDefinitionDataType.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/AbstractDemangledFunctionDefinitionDataType.java @@ -227,13 +227,21 @@ public abstract class AbstractDemangledFunctionDefinitionDataType extends Demang StringBuilder typeBuffer = new StringBuilder(); int pointerLevels = getPointerLevels(); - if (pointerLevels > 0) { + if (pointerLevels > 0 || isReference() || isRValueReference()) { addParentName(typeBuffer); for (int i = 0; i < pointerLevels; ++i) { typeBuffer.append(getTypeString()); } + // kludge for now... current type-emitting mechanism in DemangledObject hierarchy + // lacks a lot... needs revamped. + if (isLValueReference()) { + typeBuffer.append(" &"); + } + else if (isRValueReference()) { + typeBuffer.append(" &&"); + } } if (!StringUtils.isBlank(typeBuffer)) { @@ -331,7 +339,25 @@ public abstract class AbstractDemangledFunctionDefinitionDataType extends Demang dt = fddt; } - return new PointerDataType(dt, dataTypeManager); + // This was also totally wonked in terms of type-emitting. Whole mangled type needs + // gone through and revamped; not sure it is good to have pointer levels or reference + // information in a FunctionPointer... need to investigate more. + int numPointers = getPointerLevels(); + + for (int i = 0; i < numPointers; ++i) { + dt = PointerDataType.getPointer(dt, dataTypeManager); + } + + if (isLValueReference()) { + // Placeholder in prep for more lref work + dt = PointerDataType.getPointer(dt, dataTypeManager); + } + else if (isRValueReference()) { + // Placeholder in prep for more rref work + dt = PointerDataType.getPointer(dt, dataTypeManager); + } + + return dt; } private void setParameters(FunctionDefinitionDataType fddt, DataTypeManager dataTypeManager) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/DemangledDataType.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/DemangledDataType.java index 4be03fb360..cc8d613277 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/DemangledDataType.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/demangler/DemangledDataType.java @@ -47,6 +47,7 @@ public class DemangledDataType extends DemangledType { public static final String ARR_NOTATION = "[]"; public static final String REF_NOTATION = "&"; + public static final String RIGHT_REF_NOTATION = "&&"; public static final String PTR_NOTATION = "*"; public static final String VOLATILE = "volatile"; @@ -100,7 +101,10 @@ public class DemangledDataType extends DemangledType { private boolean isComplex; private boolean isEnum; private boolean isPointer64; - private boolean isReference; + // Cannot be both lref and rref. Prior to C++11, we only had reference (& operator). + // This is now distinguished as left-value reference (l-value reference or lref), and there + // is now an additional right-value reference (r-value reference or rref) with the && operator. + private boolean isLValueReference; private boolean isRValueReference; private boolean isSigned; private boolean isStruct; @@ -208,13 +212,20 @@ public class DemangledDataType extends DemangledType { } int numPointers = getPointerLevels(); - if (isReference()) { - numPointers++; - } for (int i = 0; i < numPointers; ++i) { dt = PointerDataType.getPointer(dt, dataTypeManager); } + + if (isLValueReference()) { + // Placeholder in prep for more lref work + dt = PointerDataType.getPointer(dt, dataTypeManager); + } + else if (isRValueReference()) { + // Placeholder in prep for more rref work + dt = PointerDataType.getPointer(dt, dataTypeManager); + } + return dt; } @@ -463,7 +474,13 @@ public class DemangledDataType extends DemangledType { } public void setReference() { - isReference = true; + setLValueReference(); + } + + public void setLValueReference() { + isLValueReference = true; + // Cannot be both + isRValueReference = false; } /** @@ -471,6 +488,8 @@ public class DemangledDataType extends DemangledType { */ public void setRValueReference() { isRValueReference = true; + // Cannot be both + isLValueReference = false; } public void setSigned() { @@ -550,7 +569,15 @@ public class DemangledDataType extends DemangledType { } public boolean isReference() { - return isReference; + return isLValueReference() || isRValueReference(); + } + + public boolean isLValueReference() { + return isLValueReference; + } + + public boolean isRValueReference() { + return isRValueReference; } public boolean isSigned() { @@ -697,11 +724,11 @@ public class DemangledDataType extends DemangledType { buffer.append(SPACE + PTR_NOTATION); } - if (isReference) { + if (isLValueReference) { buffer.append(SPACE + REF_NOTATION); - if (isRValueReference) { - buffer.append(REF_NOTATION); // && - } + } + else if (isRValueReference) { + buffer.append(SPACE + RIGHT_REF_NOTATION); } // the order of __ptr64 and __restrict can vary--with fuzzing... diff --git a/Ghidra/Features/GnuDemangler/src/main/java/ghidra/app/util/demangler/gnu/GnuDemanglerParser.java b/Ghidra/Features/GnuDemangler/src/main/java/ghidra/app/util/demangler/gnu/GnuDemanglerParser.java index 3c0f99b940..8dbd8ce5b3 100644 --- a/Ghidra/Features/GnuDemangler/src/main/java/ghidra/app/util/demangler/gnu/GnuDemanglerParser.java +++ b/Ghidra/Features/GnuDemangler/src/main/java/ghidra/app/util/demangler/gnu/GnuDemanglerParser.java @@ -99,7 +99,7 @@ public class GnuDemanglerParser { * * Pattern: [optional const with optional '[',']', number, '*', '&' ] * (*|&)[optional spaces]brackets with optional characters inside - * + * * Parts: * -optional const text (e.g., const[8]) (non-capture group) * -followed by '()' that contain a '&' or a '*' (capture group 1) @@ -245,9 +245,9 @@ public class GnuDemanglerParser { * -'for' or 'to' (capture group 3) | (capture group 1) * -a space -+ * -optional text (capture group 4) - * + * * Note: capture group 1 is the combination of groups 2 and 3 with trailing space - * + * * Examples: * construction vtable for * vtable for @@ -313,7 +313,7 @@ public class GnuDemanglerParser { /** * Pattern to catch literal strings of the form: - * + * * -1l * 2l * 0u @@ -870,7 +870,7 @@ public class GnuDemanglerParser { } else if (ch == '&') { if (!dt.isReference()) { - dt.setReference(); + dt.setLValueReference(); } else { dt.setRValueReference(); @@ -1052,9 +1052,9 @@ public class GnuDemanglerParser { /* Note: really, this should just be checking a list of known disallowed characters, which is something like: - + <,>,(,),&,*,[,] - + It seems like the current code below is unnecessarily restrictive */ @@ -1286,7 +1286,7 @@ public class GnuDemanglerParser { dt.incrementPointerLevels(); } else if (type.equals("&")) { - dt.setReference(); + dt.setLValueReference(); } else { throw new DemanglerParseException("Unexpected charater inside of parens: " + type); @@ -1369,9 +1369,9 @@ public class GnuDemanglerParser { /* Examples: - + NS1::Function<>()::StructureName::StructureConstructor() - + */ String nameString = removeBadSpaces(demangled).trim(); @@ -1553,13 +1553,13 @@ public class GnuDemanglerParser { Samples: prefix: construction vtable for name: construction-vtable - + prefix: vtable for name: vtable - + prefix: typeinfo name for name: typeinfo-name - + prefix: covariant return thunk name: covariant-return */ diff --git a/Ghidra/Features/GnuDemangler/src/test/java/ghidra/app/util/demangler/GnuDemanglerParserTest.java b/Ghidra/Features/GnuDemangler/src/test/java/ghidra/app/util/demangler/GnuDemanglerParserTest.java index 519b0008a4..52f57a91bd 100644 --- a/Ghidra/Features/GnuDemangler/src/test/java/ghidra/app/util/demangler/GnuDemanglerParserTest.java +++ b/Ghidra/Features/GnuDemangler/src/test/java/ghidra/app/util/demangler/GnuDemanglerParserTest.java @@ -1034,9 +1034,9 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { std::__ndk1::allocator<{lambda(dummy::it::other::Namespace*)#1}>, int (dummy::it::other::Namespace*) > - + '__func' has 3 template parameters, the operator and the allocator - + */ String dummyNs = "dummy::it::other::Namespace"; @@ -1103,13 +1103,13 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { public void testOperator_WithTemplatesMissingATemplateArgument() throws Exception { /* - + Note: the empty template type: '<, std...' <, std::__cxx11::basic_string, std::allocator>> - - + + std::__cxx11::basic_string, std::allocator > std::_Bind, std::allocator > (EduAppConfigs::*(EduAppConfigs const*))() const>::operator()<, std::__cxx11::basic_string, std::allocator > >() - + */ String mangled = "_ZNSt5_BindIFM13EduAppConfigsKFNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEvEPKS0_EEclIJES6_EET0_DpOT_"; @@ -1163,9 +1163,9 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { public void testLambdaWithLambdaParameters() throws Exception { /* - + lambda contents - lambdas in templates and as a parameter - + bool (*** const* std:: __addressof< @@ -1185,7 +1185,7 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { {lambda(bool (*** const&)(AssertHandlerContext const&))#1} ) )(AssertHandlerContext const&) - + */ String mangled = @@ -1230,25 +1230,25 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { String demangled = process.demangle(mangled); /* - + Full demangled: - + Operator Text - + entt:: basic_registry:: assure >() const:: {lambda(entt::sparse_set&, entt::basic_registry&, EntityId)#1}:: operator void (*)(entt::sparse_set&, entt::basic_registry&, EntityId)() const - + Operartor Without Namespace - + operator void (*)(entt::sparse_set&, entt::basic_registry&, EntityId)() - + Simplified Cast Operator Construct - + operator void (*)(A,B,C)() - + */ DemangledObject object = parser.parse(mangled, demangled); @@ -1872,20 +1872,20 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { // /* - + Demangled: - + auto && JsonUtil:: addMember>,AvoidBlockGoal::Definition,float> ( - + std::shared_ptr>, float AvoidBlockGoal::Definition::*, char const *, float AvoidBlockGoal::Definition::* const& - + ) - + */ String mangled = "_ZN8JsonUtil9addMemberISt10shared_ptrINS_20JsonSchemaObjectNodeINS_10EmptyClassEN14AvoidBlockGoal10DefinitionEEEES5_fEEODaT_MT0_T1_PKcRKSC_"; @@ -1927,8 +1927,18 @@ public class GnuDemanglerParserTest extends AbstractGenericTest { assertName(object, name, "WebCore", "TextCodecICU"); String signature = object.getSignature(false); + + // 20230719: Note that argument in the following function is not correctly parsed. + // The argument is a non-pointer/ref reference to a function that returns void and + // which takes the argument list shown. Parser looks for double closing parenthesis, + // but that is found on the last argument of the template parameter. It actually needed + // to find the last closing parethesis and know that there could be nested pairs. + // Modification on 20230719 has "(* &&)" emitted in place of "(*)" because of lref/rref + // work in GP-3649, but the "&&" was found due to the closing parenthesis issue, which + // already existed. + // TODO: needs fixed assertEquals( - "undefined WebCore::TextCodecICU::registerCodecs(void (*)(char const *,WTF::Function> ()> &&))", + "undefined WebCore::TextCodecICU::registerCodecs(void (* &&)(char const *,WTF::Function> ()> &&))", signature); } diff --git a/Ghidra/Features/MicrosoftDemangler/src/test/java/ghidra/app/plugin/core/analysis/MicrosoftDemanglerAnalyzerTest.java b/Ghidra/Features/MicrosoftDemangler/src/test/java/ghidra/app/plugin/core/analysis/MicrosoftDemanglerAnalyzerTest.java index 480c3a4fcb..ee313d4b30 100644 --- a/Ghidra/Features/MicrosoftDemangler/src/test/java/ghidra/app/plugin/core/analysis/MicrosoftDemanglerAnalyzerTest.java +++ b/Ghidra/Features/MicrosoftDemangler/src/test/java/ghidra/app/plugin/core/analysis/MicrosoftDemanglerAnalyzerTest.java @@ -26,6 +26,7 @@ import ghidra.framework.options.Options; import ghidra.program.database.ProgramBuilder; import ghidra.program.database.ProgramDB; import ghidra.program.model.address.Address; +import ghidra.program.model.data.ParameterDefinition; import ghidra.program.model.listing.*; import ghidra.program.model.symbol.SourceType; import ghidra.test.AbstractGhidraHeadedIntegrationTest; @@ -97,6 +98,29 @@ public class MicrosoftDemanglerAnalyzerTest extends AbstractGhidraHeadedIntegrat assertEquals("undefined InvokeHelperV(void)", function.getSignature().toString()); } + @Test + public void testApplyComplicatedFunctionSignatureHavingReference() throws Exception { + + String mangled = "?f2@@YAP6AP6AHH@ZP6ADD@Z@ZAAP6AP6AHH@Z0@Z@Z"; + + Address addr = addr("0x110"); + createSymbol(addr, mangled); + + analyze(); + + FunctionManager fm = program.getFunctionManager(); + Function function = fm.getFunctionAt(addr); + assertNotNull(function); + ParameterDefinition[] params = function.getSignature().getArguments(); + assertEquals(1, params.length); + ParameterDefinition param = params[0]; + assertEquals("_func__func_int_int_ptr__func_char_char_ptr * * param_1", + param.toString()); + assertEquals( + "_func__func_int_int_ptr__func_char_char_ptr * f2(_func__func_int_int_ptr__func_char_char_ptr * * param_1)", + function.getSignature().toString()); + } + //================================================================================================== // Private Methods //================================================================================================== diff --git a/Ghidra/Features/MicrosoftDmang/src/main/java/mdemangler/MDMangGhidra.java b/Ghidra/Features/MicrosoftDmang/src/main/java/mdemangler/MDMangGhidra.java index 431178bdc2..a6784da73d 100644 --- a/Ghidra/Features/MicrosoftDmang/src/main/java/mdemangler/MDMangGhidra.java +++ b/Ghidra/Features/MicrosoftDmang/src/main/java/mdemangler/MDMangGhidra.java @@ -452,7 +452,8 @@ public class MDMangGhidra extends MDMang { } private DemangledFunctionReference processDemangledFunctionReference(MDModifierType refType) { - if (!((refType instanceof MDReferenceType) || (refType instanceof MDDataRightReferenceType))) { + if (!((refType instanceof MDReferenceType) || + (refType instanceof MDDataRightReferenceType))) { return null; // Not planning on anything else yet. } DemangledFunctionReference functionReference = @@ -610,18 +611,19 @@ public class MDMangGhidra extends MDMang { // modifierType.getArrayString(); // resultDataType.setArray(); //Processing the referenced type (for Ghidra, and then setting attributes on it) - processDataType(resultDataType, (MDDataType) modifierType.getReferencedType()); - resultDataType.incrementPointerLevels(); + DemangledDataType newResult = + processDataType(resultDataType, (MDDataType) modifierType.getReferencedType()); + newResult.incrementPointerLevels(); if (modifierType.getCVMod().isConst()) { - resultDataType.setConst(); + newResult.setConst(); } if (modifierType.getCVMod().isVolatile()) { - resultDataType.setVolatile(); + newResult.setVolatile(); } if (modifierType.getCVMod().isPointer64()) { - resultDataType.setPointer64(); + newResult.setPointer64(); } - return resultDataType; + return newResult; } // TODO: fix. Following is a kludge because DemangledObject has no // DemangledReference @@ -648,18 +650,19 @@ public class MDMangGhidra extends MDMang { return fr; } //Processing the referenced type (for Ghidra, and then setting attributes on it) - processDataType(resultDataType, (MDDataType) modifierType.getReferencedType()); - resultDataType.setReference(); // Not sure if we should do/use this. + DemangledDataType newResult = + processDataType(resultDataType, (MDDataType) modifierType.getReferencedType()); + newResult.setLValueReference(); if (modifierType.getCVMod().isConst()) { - resultDataType.setConst(); + newResult.setConst(); } if (modifierType.getCVMod().isVolatile()) { - resultDataType.setVolatile(); + newResult.setVolatile(); } if (modifierType.getCVMod().isPointer64()) { - resultDataType.setPointer64(); + newResult.setPointer64(); } - return resultDataType; + return newResult; } // TODO: fix. Following is a kludge because DemangledObject has no DemangledReference // with corresponding referencedType. @@ -725,7 +728,7 @@ public class MDMangGhidra extends MDMang { } //Processing the referenced type (for Ghidra, and then setting attributes on it) processDataType(resultDataType, (MDDataType) modifierType.getReferencedType()); - resultDataType.setReference(); // Not sure if we should do/use this. + resultDataType.setRValueReference(); if (modifierType.getCVMod().isConst()) { resultDataType.setConst(); } @@ -808,7 +811,7 @@ public class MDMangGhidra extends MDMang { } } else if (datatype instanceof MDReferenceType) { - resultDataType.setReference(); + resultDataType.setLValueReference(); } else if (datatype instanceof MDArrayBasicType) { resultDataType.setArray(1);