Skip to content

Commit b6eef4f

Browse files
aschwaighoferaschwaighofer
authored
Merge pull request #12312 from aschwaighofer/fix_hashvalue_leak
runtime: Move String implementation stubs that want need the auto-rel…
2 parents a203aee + f9c0049 commit b6eef4f

File tree

5 files changed

+244
-35
lines changed

5 files changed

+244
-35
lines changed

‎stdlib/public/stubs/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ set(swift_stubs_objc_sources
1111
FoundationHelpers.mm
1212
OptionalBridgingHelper.mm
1313
Reflection.mm
14+
SwiftNativeNSXXXBaseARC.m
1415
SwiftNativeNSXXXBase.mm.gyb)
1516
set(swift_stubs_unicode_normalization_sources
1617
UnicodeNormalization.cpp)
@@ -41,3 +42,7 @@ add_swift_library(swiftStdlibStubs OBJECT_LIBRARY TARGET_LIBRARY
4142
LINK_FLAGS ${SWIFT_RUNTIME_CORE_LINK_FLAGS}
4243
INSTALL_IN_COMPONENT stdlib)
4344

45+
if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
46+
set_property(SOURCE SwiftNativeNSXXXBaseARC.m APPEND_STRING PROPERTY COMPILE_FLAGS
47+
"-fobjc-arc")
48+
endif()

‎stdlib/public/stubs/SwiftNativeNSXXXBase.mm.gyb

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -143,25 +143,6 @@ swift_stdlib_CFStringHashCString(const uint8_t *bytes, CFIndex len) {
143143
return Result;
144144
}
145145

146-
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
147-
size_t
148-
swift_stdlib_NSStringHashValue(NSString *NS_RELEASES_ARGUMENT str,
149-
bool isASCII) {
150-
size_t Result =
151-
isASCII ? str.hash : str.decomposedStringWithCanonicalMapping.hash;
152-
153-
swift_unknownRelease(str);
154-
return Result;
155-
}
156-
157-
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
158-
size_t
159-
swift_stdlib_NSStringHashValuePointer(void *opaque, bool isASCII) {
160-
NSString *str = (NSString *)opaque;
161-
return isASCII ? str.hash : str.decomposedStringWithCanonicalMapping.hash;
162-
}
163-
164-
165146
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
166147
bool swift_stdlib_NSStringHasPrefixNFD(NSString *theString,
167148
NSString *prefix) {
@@ -211,22 +192,6 @@ bool swift_stdlib_NSStringHasSuffixNFDPointer(void *theString,
211192
return Result;
212193
}
213194

214-
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
215-
NS_RETURNS_RETAINED NSString *
216-
swift_stdlib_NSStringLowercaseString(NSString *NS_RELEASES_ARGUMENT str) {
217-
NSString *Result = objc_retain(str.lowercaseString);
218-
swift_unknownRelease(str);
219-
return Result;
220-
}
221-
222-
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
223-
NS_RETURNS_RETAINED NSString *
224-
swift_stdlib_NSStringUppercaseString(NSString *NS_RELEASES_ARGUMENT str) {
225-
NSString *Result = objc_retain(str.uppercaseString);
226-
swift_unknownRelease(str);
227-
return Result;
228-
}
229-
230195
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
231196
void swift_stdlib_CFSetGetValues(NSSet *NS_RELEASES_ARGUMENT set,
232197
const void **values) {

‎stdlib/public/stubs/SwiftNativeNSXXXBaseARC.m

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
//===--- SwiftNativeNSXXXBaseARC.mm - Runtime stubs that require ARC ------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
#include "swift/Runtime/Config.h"
13+
14+
#if SWIFT_OBJC_INTEROP
15+
16+
#import <Foundation/Foundation.h>
17+
#import <CoreFoundation/CoreFoundation.h>
18+
#include <objc/NSObject.h>
19+
#include <objc/runtime.h>
20+
#include <objc/objc.h>
21+
22+
// The following two routines need to be implemented in ARC because
23+
// decomposedStringWithCanonicalMapping returns its result autoreleased. And we
24+
// want ARC to insert 'objc_retainAutoreleasedReturnValue' and the necessary
25+
// markers for the hand-off to facilitate the remove from autorelease pool
26+
// optimization such that the object is not handed into the current autorelease
27+
// pool which might be scoped such that repeatedly placing objects into it
28+
// results in unbounded memory growth.
29+
30+
// On i386 the remove from autorelease pool optimization is foiled by the
31+
// decomposedStringWithCanonicalMapping implementation. Instead, we use a local
32+
// autorelease pool to prevent leaking of the temporary object into the callers
33+
// autorelease pool.
34+
#if defined(__i386__)
35+
#define AUTORELEASEPOOL @autoreleasepool
36+
#else
37+
// On other platforms we rely on the remove from autorelease pool optimization.
38+
#define AUTORELEASEPOOL
39+
#endif
40+
41+
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
42+
size_t swift_stdlib_NSStringHashValue(NSString *NS_RELEASES_ARGUMENT str,
43+
bool isASCII) {
44+
AUTORELEASEPOOL {
45+
return isASCII ? str.hash : str.decomposedStringWithCanonicalMapping.hash;
46+
}
47+
}
48+
49+
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
50+
size_t
51+
swift_stdlib_NSStringHashValuePointer(void *opaque, bool isASCII) {
52+
NSString __unsafe_unretained *str =
53+
(__bridge NSString __unsafe_unretained *)opaque;
54+
AUTORELEASEPOOL {
55+
return isASCII ? str.hash : str.decomposedStringWithCanonicalMapping.hash;
56+
}
57+
}
58+
59+
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
60+
NS_RETURNS_RETAINED NSString *
61+
swift_stdlib_NSStringLowercaseString(NSString *NS_RELEASES_ARGUMENT str) {
62+
AUTORELEASEPOOL {
63+
return str.lowercaseString;
64+
}
65+
}
66+
67+
SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERFACE
68+
NS_RETURNS_RETAINED NSString *
69+
swift_stdlib_NSStringUppercaseString(NSString *NS_RELEASES_ARGUMENT str) {
70+
AUTORELEASEPOOL {
71+
return str.uppercaseString;
72+
}
73+
}
74+
75+
#endif

‎test/stdlib/StringMemoryTest.swift

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-build-swift -O %s -o %t/StringMemoryTest
3+
// RUN: %target-run %t/StringMemoryTest | %FileCheck %s
4+
5+
// REQUIRES: optimized_stdlib
6+
// REQUIRES: executable_test
7+
// REQUIRES: objc_interop
8+
9+
import Foundation
10+
11+
let str = "abcdefg\u{A758}hijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz\u{A759}"
12+
let str2 = "abcdefg\u{A759}hijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz\u{A758}"
13+
14+
@inline(never)
15+
func lookup(_ str: String, _ dict: [String: Int]) -> Bool {
16+
if let _ = dict[str] {
17+
return true
18+
}
19+
return false
20+
}
21+
22+
@inline(never)
23+
func uppercase(_ str: String) -> String {
24+
return str.uppercased()
25+
}
26+
27+
@inline(never)
28+
func lowercase(_ str: String) -> String {
29+
return str.lowercased()
30+
}
31+
32+
/// Make sure the hash function does not leak.
33+
34+
let dict = [ "foo" : 1]
35+
for _ in 0 ..< 10_000_000 {
36+
if lookup("\u{1F1E7}\u{1F1E7}", dict) {
37+
print("Found?!")
38+
}
39+
if uppercase(str) == "A" {
40+
print("Found?!")
41+
}
42+
if lowercase(str2) == "A" {
43+
print("Found?!")
44+
}
45+
}
46+
47+
// CHECK-NOT: Found?!
48+
// CHECK: Not found
49+
50+
print("Not found")
51+
52+
var usage = rusage()
53+
getrusage(RUSAGE_SELF, &usage)
54+
55+
// CHECK: success
56+
// CHECK-NOT: failure
57+
58+
// We should not need 50MB for this.
59+
if usage.ru_maxrss > 50 * 1024 * 1024 {
60+
print("failure - should not need 50MB!")
61+
} else {
62+
print("success")
63+
}

‎test/stdlib/runtime_autorelease_optimization.txt

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// REQUIRES: objc_interop
2+
// RUN: otool -tvV %platform-module-dir/libswiftCore.dylib | %FileCheck %s --check-prefix=CHECK-%target-cpu
3+
4+
// Verify the autorelease return optimization sequence.
5+
6+
/// Test x86-64:
7+
8+
// CHECK-x86_64-LABEL: _swift_stdlib_NSStringHashValue:
9+
// CHECK-x86_64-NOT: ret
10+
// CHECK-x86_64: movq {{.*}}(%rip), %rsi ## Objc selector ref: decomposedStringWithCanonicalMapping
11+
// CHECK-x86_64: movq {{.*}}(%rip), [[MSG:%.*]] ## Objc message: -[%rdi decomposedStringWithCanonicalMapping]
12+
// CHECK-x86_64: callq *[[MSG]]
13+
// CHECK-x86_64: movq %rax, %rdi
14+
// CHECK-x86_64: callq {{.*}} ## symbol stub for: _objc_retainAutoreleasedReturnValue
15+
// CHECK-x86_64: ret
16+
17+
18+
// CHECK-x86_64-LABEL: _swift_stdlib_NSStringHashValuePointer:
19+
// CHECK-x86_64-NOT: ret
20+
// CHECK-x86_64: movq {{.*}}(%rip), %rsi ## Objc selector ref: decomposedStringWithCanonicalMapping
21+
// CHECK-x86_64: movq {{.*}}(%rip), [[MSG:%.*]] ## Objc message: -[%rdi decomposedStringWithCanonicalMapping]
22+
// CHECK-x86_64: callq *[[MSG]]
23+
// CHECK-x86_64: movq %rax, %rdi
24+
// CHECK-x86_64: callq {{.*}} ## symbol stub for: _objc_retainAutoreleasedReturnValue
25+
// CHECK-x86_64: ret
26+
27+
/// Test i386:
28+
29+
// CHECK-i386-LABEL: _swift_stdlib_NSStringHashValue:
30+
// CHECK-i386-NOT: ret
31+
// CHECK-i386: calll {{.*}} ## symbol stub for: _objc_msgSend
32+
// CHECK-i386: movl %ebp, %ebp
33+
// CHECK-i386: calll {{.*}} ## symbol stub for: _objc_retainAutoreleasedReturnValue
34+
// CHECK-i386: ret
35+
// CHECK-i386-LABEL: _swift_stdlib_NSStringHashValuePointer:
36+
// CHECK-i386-NOT: ret
37+
// CHECK-i386: calll {{.*}} ## symbol stub for: _objc_msgSend
38+
// CHECK-i386: movl %ebp, %ebp
39+
// CHECK-i386: calll {{.*}} ## symbol stub for: _objc_retainAutoreleasedReturnValue
40+
// CHECK-i386: ret
41+
42+
/// Test armv7:
43+
44+
// CHECK-armv7-LABEL: _swift_stdlib_NSStringHashValue:
45+
// CHECK-armv7-NOT: pop {{.*}}pc{{.*}}
46+
// CHECK-armv7: blx {{.*}} @ symbol stub for: _objc_msgSend
47+
// CHECK-armv7: mov r7, r7
48+
// CHECK-armv7: blx {{.*}} @ symbol stub for: _objc_retainAutoreleasedReturnValue
49+
// CHECK-armv7: pop {{.*}}pc{{.*}}
50+
// CHECK-armv7-LABEL: _swift_stdlib_NSStringHashValuePointer:
51+
// CHECK-armv7-NOT: pop {{.*}}pc{{.*}}
52+
// CHECK-armv7: blx {{.*}} @ symbol stub for: _objc_msgSend
53+
// CHECK-armv7: mov r7, r7
54+
// CHECK-armv7: blx {{.*}} @ symbol stub for: _objc_retainAutoreleasedReturnValue
55+
// CHECK-armv7: pop {{.*}}pc{{.*}}
56+
57+
/// Test armv7s:
58+
59+
// CHECK-armv7s-LABEL: _swift_stdlib_NSStringHashValue:
60+
// CHECK-armv7s-NOT: pop {{.*}}pc{{.*}}
61+
// CHECK-armv7s: blx {{.*}} @ symbol stub for: _objc_msgSend
62+
// CHECK-armv7s: mov r7, r7
63+
// CHECK-armv7s: blx {{.*}} @ symbol stub for: _objc_retainAutoreleasedReturnValue
64+
// CHECK-armv7s: pop {{.*}}pc{{.*}}
65+
// CHECK-armv7s-LABEL: _swift_stdlib_NSStringHashValuePointer:
66+
// CHECK-armv7s-NOT: pop {{.*}}pc{{.*}}
67+
// CHECK-armv7s: blx {{.*}} @ symbol stub for: _objc_msgSend
68+
// CHECK-armv7s: mov r7, r7
69+
// CHECK-armv7s: blx {{.*}} @ symbol stub for: _objc_retainAutoreleasedReturnValue
70+
// CHECK-armv7s: pop {{.*}}pc{{.*}}
71+
72+
73+
/// Test armv7k:
74+
75+
// CHECK-armv7k-LABEL: _swift_stdlib_NSStringHashValue:
76+
// CHECK-armv7k-NOT: pop {{.*}}pc{{.*}}
77+
// CHECK-armv7k: blx {{.*}} @ symbol stub for: _objc_msgSend
78+
// CHECK-armv7k: mov r7, r7
79+
// CHECK-armv7k: blx {{.*}} @ symbol stub for: _objc_retainAutoreleasedReturnValue
80+
// CHECK-armv7k: pop {{.*}}pc{{.*}}
81+
// CHECK-armv7k-LABEL: _swift_stdlib_NSStringHashValuePointer:
82+
// CHECK-armv7k-NOT: pop {{.*}}pc{{.*}}
83+
// CHECK-armv7k: blx {{.*}} @ symbol stub for: _objc_msgSend
84+
// CHECK-armv7k: mov r7, r7
85+
// CHECK-armv7k: blx {{.*}} @ symbol stub for: _objc_retainAutoreleasedReturnValue
86+
// CHECK-armv7k: pop {{.*}}pc{{.*}}
87+
88+
/// Test arm64:
89+
90+
// CHECK-arm64-LABEL: _swift_stdlib_NSStringHashValue:
91+
// CHECK-arm64-NOT: ret
92+
// CHECK-arm64: bl {{.*}} ; Objc message: -[x0 decomposedStringWithCanonicalMapping]
93+
// CHECK-arm64: mov x29, x29
94+
// CHECK-arm64: bl {{.*}} ; symbol stub for: _objc_retainAutoreleasedReturnValue
95+
// CHECK-arm64: ret
96+
// CHECK-arm64-LABEL: _swift_stdlib_NSStringHashValuePointer:
97+
// CHECK-arm64-NOT: ret
98+
// CHECK-arm64: bl {{.*}} ; Objc message: -[x0 decomposedStringWithCanonicalMapping]
99+
// CHECK-arm64: mov x29, x29
100+
// CHECK-arm64: bl {{.*}} ; symbol stub for: _objc_retainAutoreleasedReturnValue
101+
// CHECK-arm64: ret

0 commit comments

Comments
 (0)