apply code review feedback

Author: ktoso

include/swift/Runtime/Runtime.h

@@ -37,14 +37,14 @@ namespace runtime {
 namespace mangling {
 #endif
 
-  SWIFT_RUNTIME_STDLIB_SPI
-  char * _swift_runtime_demangle(
-    const char *mangledName,
-    size_t mangledNameLength,
-    char *outputBuffer,
-    size_t *outputBufferSize,
-    uint32_t flags
-  );
+SWIFT_RUNTIME_STDLIB_SPI
+char * _swift_runtime_demangle(
+  const char *mangledName,
+  size_t mangledNameLength,
+  char *outputBuffer,
+  size_t *outputBufferSize,
+  uint32_t flags
+);
 
 #ifdef __cplusplus
 } // namespace mangling

stdlib/public/RuntimeModule/Mangling.swift

@@ -29,7 +29,7 @@ internal import BacktracingImpl.Runtime
 
 // - MARK: Demangling
 
-/// Given a mangled Swift symbol, demangle it into a human readable format.
+/// Given a mangled Swift, or C++, symbol, demangle it into a human readable format.
 ///
 /// If the provided bytes are not a valid mangled swift name, the output span will be initialized with zero elements.
 /// If mangling succeeds the output span will contain the resulting demangled string.
@@ -52,6 +52,8 @@ internal import BacktracingImpl.Runtime
 ///   ├─────────────────────╫────────┤
 ///   │ Swift 5+            ║   $s   │
 ///   └─────────────────────╨────────┘
+/// 
+/// This function also attempts to demangle C++ symbols starting with `_Z`.
 ///
 /// - Parameters:
 ///   - mangledName: A mangled Swift symbol.
@@ -62,7 +64,7 @@ internal import BacktracingImpl.Runtime
 public func demangle(_ mangledName: String) -> String? {
   var length: size_t = 0
 
-  let demangled: UnsafeMutablePointer<Int8>? = _swift_runtime_demangle(
+  let demangled: UnsafeMutablePointer<CChar>? = _swift_runtime_demangle(
     mangledName, mangledName.utf8.count,
     nil, &length,
     /*flags=*/0

stdlib/public/RuntimeModule/SymbolicatedBacktrace.swift

@@ -202,28 +202,23 @@ public struct SymbolicatedBacktrace: CustomStringConvertible {
 
     /// Demangle the raw name, if possible.
     private func demangleRawName() -> String {
-      if #available(SwiftStdlib 6.3, *) {
-        // Reuse the public API demangle() when available
-        return demangle(rawName) ?? rawName
-      } else {
-        // Fallback to backtrace entrypoint to the demangler.
-        var length: size_t = 0
-        if let demangled = _swift_backtrace_demangle(rawName, rawName.utf8.count, nil, &length) {
-          defer { free(demangled) }
-
-          // length is the size of the buffer that was allocated, *not* the
-          // length of the string.
-          let stringLen = strlen(demangled)
-          if stringLen > 0 {
-            return demangled.withMemoryRebound(to: UInt8.self, capacity: stringLen) {
-              let demangledBytes = UnsafeBufferPointer<UInt8>(start: $0,
-              count: stringLen)
-              return String(decoding: demangledBytes, as: UTF8.self)
-            }
+      // Fallback to backtrace entrypoint to the demangler.
+      var length: size_t = 0
+      if let demangled = _swift_backtrace_demangle(rawName, rawName.utf8.count, nil, &length) {
+        defer { free(demangled) }
+
+        // length is the size of the buffer that was allocated, *not* the
+        // length of the string.
+        let stringLen = strlen(demangled)
+        if stringLen > 0 {
+          return demangled.withMemoryRebound(to: UInt8.self, capacity: stringLen) {
+            let demangledBytes = UnsafeBufferPointer<UInt8>(start: $0,
+            count: stringLen)
+            return String(decoding: demangledBytes, as: UTF8.self)
           }
         }
-        return rawName
       }
+      return rawName
     }
 
     /// A textual description of this symbol.

stdlib/public/RuntimeModule/modules/Runtime/Runtime.h

@@ -33,8 +33,6 @@ void swift_reportWarning(uint32_t flags, const char *message);
 bool _swift_backtrace_isThunkFunction(const char *rawName);
 
 // Demangle the given raw name (supports Swift and C++)
-//
-// The demangled string IS a null-terminated c-string.
 char *_swift_backtrace_demangle(const char *rawName,
                                 size_t rawNameLength,
                                 char *outputBuffer,
@@ -46,9 +44,14 @@ char *_swift_backtrace_demangle(const char *rawName,
 // If null is passed as the 'outputBuffer' the runtime function will allocate a buffer and return it.
 // If an 'outputBuffer' is passed, the output will be written into it, and the same buffer will be returned from this
 //
-// The demangled result String is NOT null-terminated. 
+// The demangled result string is NOT null-terminated. 
 // The demangled string length is indicated through the outputBufferSize parameter.
 // 
+// Currently supported flags:
+//  - 0: 'default'
+//  - 1: '_swift_backtrace_demangle compatible mode', uses SimplifiedUIDemangleOptions for formatting
+//  - *: unsupported values, result in immediate demangling failure.
+
 // Introduced in Swift 6.3.
 char *_swift_runtime_demangle(const char *rawName,
                               size_t rawNameLength,

stdlib/public/runtime/Demangle.cpp

@@ -1050,12 +1050,14 @@ namespace swift {
                             size_t *outputBufferSize,
                             uint32_t flags) {
       if (flags > 1) {
-        swift::fatalError(0, "Only 'flags' value of '0' and '1' is currently supported.");
-      }
-      if (outputBuffer != nullptr && outputBufferSize == nullptr) {
-        swift::fatalError(0, "'outputBuffer' is passed but the size is 'nullptr'.");
+        // ignore not supported flags
+        return nullptr;
       }
 
+      // Very simple flags parsing, move to something more proper once we support more flags
+      bool shouldNullTerminateString = flags & 1;
+      bool shouldUseSimplifiedUIDemangleOptions = flags & 1;
+
       llvm::StringRef name = llvm::StringRef(mangledName, mangledNameLength);
 
       // You must provide buffer size if you're providing your own output buffer
@@ -1066,7 +1068,7 @@ namespace swift {
       if (Demangle::isSwiftSymbol(name)) {
         // Determine demangling/formatting options:
         auto options = DemangleOptions();
-        if (flags == 1) {
+        if (shouldUseSimplifiedUIDemangleOptions) {
           // simplified display options, for backtraces
           options = DemangleOptions::SimplifiedUIDemangleOptions();
         }
@@ -1080,12 +1082,19 @@ namespace swift {
         }
 
         if (outputBuffer == nullptr) {
-          outputBuffer = (char *)::malloc(result.length());
+          auto totalLength = shouldNullTerminateString ? (result.length() + 1) : result.length();
+          outputBuffer = (char *)::malloc(totalLength);
           bufferSize = result.length();
         }
 
-        size_t toCopy = std::min(bufferSize, result.length());
+        size_t toCopy = std::min(
+          shouldNullTerminateString ? (bufferSize - 1) : bufferSize, 
+          shouldNullTerminateString ? (result.length() - 1) : result.length()
+        );
         ::memcpy(outputBuffer, result.data(), toCopy);
+        if (shouldNullTerminateString) {
+          outputBuffer[toCopy] = '\0';
+        }
 
         return outputBuffer;
     #ifndef _WIN32
@@ -1110,8 +1119,14 @@ namespace swift {
             return result;
           }
 
-          size_t toCopy = std::min(bufferSize, resultLen);
+          size_t toCopy = std::min(
+            shouldNullTerminateString ? (bufferSize - 1) : bufferSize, 
+            shouldNullTerminateString ? (resultLen - 1) : resultLen
+          );
           ::memcpy(outputBuffer, result, toCopy);
+          if (shouldNullTerminateString) {
+            outputBuffer[toCopy] = '\0';
+          }
 
           free(result);
 
@@ -1123,7 +1138,9 @@ namespace swift {
     #endif
       }
 
-      *outputBufferSize = 0; // indicate we did not write to buffer
+      if (outputBufferSize) {
+        *outputBufferSize = 0; // indicate we did not write to buffer
+      }
       return nullptr;
     }
   }

test/Concurrency/Runtime/RuntimeDemangle.swift

@@ -16,7 +16,7 @@ var DemangleTests = TestSuite("Demangle")
 
 if #available(SwiftStdlib 6.3, *) {
   DemangleTests.test("basic string return API") {
-    // First, test that we get back the mangled name with invalid input.
+    // First, test that we get back 'nil' with invalid input.
     expectEqual(demangle("abc123"), nil)
     expectEqual(demangle("Si"), nil)
     expectEqual(demangle("Swift is super cool!"), nil)
@@ -117,26 +117,23 @@ if #available(SwiftStdlib 6.3, *) {
     // Test the return of demangle into with a smaller buffer.
     // Swift.Int requires 9 bytes, give this 8
     let smolBuffer = UnsafeMutableBufferPointer<UInt8>.allocate(capacity: 8)
-    var outputSpan = OutputSpan<UTF8.CodeUnit>(buffer: smolBuffer, initializedCount: 0)
+    var smolOutputSpan = OutputSpan<UTF8.CodeUnit>(buffer: smolBuffer, initializedCount: 0)
     defer { smolBuffer.deallocate() }
 
-    let smolPtr = smolBuffer.baseAddress!
-
-    let fail = demangle("$sSi".utf8Span, into: &outputSpan)
+    let fail = demangle("$sSi".utf8Span, into: &smolOutputSpan)
     expectEqual(fail, .truncated(9))
-    expectEqual(String(cString: smolPtr), "Swift.In")
+    expectEqual(String(copying: try! UTF8Span(validating: smolOutputSpan.span)), "Swift.In")
   }
 
   DemangleTests.test("Span API - small buffer, success") {
     let smolBuffer = UnsafeMutableBufferPointer<UInt8>.allocate(capacity: 9)
-    var outputSpan = OutputSpan<UTF8.CodeUnit>(buffer: smolBuffer, initializedCount: 0)
+    var smolOutputSpan = OutputSpan<UTF8.CodeUnit>(buffer: smolBuffer, initializedCount: 0)
     defer { smolBuffer.deallocate() }
 
-    let smolPtr = smolBuffer.baseAddress!
     // Test nil return on successful demangle.
-    let success = demangle("$s4Smol3IntV".utf8Span, into: &outputSpan)
+    let success = demangle("$s4Smol3IntV".utf8Span, into: &smolOutputSpan)
     expectEqual(success, .success)
-    expectEqual(String(cString: smolPtr), "Smol.Int")
+    expectEqual(String(copying: try! UTF8Span(validating: smolOutputSpan.span)), "Smol.Int")
   }
 
 }