From e8bd036022d2590790ce60c8d0f772fb38cdf030 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Mon, 12 Dec 2022 15:04:00 -0500 Subject: [PATCH 1/3] Remove lookupSymbol() and have all callers use SymbolInfo::lookup() instead --- .../SwiftShims/swift/shims/MetadataSections.h | 4 ++-- stdlib/public/runtime/Errors.cpp | 14 ++++++------- .../public/runtime/ImageInspectionCommon.cpp | 16 +++++++------- stdlib/public/runtime/MetadataLookup.cpp | 5 ++--- stdlib/public/runtime/ProtocolConformance.cpp | 11 +++++----- stdlib/public/runtime/ReflectionMirror.cpp | 5 ++--- stdlib/public/runtime/SymbolInfo.h | 21 ------------------- 7 files changed, 25 insertions(+), 51 deletions(-) diff --git a/stdlib/public/SwiftShims/swift/shims/MetadataSections.h b/stdlib/public/SwiftShims/swift/shims/MetadataSections.h index 386608fac0b7d..1667d13e6687a 100644 --- a/stdlib/public/SwiftShims/swift/shims/MetadataSections.h +++ b/stdlib/public/SwiftShims/swift/shims/MetadataSections.h @@ -53,8 +53,8 @@ struct MetadataSections { /// reported when the section was registered with the Swift runtime. /// /// The value of this field is equivalent to the value of - /// \c SymbolInfo::baseAddress as returned from \c lookupSymbol() for a symbol - /// in the image that contains these sections. + /// \c SymbolInfo::baseAddress as returned from \c SymbolInfo::lookup() for a + /// symbol in the image that contains these sections. /// /// For Mach-O images, set this field to \c __dso_handle (i.e. the Mach header /// for the image.) For ELF images, set it to \c __dso_handle (the runtime diff --git a/stdlib/public/runtime/Errors.cpp b/stdlib/public/runtime/Errors.cpp index f396aeb0594dc..221e2b0712a6d 100644 --- a/stdlib/public/runtime/Errors.cpp +++ b/stdlib/public/runtime/Errors.cpp @@ -144,18 +144,16 @@ static bool getSymbolNameAddr(llvm::StringRef libraryName, void swift::dumpStackTraceEntry(unsigned index, void *framePC, bool shortOutput) { #if SWIFT_STDLIB_SUPPORTS_BACKTRACE_REPORTING && SWIFT_STDLIB_HAS_DLADDR - SymbolInfo syminfo; - - // 0 is failure for lookupSymbol - if (0 == lookupSymbol(framePC, &syminfo)) { + auto syminfo = SymbolInfo::lookup(framePC); + if (!syminfo.has_value()) { return; } - // If lookupSymbol succeeded then fileName is non-null. Thus, we find the + // If SymbolInfo:lookup succeeded then fileName is non-null. Thus, we find the // library name here. Avoid using StringRef::rsplit because its definition // is not provided in the header so that it requires linking with // libSupport.a. - llvm::StringRef libraryName{syminfo.getFilename()}; + llvm::StringRef libraryName{syminfo->getFilename()}; libraryName = libraryName.substr(libraryName.rfind('/')).substr(1); // Next we get the symbol name that we are going to use in our backtrace. @@ -165,12 +163,12 @@ void swift::dumpStackTraceEntry(unsigned index, void *framePC, // we just get HexAddr + 0. uintptr_t symbolAddr = uintptr_t(framePC); bool foundSymbol = - getSymbolNameAddr(libraryName, syminfo, symbolName, symbolAddr); + getSymbolNameAddr(libraryName, syminfo.value(), symbolName, symbolAddr); ptrdiff_t offset = 0; if (foundSymbol) { offset = ptrdiff_t(uintptr_t(framePC) - symbolAddr); } else { - auto baseAddress = syminfo.getBaseAddress(); + auto baseAddress = syminfo->getBaseAddress(); offset = ptrdiff_t(uintptr_t(framePC) - uintptr_t(baseAddress)); symbolAddr = uintptr_t(framePC); symbolName = ""; diff --git a/stdlib/public/runtime/ImageInspectionCommon.cpp b/stdlib/public/runtime/ImageInspectionCommon.cpp index 2a8147b4c555e..8baf00d31cd5b 100644 --- a/stdlib/public/runtime/ImageInspectionCommon.cpp +++ b/stdlib/public/runtime/ImageInspectionCommon.cpp @@ -64,8 +64,8 @@ static void fixupMetadataSectionBaseAddress(swift::MetadataSections *sections) { if (fixupNeeded) { // We need to fix up the base address. We'll need a known-good address in // the same image: `sections` itself will work nicely. - swift::SymbolInfo symbolInfo; - if (lookupSymbol(sections, &symbolInfo) && symbolInfo.getBaseAddress()) { + auto symbolInfo = SymbolInfo::lookup(sections); + if (symbolInfo.has_value() && symbolInfo.getBaseAddress()) { sections->baseAddress.store(symbolInfo.getBaseAddress(), std::memory_order_relaxed); } @@ -190,10 +190,9 @@ const swift::MetadataSections *swift_getMetadataSection(size_t index) { SWIFT_RUNTIME_EXPORT const char * swift_getMetadataSectionName(const swift::MetadataSections *section) { - swift::SymbolInfo info; - if (lookupSymbol(section, &info)) { - if (info.getFilename()) { - return info.getFilename(); + if (auto info = SymbolInfo::lookup(section)) { + if (info->getFilename()) { + return info->getFilename(); } } return ""; @@ -203,9 +202,8 @@ SWIFT_RUNTIME_EXPORT void swift_getMetadataSectionBaseAddress(const swift::MetadataSections *section, void const **out_actual, void const **out_expected) { - swift::SymbolInfo info; - if (lookupSymbol(section, &info)) { - *out_actual = info.getBaseAddress(); + if (auto info = SymbolInfo::lookup(section)) { + *out_actual = info->getBaseAddress(); } else { *out_actual = nullptr; } diff --git a/stdlib/public/runtime/MetadataLookup.cpp b/stdlib/public/runtime/MetadataLookup.cpp index 06c2fdc6c3511..fa62fbe271550 100644 --- a/stdlib/public/runtime/MetadataLookup.cpp +++ b/stdlib/public/runtime/MetadataLookup.cpp @@ -1105,9 +1105,8 @@ _gatherGenericParameters(const ContextDescriptor *context, str += "_gatherGenericParameters: context: "; - SymbolInfo contextInfo; - if (lookupSymbol(context, &contextInfo)) { - str += contextInfo.getSymbolName(); + if (auto contextInfo = SymbolInfo::lookup(context)) { + str += contextInfo->getSymbolName(); str += " "; } diff --git a/stdlib/public/runtime/ProtocolConformance.cpp b/stdlib/public/runtime/ProtocolConformance.cpp index 3853e322fbcbc..36eb28cdb3791 100644 --- a/stdlib/public/runtime/ProtocolConformance.cpp +++ b/stdlib/public/runtime/ProtocolConformance.cpp @@ -124,12 +124,13 @@ static const char *class_getName(const ClassMetadata* type) { } template<> void ProtocolConformanceDescriptor::dump() const { - SymbolInfo info; + llvm::Optional info; auto symbolName = [&](const void *addr) -> const char * { - int ok = lookupSymbol(addr, &info); - if (!ok || !info.getSymbolName()) - return ""; - return info.getSymbolName(); + info = SymbolInfo::lookup(addr); + if (info.has_value() && info->getSymbolName()) { + return info->getSymbolName(); + } + return ""; }; switch (auto kind = getTypeKind()) { diff --git a/stdlib/public/runtime/ReflectionMirror.cpp b/stdlib/public/runtime/ReflectionMirror.cpp index 39ec8da6b4000..1f81cc5687e25 100644 --- a/stdlib/public/runtime/ReflectionMirror.cpp +++ b/stdlib/public/runtime/ReflectionMirror.cpp @@ -1116,9 +1116,8 @@ id swift_reflectionMirror_quickLookObject(OpaqueValue *value, const Metadata *T) SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL const char *swift_keyPath_copySymbolName(void *address) { - SymbolInfo info; - if (lookupSymbol(address, &info) && info.getSymbolName()) { - return strdup(info.getSymbolName()); + if (auto info = SymbolInfo::lookup(address) && info->getSymbolName()) { + return strdup(info->getSymbolName()); } return 0; } diff --git a/stdlib/public/runtime/SymbolInfo.h b/stdlib/public/runtime/SymbolInfo.h index 5c474f19b7ab8..c6868afe05a59 100644 --- a/stdlib/public/runtime/SymbolInfo.h +++ b/stdlib/public/runtime/SymbolInfo.h @@ -76,27 +76,6 @@ struct SymbolInfo { static llvm::Optional lookup(const void *address); }; -/// Look up a symbol by address and store the result in a locally-declared -/// \c SymbolInfo value. -/// -/// \param address The address where the symbol is located. -/// \param outInfo On successful return, populated with information about the -/// symbol at \a address. On failure, unspecified. -/// -/// \returns On success, a non-zero integer. On failure, zero. -/// -/// \note This function will be replaced with \c SymbolInfo::lookup() in a -/// future update. -static inline int lookupSymbol(const void *address, SymbolInfo *outInfo) { - auto info = SymbolInfo::lookup(address); - if (info.has_value()) { - *outInfo = info.value(); - return 1; - } else { - return 0; - } -} - } // end namespace swift #endif From 0de9d58c58c62eaae032bb1dd4d836bb022d4c58 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 13 Dec 2022 10:15:51 -0500 Subject: [PATCH 2/3] Fix typo --- stdlib/public/runtime/ReflectionMirror.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/stdlib/public/runtime/ReflectionMirror.cpp b/stdlib/public/runtime/ReflectionMirror.cpp index 1f81cc5687e25..22351a58644f4 100644 --- a/stdlib/public/runtime/ReflectionMirror.cpp +++ b/stdlib/public/runtime/ReflectionMirror.cpp @@ -1116,10 +1116,12 @@ id swift_reflectionMirror_quickLookObject(OpaqueValue *value, const Metadata *T) SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL const char *swift_keyPath_copySymbolName(void *address) { - if (auto info = SymbolInfo::lookup(address) && info->getSymbolName()) { - return strdup(info->getSymbolName()); + if (auto info = SymbolInfo::lookup(address)) { + if (info->getSymbolName()) { + return strdup(info->getSymbolName()); + } } - return 0; + return nullptr; } SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL From 747e5fd2956bc8f695c156312b759af2da5a705c Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 13 Dec 2022 12:03:17 -0500 Subject: [PATCH 3/3] Fix Linux build --- stdlib/public/runtime/ImageInspectionCommon.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stdlib/public/runtime/ImageInspectionCommon.cpp b/stdlib/public/runtime/ImageInspectionCommon.cpp index 8baf00d31cd5b..f692223d59428 100644 --- a/stdlib/public/runtime/ImageInspectionCommon.cpp +++ b/stdlib/public/runtime/ImageInspectionCommon.cpp @@ -65,8 +65,8 @@ static void fixupMetadataSectionBaseAddress(swift::MetadataSections *sections) { // We need to fix up the base address. We'll need a known-good address in // the same image: `sections` itself will work nicely. auto symbolInfo = SymbolInfo::lookup(sections); - if (symbolInfo.has_value() && symbolInfo.getBaseAddress()) { - sections->baseAddress.store(symbolInfo.getBaseAddress(), + if (symbolInfo.has_value() && symbolInfo->getBaseAddress()) { + sections->baseAddress.store(symbolInfo->getBaseAddress(), std::memory_order_relaxed); } }