Skip to content
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

[SR-6848] Protocol conformance registration on Linux leaks in Swift 4.1 #49397

Open
Lukasa opened this issue Jan 26, 2018 · 12 comments
Open

[SR-6848] Protocol conformance registration on Linux leaks in Swift 4.1 #49397

Lukasa opened this issue Jan 26, 2018 · 12 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself memory leak bug: Memory leak runtime The Swift Runtime

Comments

@Lukasa
Copy link
Contributor

Lukasa commented Jan 26, 2018

Previous ID SR-6848
Radar rdar://problem/36909854
Original Reporter @Lukasa
Type Bug
Environment

Linux Ubuntu 16.04
Swift 4.1 Nightly from the 25th of January

Swift version 4.1-dev (LLVM 4d75b446be, Clang 745240d98f, Swift 49734173b3)
Target: x86_64-unknown-linux-gnu
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, Leak, Runtime
Assignee None
Priority Medium

md5: 3e2f791cf990fa92581d802dccce573e

Issue Description:

Spotted while attempting to fuzz test some Swift projects.

It seems that something in protocol conformance is leaking on the Swift nightly builds. At the very least a trivial program is capable of leaking the protocol conformances. Compiling the following program with ASAN on Linux immediately shows a leak:

import Foundation

func sum<T: Sequence>(_ iter: T) -> Int where T.Element == UInt8 {
    var val = 0
    for element in iter {
        val += Int(element)
    }
    return val
}

let data = Data(base64Encoded: "UE9TVCAvIEhUVFAvMS4wDQpDb250ZW50LUxlbmd0aDoNCg0K")!
print(sum(data))

Compiling with ASAN (swiftc -sanitize=address test.swift) from one of the recent 4.1 nightlies and then running gives this output:

=================================================================
==5946==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #&#8203;0 0x55badb344473  (/home/cbenfield/test+0x9e473)
    #&#8203;1 0x7f29f0a16e77  (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x8de77)

SUMMARY: AddressSanitizer: 32 byte(s) leaked in 1 allocation(s).

Removing ASAN and using Valgrind gives us a more useful stack trace:

==5968== 32 bytes in 1 blocks are definitely lost in loss record 4 of 28
==5968==    at 0x4C2E0EF: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==5968==    by 0x51E3781: _registerProtocolConformances(ConformanceState&, swift::TargetProtocolConformanceRecord<swift::InProcess> const*, swift::TargetProtocolConformanceRecord<swift::InProcess> const*) (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libswiftCore.so)
==5968==    by 0x521AAC9: swift_addNewDSOImage (in /usr/local/swift/swift-4.1-DEVELOPMENT-SNAPSHOT-2018-01-25-a-ubuntu16.04/usr/lib/swift/linux/libswiftCore.so)
==5968==    by 0x40106B9: call_init.part.0 (dl-init.c:72)
==5968==    by 0x40107CA: call_init (dl-init.c:30)
==5968==    by 0x40107CA: _dl_init (dl-init.c:120)
==5968==    by 0x4000C69: ??? (in /lib/x86_64-linux-gnu/ld-2.23.so)

This is not the only leak here, I have found others, I just don't yet have small reproducers for them.

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

At the request of @alblue I tried a couple of tests to narrow things down. Adding an extra call to sum with a [UInt8] does not leak extra memory. Neither does looping around the code.

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

In fact I have a simpler repro. The following Swift file also leaks in the same way:

import Foundation

let data = Data(base64Encoded: "UE9TVCAvIEhUVFAvMS4wDQpDb250ZW50LUxlbmd0aDoNCg0K")!
print(data.count)

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

Some bisects on the Swift 4.1 nightlies reveal that swift-4.1-DEVELOPMENT-SNAPSHOT-2017-12-04 is fine, and swift-4.1-DEVELOPMENT-SNAPSHOT-2017-12-07 fails. So the bad change was introduced in that window.

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

Ok, let's remove Foundation from our list of possible culprits, as the following definitely minimal repro also exhibits the same leak:

protocol P { }
let x: Any = 1
let y = x as? P

I think at this point we can call this either a compiler or standard library bug.

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

@swift-ci create

@DougGregor
Copy link
Member

The runtime does need to allocate memory in _registerProtocolProtocols to track the sections in which it can find protocol conformances. That'll happen the first time one dynamically queries conformances (e.g., triggered here by the "as? P"), as well as any time one dlopen's another shared library. We hold on to the memory forever because we may need to rescan these sections later on. The actual allocations here should be very few, so if we're seeing a lot of growth here that would be a bug. However, it looks like something where we need to whitelist these allocations as intentionally never freed.

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

@DougGregor Are you sure that's what's happening here?

Both valgrind and ASAN are adamant that the allocations are entirely lost: that is, they're heap-allocated and there are no pointers remaining to them. That's not quite the same as "allocating but never freeing": it's "allocating and then losing".

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

Put another way: older versions of the runtime on Linux do not trip this issue, and the Darwin runtime does not either.

@DougGregor
Copy link
Member

Oh, they're entirely lost. That is indeed bad!

@DougGregor
Copy link
Member

And now I see the "definitely lost"; sorry for jumping the gun here.

@Lukasa
Copy link
Contributor Author

Lukasa commented Jan 26, 2018

No problem, it took me a while to convince myself this was a real problem too.

FWIW, I should stress that this isn't a particularly high-urgency leak, unlike SR-6849. The size of the leak in this case is small: in all my tests I've never seen it leak more than 64 bytes. This doesn't pose a threat to any running programs, obviously![]( It mostly just makes ASAN tricky to use on Linux, which is a bit sad. The Swift team has done some great work to bring sanitizers to the Linux platform, I'd really like people to be able to use them effectively)

@DougGregor
Copy link
Member

This bit of code over at https://github.com/apple/swift/blob/master/stdlib/public/runtime/ProtocolConformance.cpp#L291-L293 is potential suspicious:

  // Conformance cache should always be sufficiently initialized by this point.
  _registerProtocolConformances(Conformances.unsafeGetAlreadyInitialized(),
                                recordsBegin, recordsEnd);

It's plausible that, with ELF, we're getting into this code (addImageProtocolConformanceBlockCallback) before the Conformances table gets initialized. I'm not set up to easily test this at the moment, but I'd want to try changing this to:

// Conformance cache should always be sufficiently initialized by this point.
  _registerProtocolConformances(Conformances.get(), recordsBegin, recordsEnd);

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself memory leak bug: Memory leak runtime The Swift Runtime
Projects
None yet
Development

No branches or pull requests

2 participants