Adopt feedback from PR, make lookupSymbol() a to-be-deprecated function that calls SymbolInfo:lookup() with a better signature

Author: grynspan

stdlib/public/runtime/Errors.cpp

@@ -155,7 +155,7 @@ void swift::dumpStackTraceEntry(unsigned index, void *framePC,
   // 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.

stdlib/public/runtime/ImageInspectionCommon.cpp

@@ -192,8 +192,8 @@ const char *
 swift_getMetadataSectionName(const swift::MetadataSections *section) {
   swift::SymbolInfo info;
   if (lookupSymbol(section, &info)) {
-    if (info.getFileName()) {
-      return info.getFileName();
+    if (info.getFilename()) {
+      return info.getFilename();
     }
   }
   return "";

stdlib/public/runtime/SymbolInfo.cpp

@@ -21,11 +21,13 @@
 #include <dlfcn.h>
 #endif
 
+#include <utility>
+
 #include "ImageInspection.h"
 
 using namespace swift;
 
-const char *swift::SymbolInfo::getFileName() const {
+const char *swift::SymbolInfo::getFilename() const {
 #if defined(_WIN32) && !defined(__CYGWIN__)
   return nullptr;
 #elif SWIFT_STDLIB_HAS_DLADDR
@@ -65,31 +67,32 @@ const void *swift::SymbolInfo::getSymbolAddress() const {
 #endif
 }
 
-int swift::SymbolInfo::lookup(const void *address, SymbolInfo *outInfo) {
-  int result = 0;
+llvm::Optional<SymbolInfo> swift::SymbolInfo::lookup(const void *address) {
+  llvm::Optional<SymbolInfo> result;
 
 #if defined(__wasm__)
   // Currently, Wasm doesn't have a standard stable ABI for exporting address <->
   // symbol table, it's work in progress. Also, there is no API to access such
   // information from Wasm binary side. It's accessible only from host VM.
   // See https://github.com/WebAssembly/tool-conventions/blob/main/DynamicLinking.md
-
 #elif defined(_WIN32) && !defined(__CYGWIN__)
   _swift_win32_withDbgHelpLibrary([&] (HANDLE hProcess) {
     if (!hProcess) {
-      return;
+      return 0;
     }
 
-    outInfo->_package.si.SizeOfStruct = sizeof(SYMBOL_INFO);
-    outInfo->_package.si.MaxNameLen = MAX_SYM_NAME;
+    SymbolInfo info;
+    info._package.si.SizeOfStruct = sizeof(SYMBOL_INFO);
+    info._package.si.MaxNameLen = MAX_SYM_NAME;
     if (SymFromAddr(hProcess, reinterpret_cast<const DWORD64>(address),
-                    nullptr, &outInfo->_package.si)) {
-      result = 1;
+                    nullptr, &info._package.si)) {
+      result = std::move(info);
     }
   });
 #elif SWIFT_STDLIB_HAS_DLADDR
-  if (dladdr(address, &outInfo->_info)) {
-    result = 1;
+  SymbolInfo info;
+  if (dladdr(address, &info._info)) {
+    result = std::move(info);
   }
 #endif
 

stdlib/public/runtime/SymbolInfo.h

@@ -37,6 +37,8 @@
 #include <dlfcn.h>
 #endif
 
+#include "llvm/ADT/Optional.h"
+
 namespace swift {
 struct SymbolInfo {
 private:
@@ -50,22 +52,49 @@ struct SymbolInfo {
   SymbolInfo() {}
 
   /// Get the file name of the image where the symbol was found.
-  const char *getFileName() const;
+  ///
+  /// The resulting C string is only valid for the lifetime of \c this.
+  const char *getFilename() const;
 
   /// Get the base address of the image where the symbol was found.
   const void *getBaseAddress() const;
 
   /// Get the name of the symbol.
+  ///
+  /// The resulting C string is only valid for the lifetime of \c this.
   const char *getSymbolName() const;
 
   /// Get the address of the symbol.
   const void *getSymbolAddress() const;
 
-  static int lookup(const void *address, SymbolInfo *outInfo);
+  /// Look up a symbol by address.
+  ///
+  /// \param address The address where the symbol is located.
+  ///
+  /// \returns On success, an instance of \c SymbolInfo containing information
+  ///   about the symbol at \a address. On failure, \c llvm::None.
+  static llvm::Optional<SymbolInfo> 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) {
-  return SymbolInfo::lookup(address, outInfo);
+  auto info = SymbolInfo::lookup(address);
+  if (info.has_value()) {
+    *outInfo = info.value();
+    return 1;
+  } else {
+    return 0;
+  }
 }
 
 } // end namespace swift