Skip to content

[embedded] Fix a memory leak caused by incorrect refcounting logic around doNotFreeBit #77121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/IRGen/GenConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,8 @@ llvm::Constant *irgen::emitConstantObject(IRGenModule &IGM, ObjectInst *OI,
if (IGM.canMakeStaticObjectReadOnly(OI->getType())) {
if (!IGM.swiftImmortalRefCount) {
if (IGM.Context.LangOpts.hasFeature(Feature::Embedded)) {
// = HeapObject.immortalRefCount
// = HeapObject.immortalRefCount | HeapObject.doNotFreeBit
// 0xffff_ffff on 32-bit, 0xffff_ffff_ffff_ffff on 64-bit
IGM.swiftImmortalRefCount = llvm::ConstantInt::get(IGM.IntPtrTy, -1);
} else {
IGM.swiftImmortalRefCount = llvm::ConstantExpr::getPtrToInt(
Expand Down
29 changes: 16 additions & 13 deletions stdlib/public/core/EmbeddedRuntime.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,21 @@ public struct HeapObject {
// to think about supporting (or banning) weak and/or unowned references.
var refcount: Int

// Note: The immortalRefCount value is also hard-coded in IRGen in `irgen::emitConstantObject`.
#if _pointerBitWidth(_64)
static let doNotFreeBit = Int(bitPattern: 0x8000_0000_0000_0000)
static let refcountMask = Int(bitPattern: 0x7fff_ffff_ffff_ffff)
static let doNotFreeBit = Int(bitPattern: 0x8000_0000_0000_0000)
static let refcountMask = Int(bitPattern: 0x7fff_ffff_ffff_ffff)
static let immortalRefCount = Int(bitPattern: 0x7fff_ffff_ffff_ffff) // Make sure we don't have doNotFreeBit set
#elseif _pointerBitWidth(_32)
static let doNotFreeBit = Int(bitPattern: 0x8000_0000)
static let refcountMask = Int(bitPattern: 0x7fff_ffff)
static let doNotFreeBit = Int(bitPattern: 0x8000_0000)
static let refcountMask = Int(bitPattern: 0x7fff_ffff)
static let immortalRefCount = Int(bitPattern: 0x7fff_ffff) // Make sure we don't have doNotFreeBit set
#elseif _pointerBitWidth(_16)
static let doNotFreeBit = Int(bitPattern: 0x8000)
static let refcountMask = Int(bitPattern: 0x7fff)
static let doNotFreeBit = Int(bitPattern: 0x8000)
static let refcountMask = Int(bitPattern: 0x7fff)
static let immortalRefCount = Int(bitPattern: 0x7fff) // Make sure we don't have doNotFreeBit set
#endif

// Note: The immortalRefCount value of -1 is also hard-coded in IRGen in `irgen::emitConstantObject`.
static let immortalRefCount = -1

#if _pointerBitWidth(_64)
static let immortalObjectPointerBit = UInt(0x8000_0000_0000_0000)
#endif
Expand Down Expand Up @@ -158,7 +159,7 @@ public func swift_initStaticObject(metadata: Builtin.RawPointer, object: Builtin

func swift_initStaticObject(metadata: UnsafeMutablePointer<ClassMetadata>, object: UnsafeMutablePointer<HeapObject>) -> UnsafeMutablePointer<HeapObject> {
_swift_embedded_set_heap_object_metadata_pointer(object, metadata)
object.pointee.refcount = HeapObject.immortalRefCount
object.pointee.refcount = HeapObject.immortalRefCount | HeapObject.doNotFreeBit
return object
}

Expand Down Expand Up @@ -251,7 +252,7 @@ public func swift_retain_n(object: Builtin.RawPointer, n: UInt32) -> Builtin.Raw

func swift_retain_n_(object: UnsafeMutablePointer<HeapObject>, n: UInt32) -> UnsafeMutablePointer<HeapObject> {
let refcount = refcountPointer(for: object)
if loadRelaxed(refcount) == HeapObject.immortalRefCount {
if loadRelaxed(refcount) & HeapObject.refcountMask == HeapObject.immortalRefCount {
return object
}

Expand Down Expand Up @@ -294,7 +295,8 @@ func swift_release_n_(object: UnsafeMutablePointer<HeapObject>?, n: UInt32) {
}

let refcount = refcountPointer(for: object)
if loadRelaxed(refcount) == HeapObject.immortalRefCount {
let loadedRefcount = loadRelaxed(refcount)
if loadedRefcount & HeapObject.refcountMask == HeapObject.immortalRefCount {
return
}

Expand All @@ -309,7 +311,8 @@ func swift_release_n_(object: UnsafeMutablePointer<HeapObject>?, n: UInt32) {
// There can only be one thread with a reference at this point (because
// we're releasing the last existing reference), so a relaxed store is
// enough.
storeRelaxed(refcount, newValue: HeapObject.immortalRefCount)
let doNotFree = (loadedRefcount & HeapObject.doNotFreeBit) != 0
storeRelaxed(refcount, newValue: HeapObject.immortalRefCount | (doNotFree ? HeapObject.doNotFreeBit : 0))

_swift_embedded_invoke_heap_object_destroy(object)
} else if resultingRefcount < 0 {
Expand Down
38 changes: 38 additions & 0 deletions test/embedded/Inputs/debug-malloc.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#include <stdlib.h>
#include <stdio.h>

#define HEAP_SIZE (8 * 1024)

__attribute__((aligned(16)))
char heap[HEAP_SIZE] = {};

size_t next_heap_index = 0;

void *calloc(size_t count, size_t size) {
printf("malloc(%ld)", count);

if (next_heap_index + count * size > HEAP_SIZE) {
puts("HEAP EXHAUSTED\n");
__builtin_trap();
}
void *p = &heap[next_heap_index];
next_heap_index += count * size;
printf("-> %p\n", p);
return p;
}

void *malloc(size_t count) {
return calloc(count, 1);
}

int posix_memalign(void **memptr, size_t alignment, size_t size) {
*memptr = calloc(size + alignment, 1);
if (((uintptr_t)*memptr) % alignment == 0) return 0;
*(uintptr_t *)memptr += alignment - ((uintptr_t)*memptr % alignment);
return 0;
}

void free(void *ptr) {
// don't actually free
printf("free(%p)\n", ptr);
}
35 changes: 35 additions & 0 deletions test/embedded/refcounting-leak.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -enable-experimental-feature Embedded -parse-as-library %s -c -o %t/a.o
// RUN: %target-clang -x c -std=c11 -c %S/Inputs/debug-malloc.c -o %t/debug-malloc.o
// RUN: %target-clang %t/a.o %t/debug-malloc.o -o %t/a.out
// RUN: %target-run %t/a.out | %FileCheck %s

// REQUIRES: executable_test
// REQUIRES: swift_in_compiler
// REQUIRES: optimized_stdlib
// REQUIRES: OS=macosx

var x: UInt = 1
func randomNumber() -> UInt {
x = ((x >> 16) ^ x) &* 0x45d9f3b
x = ((x >> 16) ^ x) &* 0x45d9f3b
x = (x >> 16) ^ x
return x
}

@main
struct Main {
static func main() {
for _ in 0 ..< 3 {
_ = randomNumber().description
}
print("OK!")
// CHECK: malloc
// CHECK: free
// CHECK: malloc
// CHECK: free
// CHECK: malloc
// CHECK: free
// CHECK: OK!
}
}