Skip to content

Commit d8abd2f

Browse files
committed
runtime: Fix overflow of swift_unownedRetain reference counts
On 32bit platforms there are 7 bits reserved for the unowned retain count. This makes overflow a likely scenario. Implement overflow into the side table. rdar://33495003
1 parent 17ddc58 commit d8abd2f

File tree

6 files changed

+100
-7
lines changed

6 files changed

+100
-7
lines changed

Diff for: include/swift/Runtime/Debug.h

+4
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,10 @@ void swift_abortRetainOverflow();
124124
LLVM_ATTRIBUTE_NORETURN LLVM_ATTRIBUTE_NOINLINE
125125
void swift_abortRetainUnowned(const void *object);
126126

127+
// Halt due to an overflow in swift_unownedRetain().
128+
LLVM_ATTRIBUTE_NORETURN LLVM_ATTRIBUTE_NOINLINE
129+
void swift_abortUnownedRetainOverflow();
130+
127131
/// This function dumps one line of a stack trace. It is assumed that \p framePC
128132
/// is the address of the stack frame at index \p index. If \p shortOutput is
129133
/// true, this functions prints only the name of the symbol and offset, ignores

Diff for: stdlib/public/SwiftShims/RefCount.h

+20-7
Original file line numberDiff line numberDiff line change
@@ -564,10 +564,12 @@ class RefCountBitsT {
564564
else
565565
return doDecrementStrongExtraRefCount<DontClearPinnedFlag>(dec);
566566
}
567-
567+
// Returns the old reference count before the increment.
568568
LLVM_ATTRIBUTE_ALWAYS_INLINE
569-
void incrementUnownedRefCount(uint32_t inc) {
570-
setUnownedRefCount(getUnownedRefCount() + inc);
569+
uint32_t incrementUnownedRefCount(uint32_t inc) {
570+
uint32_t old = getUnownedRefCount();
571+
setUnownedRefCount(old + inc);
572+
return old;
571573
}
572574

573575
LLVM_ATTRIBUTE_ALWAYS_INLINE
@@ -757,6 +759,9 @@ class RefCounts {
757759
LLVM_ATTRIBUTE_NOINLINE
758760
bool tryIncrementNonAtomicSlow(RefCountBits oldbits);
759761

762+
LLVM_ATTRIBUTE_NOINLINE
763+
void incrementUnownedSlow(uint32_t inc);
764+
760765
public:
761766
enum Initialized_t { Initialized };
762767

@@ -1139,8 +1144,12 @@ class RefCounts {
11391144

11401145
newbits = oldbits;
11411146
assert(newbits.getUnownedRefCount() != 0);
1142-
newbits.incrementUnownedRefCount(inc);
1143-
// FIXME: overflow check?
1147+
uint32_t oldValue = newbits.incrementUnownedRefCount(inc);
1148+
1149+
// Check overflow and use the side table on overflow.
1150+
if (newbits.getUnownedRefCount() != oldValue + inc)
1151+
return incrementUnownedSlow(inc);
1152+
11441153
} while (!refCounts.compare_exchange_weak(oldbits, newbits,
11451154
std::memory_order_relaxed));
11461155
}
@@ -1152,8 +1161,12 @@ class RefCounts {
11521161

11531162
auto newbits = oldbits;
11541163
assert(newbits.getUnownedRefCount() != 0);
1155-
newbits.incrementUnownedRefCount(inc);
1156-
// FIXME: overflow check?
1164+
uint32_t oldValue = newbits.incrementUnownedRefCount(inc);
1165+
1166+
// Check overflow and use the side table on overflow.
1167+
if (newbits.getUnownedRefCount() != oldValue + inc)
1168+
return incrementUnownedSlow(inc);
1169+
11571170
refCounts.store(newbits, std::memory_order_relaxed);
11581171
}
11591172

Diff for: stdlib/public/runtime/Errors.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,13 @@ void swift::swift_abortRetainOverflow() {
339339
"fatal error: object was retained too many times");
340340
}
341341

342+
// Crash due to an unowned retain count overflow.
343+
// FIXME: can't pass the object's address from InlineRefCounts without hacks
344+
void swift::swift_abortUnownedRetainOverflow() {
345+
swift::fatalError(FatalErrorFlags::ReportBacktrace,
346+
"fatal error: object's unowned reference was retained too many times");
347+
}
348+
342349
// Crash due to retain of a dead unowned reference.
343350
// FIXME: can't pass the object's address from InlineRefCounts without hacks
344351
void swift::swift_abortRetainUnowned(const void *object) {

Diff for: stdlib/public/runtime/RefCount.cpp

+16
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,22 @@ HeapObjectSideTableEntry* RefCounts<InlineRefCountBits>::formWeakReference()
143143
return nullptr;
144144
}
145145

146+
template <typename RefCountBits>
147+
void RefCounts<RefCountBits>::incrementUnownedSlow(uint32_t n) {
148+
auto side = allocateSideTable();
149+
if (side)
150+
return side->incrementUnowned(n);
151+
// Overflow but side table allocation failed.
152+
swift_abortUnownedRetainOverflow();
153+
}
154+
155+
template void RefCounts<InlineRefCountBits>::incrementUnownedSlow(uint32_t n);
156+
template <>
157+
void RefCounts<SideTableRefCountBits>::incrementUnownedSlow(uint32_t n) {
158+
// Overflow from side table to a new side table?!
159+
swift_abortUnownedRetainOverflow();
160+
}
161+
146162
// namespace swift
147163
} // namespace swift
148164

Diff for: test/Interpreter/unowned_overflow.swift

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// RUN: %target-run-simple-swift | %FileCheck %s
2+
// REQUIRES: executable_test
3+
4+
class Owner {
5+
var children: [Child] = []
6+
7+
func addChild(_ c: Child) {
8+
children.append(c)
9+
}
10+
11+
func removeChildren() {
12+
children.removeAll()
13+
}
14+
15+
func test() {
16+
// Overflow of unowned ref count on 32bit.
17+
for _ in 0 ..< 500 {
18+
addChild(Child(self))
19+
}
20+
removeChildren()
21+
}
22+
}
23+
24+
class Child {
25+
unowned var owner: Owner
26+
27+
init(_ o: Owner) {
28+
owner = o
29+
}
30+
}
31+
32+
let o = Owner()
33+
o.test()
34+
print("success")
35+
// CHECK: success

Diff for: unittests/runtime/Refcounting.cpp

+18
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,24 @@ TEST(RefcountingTest, unowned_retain_release_n) {
153153
EXPECT_EQ(1u, value);
154154
}
155155

156+
TEST(RefcountingTest, unowned_retain_release_n_overflow) {
157+
// This test would test overflow on 32bit platforms.
158+
// These platforms have 7 unowned reference count bits.
159+
size_t value = 0;
160+
auto object = allocTestObject(&value, 1);
161+
EXPECT_EQ(0u, value);
162+
swift_unownedRetain_n(object, 128);
163+
EXPECT_EQ(129u, swift_unownedRetainCount(object));
164+
swift_unownedRetain(object);
165+
EXPECT_EQ(130u, swift_unownedRetainCount(object));
166+
swift_unownedRelease_n(object, 128);
167+
EXPECT_EQ(2u, swift_unownedRetainCount(object));
168+
swift_unownedRelease(object);
169+
EXPECT_EQ(1u, swift_unownedRetainCount(object));
170+
swift_release(object);
171+
EXPECT_EQ(1u, value);
172+
}
173+
156174
TEST(RefcountingTest, isUniquelyReferenced) {
157175
size_t value = 0;
158176
auto object = allocTestObject(&value, 1);

0 commit comments

Comments
 (0)