[Reflection] Prevent symbolic reference mangling induced crashes

[kastiglione]

Based on lldb crash logs, TypeRefBuilder is crashing on instances of symbolic references in mangled names.

The crash happens when a symbolic reference is attempted to be mangled (via mangleNode()), which leads to a call to unreachable which aborts. For library usage, such as from lldb, these cases need to be handled without an abort.

The improvements to gracefully handle symbolic references are:

1. Change the return type of normalizeReflectionName from std::string to llvm::Optional<std::string>
2. Thread through a bool flag named useOpaqueTypeSymbolicReferences

Both of these start in normalizeReflectionName.

First, the call to demangleTypeRef() now sets useOpaqueTypeSymbolicReferences to false. Without this, demangleTypeRef() can return a node of kind OpaqueTypeDescriptorSymbolicReference, which is guaranteed to fail in this code path when in subsequent call to mangleNode().

Second, if the result of demangleTypeRef() is one of the symbolic reference kinds, then the function exits early with a value of None. Callers of normalizeReflectionName() are now forced to handle such cases, where the mangled name could not be normalized.

In TypeRefBuilder::getFieldTypeInfo(), if normalizeReflectionName() fails, then the corresponding field is not supported, or in other words, dropped.

rdar://77613304

[adrian-prantl]

The fix here is to avoid calling mangleNode() for symbolic reference nodes, since that is guaranteed to be bad.

Do we understand what that means, i.e., when we hit the mitigation, do we create incomplete mangled names that might cause crashes later?
For the longer-term perspective, should mangleNode be able to return errors?

[slavapestov]

@jckarter any ideas on how to handle symbolic references here?

[kastiglione]

Do we understand what that means, i.e., when we hit the mitigation, do we create incomplete mangled names that might cause crashes later?

Good question, and no. I am working on being able to reproduce the bug reports, but without that we don't know what the downstream effects are.

[mikeash]

You want to call the mangleNode overload that takes a SymbolicResolver. We have a few pre-built resolvers in Private.h (ResolveToDemanglingForContext, ResolveAsSymbolicReference, ExpandResolvedSymbolicReferences) but those are all in-process. It shouldn't be too hard to make a templatized version of _buildDemanglingForSymbolicReference that works remotely. Of course, if you ever encounter SymbolicReferenceKind::AccessorFunctionReference then you're just completely doomed in the remote case, so you'll have to have a failure case regardless.

I am very confused by all this, though. Does normalizeReflectionName just not work, but we get away with it because it rarely encounters symbolic references? Or is there some hidden path I haven't spotted which handles the common cases?

[kastiglione]

@mikeash I have updated this PR to handle the one potential case we discussed, where normalizeReflectionName returns a node of kind OpaqueTypeDescriptorSymbolicReference. If that were to happen it would be certain crash.

The fix is to thread through useOpaqueTypeSymbolicReferences, with a value of false – from normalizeReflectionName. This prevents one potential crash.

[kastiglione]

@swift-ci test

[kastiglione]

@swift-ci nominate

[kastiglione]

@swift-ci test

[mikeash]

Looks good. Thank you!

[kastiglione]

Closing for this branch only. See linked PRs for main and release/5.5.

[kastiglione]

@swift-ci test