Skip to content

Demangler: handle errors in demangleType #74767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/swift/RemoteInspection/TypeRefBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,8 @@ class TypeRefBuilder {
auto substitutedDemangleTree = Builder.demangleTypeRef(
substitutedTypeRef,
/* useOpaqueTypeSymbolicReferences */ true);
if (!substitutedDemangleTree)
continue;

// If the substituted type is an opaque type, also gather info
// about which protocols it is required to conform to and the
Expand Down
22 changes: 18 additions & 4 deletions lib/Demangling/Demangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ llvm::StringRef swift::Demangle::dropSwiftManglingPrefix(StringRef mangledName){
}

static bool isAliasNode(Demangle::NodePointer Node) {
if (!Node)
return false;
switch (Node->getKind()) {
case Demangle::Node::Kind::Type:
return isAliasNode(Node->getChild(0));
Expand All @@ -241,6 +243,8 @@ bool swift::Demangle::isAlias(llvm::StringRef mangledName) {
}

static bool isClassNode(Demangle::NodePointer Node) {
if (!Node)
return false;
switch (Node->getKind()) {
case Demangle::Node::Kind::Type:
return isClassNode(Node->getChild(0));
Expand All @@ -259,6 +263,8 @@ bool swift::Demangle::isClass(llvm::StringRef mangledName) {
}

static bool isEnumNode(Demangle::NodePointer Node) {
if (!Node)
return false;
switch (Node->getKind()) {
case Demangle::Node::Kind::Type:
return isEnumNode(Node->getChild(0));
Expand All @@ -277,6 +283,8 @@ bool swift::Demangle::isEnum(llvm::StringRef mangledName) {
}

static bool isProtocolNode(Demangle::NodePointer Node) {
if (!Node)
return false;
switch (Node->getKind()) {
case Demangle::Node::Kind::Type:
return isProtocolNode(Node->getChild(0));
Expand All @@ -296,6 +304,8 @@ bool swift::Demangle::isProtocol(llvm::StringRef mangledName) {
}

static bool isStructNode(Demangle::NodePointer Node) {
if (!Node)
return false;
switch (Node->getKind()) {
case Demangle::Node::Kind::Type:
return isStructNode(Node->getChild(0));
Expand Down Expand Up @@ -774,12 +784,16 @@ NodePointer Demangler::demangleType(StringRef MangledName,
std::function<SymbolicReferenceResolver_t> Resolver) {
DemangleInitRAII state(*this, MangledName, std::move(Resolver));

parseAndPushNodes();
if (!parseAndPushNodes())
return nullptr;

NodePointer Result = popNode();

if (NodePointer Result = popNode())
return Result;
// The result is only valid if it was the only node on the stack.
if (popNode())
return nullptr;

return createNode(Node::Kind::Suffix, Text);
return Result;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there harm to keeping the Suffix production? The runtime demangler should never accept it as a valid mangling, but it's helpful when demangling generated symbol names in binaries. Could it be kept as an option?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it can also be helpful when debugging mangling errors, since it generally points at exactly what you messed up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change only affects demangleType (the runtime demangler). Nothing changes for demangling symbols

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

debugging mangling errors

Pro tip: Demangler::dump() prints the internal state of the demangler which is very useful for debugging mangling problems

}

bool Demangler::parseAndPushNodes() {
Expand Down
9 changes: 9 additions & 0 deletions test/Demangle/demangle-types.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
RUN: %swift-demangle -type Si | %FileCheck %s --check-prefix=SWIFT-INT
SWIFT-INT: Swift.Int

RUN: %swift-demangle -type SS_ | %FileCheck %s --check-prefix=MULTI-NODE-ERROR
MULTI-NODE-ERROR: <<invalid type>>

RUN: %swift-demangle -type SSIeAghrx_ | %FileCheck %s --check-prefix=PARSE-ERROR
PARSE-ERROR: <<invalid type>>

26 changes: 13 additions & 13 deletions test/Runtime/demangleToMetadata.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,28 +79,28 @@ DemangleToMetadataTests.test("function types") {
expectEqual(type(of: f0_throws), _typeByName("yyKc")!)

// More parameters.
expectEqual(type(of: f1), _typeByName("yyyt_tc")!)
expectEqual(type(of: f2), _typeByName("yyyt_yttc")!)
expectEqual(type(of: f1), _typeByName("yyt_tc")!)
expectEqual(type(of: f2), _typeByName("yyt_yttc")!)

// Variadic parameters.
expectEqual(type(of: f1_variadic), _typeByName("yyytd_tc")!)
expectEqual(type(of: f1_variadic), _typeByName("yytd_tc")!)

// Inout parameters.
expectEqual(type(of: f1_inout), _typeByName("yyytzc")!)
expectEqual(type(of: f1_inout), _typeByName("yytzc")!)

// Ownership parameters.
expectEqual(type(of: f1_shared), _typeByName("yyyXlhc")!)
expectEqual(type(of: f1_owned), _typeByName("yyyXlnc")!)
expectEqual(type(of: f1_shared), _typeByName("yyXlhc")!)
expectEqual(type(of: f1_owned), _typeByName("yyXlnc")!)

// Concurrent function types.
expectEqual(type(of: f1_takes_concurrent), _typeByName("yyyyYbXEc")!)
expectEqual(type(of: f1_takes_concurrent), _typeByName("yyyYbXEc")!)

// Mix-and-match.
expectEqual(type(of: f2_variadic_inout), _typeByName("yyytd_ytztc")!)
expectEqual(type(of: f2_variadic_inout), _typeByName("yytd_ytztc")!)

// A function type that hasn't been built before.
expectEqual("(Int, Float, Double, String, Character, UInt, Bool) -> ()",
String(describing: _typeByName("yySi_SfSdSSs9CharacterVSuSbtc")!))
String(describing: _typeByName("ySi_SfSdSSs9CharacterVSuSbtc")!))

// Escaping
expectEqual(type(of: f1_escaping), _typeByName("ySfSicc")!)
Expand Down Expand Up @@ -145,16 +145,16 @@ func f1_composition_superclass(_: C & P1 & P2) { }

DemangleToMetadataTests.test("existential types") {
// Any, AnyObject
expectEqual(type(of: f2_any_anyobject), _typeByName("yyyp_yXltc")!)
expectEqual(type(of: f2_any_anyobject), _typeByName("yyp_yXltc")!)

// References to protocols.
expectEqual(type(of: f1_composition), _typeByName("yy4main2P1_4main2P2pc")!)
expectEqual(type(of: f1_composition), _typeByName("y4main2P1_4main2P2pc")!)

// Reference to protocol with AnyObject.
expectEqual(type(of: f1_composition_anyobject), _typeByName("yy4main2P1_Xlc")!)
expectEqual(type(of: f1_composition_anyobject), _typeByName("y4main2P1_Xlc")!)

// References to superclass.
expectEqual(type(of: f1_composition_superclass), _typeByName("yy4main2P1_4main2P2AA1CCXcc")!)
expectEqual(type(of: f1_composition_superclass), _typeByName("y4main2P1_4main2P2AA1CCXcc")!)

// Demangle an existential type that hasn't been seen before.
expectEqual("P1 & P2 & P3", String(describing: _typeByName("4main2P1_4main2P24main2P3p")!))
Expand Down
10 changes: 5 additions & 5 deletions test/Runtime/demangleToMetadataObjC.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ func f1_composition_objc_protocol_P4(_: mainP4) { }

DemangleToMetadataTests.test("@objc protocols") {
expectEqual(type(of: f1_composition_objc_protocol),
_typeByName("yy4main2P1_pc")!)
_typeByName("y4main2P1_pc")!)
expectEqual(type(of: f1_composition_objc_protocol_P4),
_typeByName("yy4main0A2P4_pc")!)
_typeByName("y4main0A2P4_pc")!)
}

DemangleToMetadataTests.test("Objective-C classes") {
Expand All @@ -53,13 +53,13 @@ func f1_composition_NSCoding(_: NSCoding) { }
func f1_composition_P5(_: P5) { }

DemangleToMetadataTests.test("Objective-C protocols") {
expectEqual(type(of: f1_composition_NSCoding), _typeByName("yySo8NSCoding_pc")!)
expectEqual(type(of: f1_composition_NSCoding), _typeByName("ySo8NSCoding_pc")!)

// @objc Swift protocols can be found by their Objective-C names...
expectEqual(type(of: f1_composition_P5), _typeByName("yySo15P5RenamedInObjC_pc")!)
expectEqual(type(of: f1_composition_P5), _typeByName("ySo15P5RenamedInObjC_pc")!)

// ... but not their Swift names.
expectNil(_typeByName("yy4main2P5_pc"))
expectNil(_typeByName("y4main2P5_pc"))
}

DemangleToMetadataTests.test("Classes that don't exist") {
Expand Down
22 changes: 20 additions & 2 deletions tools/swift-demangle/swift-demangle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ static llvm::cl::opt<bool>
TreeOnly("tree-only",
llvm::cl::desc("Tree-only mode (do not show the demangled string)"));

static llvm::cl::opt<bool>
DemangleType("type",
llvm::cl::desc("Demangle a runtime type string"));

static llvm::cl::opt<bool>
StripSpecialization("strip-specialization",
llvm::cl::desc("Remangle the origin of a specialized function"));
Expand Down Expand Up @@ -141,7 +145,17 @@ static void demangle(llvm::raw_ostream &os, llvm::StringRef name,
hadLeadingUnderscore = true;
name = name.substr(1);
}
swift::Demangle::NodePointer pointer = DCtx.demangleSymbolAsNode(name);
swift::Demangle::NodePointer pointer;
if (DemangleType) {
pointer = DCtx.demangleTypeAsNode(name);
if (!pointer) {
os << "<<invalid type>>";
return;
}
} else {
pointer = DCtx.demangleSymbolAsNode(name);
}

if (ExpandMode || TreeOnly) {
os << "Demangling for " << name << '\n';
os << getNodeTreeAsString(pointer);
Expand Down Expand Up @@ -399,6 +413,10 @@ int main(int argc, char **argv) {

if (InputNames.empty()) {
CompactMode = true;
if (DemangleType) {
llvm::errs() << "The option -type cannot be used to demangle stdin.\n";
return EXIT_FAILURE;
}
return demangleSTDIN(options);
} else {
swift::Demangle::Context DCtx;
Expand All @@ -415,7 +433,7 @@ int main(int argc, char **argv) {
"is quoted or escaped.\n";
continue;
}
if (name.starts_with("S") || name.starts_with("s") ) {
if (!DemangleType && (name.starts_with("S") || name.starts_with("s"))) {
std::string correctedName = std::string("$") + name.str();
demangle(llvm::outs(), correctedName, DCtx, options);
} else {
Expand Down