From 2806354ec5293c79c4b2225e0d6418ebeaf540fb Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Mon, 15 Aug 2022 23:33:18 -0400 Subject: [PATCH] Set detect_leaks=1 in various places where we do ASAN builds in CI. (#20913) This is a relanding of https://github.com/project-chip/connectedhomeip/pull/20246 --- .github/workflows/build.yaml | 3 ++ .github/workflows/darwin-tests.yaml | 3 +- .github/workflows/tests.yaml | 2 ++ .../commands/common/CHIPToolKeypair.mm | 31 ++++++++++------ .../tests/chiptest/lsan-mac-suppressions.txt | 35 +++++++++++++++++++ 5 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 scripts/tests/chiptest/lsan-mac-suppressions.txt diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 4d85f92b4521cd..ecef4f5a31da24 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -377,6 +377,9 @@ jobs: scripts/run_in_build_env.sh "ninja -C ./out/$BUILD_TYPE" - name: Setup Build, Run Build and Run Tests timeout-minutes: 120 + # We can't enable leak checking here in LSAN_OPTIONS, because on + # Darwin that's only supported with a new enough clang, and we're + # not building with the pigweed clang here. run: | for BUILD_TYPE in default python_lib; do case $BUILD_TYPE in diff --git a/.github/workflows/darwin-tests.yaml b/.github/workflows/darwin-tests.yaml index eb8106f6b26fa6..82315575292a49 100644 --- a/.github/workflows/darwin-tests.yaml +++ b/.github/workflows/darwin-tests.yaml @@ -32,9 +32,10 @@ jobs: strategy: matrix: - build_variant: [no-ble-asan] + build_variant: [no-ble-asan-clang] env: BUILD_VARIANT: ${{matrix.build_variant}} + LSAN_OPTIONS: detect_leaks=1 malloc_context_size=40 suppressions=scripts/tests/chiptest/lsan-mac-suppressions.txt if: github.actor != 'restyled-io[bot]' runs-on: macos-latest diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 835b5b27415cd5..f17293a19f381d 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -38,6 +38,7 @@ jobs: BUILD_VARIANT: ${{matrix.build_variant}} CHIP_TOOL_VARIANT: ${{matrix.chip_tool}} TSAN_OPTIONS: "halt_on_error=1 suppressions=scripts/tests/chiptest/tsan-linux-suppressions.txt" + LSAN_OPTIONS: detect_leaks=1 if: github.actor != 'restyled-io[bot]' runs-on: ubuntu-latest @@ -132,6 +133,7 @@ jobs: BUILD_VARIANT: ${{matrix.build_variant}} CHIP_TOOL_VARIANT: ${{matrix.chip_tool}} TSAN_OPTIONS: "halt_on_error=1" + LSAN_OPTIONS: detect_leaks=1 suppressions=scripts/tests/chiptest/lsan-mac-suppressions.txt if: github.actor != 'restyled-io[bot]' runs-on: macos-latest diff --git a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm index 425a45fba62f83..6c60345fc973a1 100644 --- a/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm +++ b/examples/darwin-framework-tool/commands/common/CHIPToolKeypair.mm @@ -17,6 +17,7 @@ @interface CHIPToolKeypair () @property (nonatomic) chip::Crypto::P256Keypair mIssuer; @property (nonatomic) NSData * ipk; @property (atomic) uint32_t mNow; +@property (nonatomic, readonly) SecKeyRef mPublicKey; @end @implementation CHIPToolKeypair @@ -46,16 +47,19 @@ - (NSData *)signMessageECDSA_RAW:(NSData *)message - (SecKeyRef)publicKey { - chip::Crypto::P256PublicKey publicKey = _mKeyPair.Pubkey(); - NSData * publicKeyNSData = [NSData dataWithBytes:publicKey.Bytes() length:publicKey.Length()]; - NSDictionary * attributes = @{ - (__bridge NSString *) kSecAttrKeyClass : (__bridge NSString *) kSecAttrKeyClassPublic, - (NSString *) kSecAttrKeyType : (NSString *) kSecAttrKeyTypeECSECPrimeRandom, - (NSString *) kSecAttrKeySizeInBits : @Public_KeySize, - (NSString *) kSecAttrLabel : kCHIPToolKeychainLabel, - (NSString *) kSecAttrApplicationTag : @CHIPPlugin_CAKeyTag, - }; - return SecKeyCreateWithData((__bridge CFDataRef) publicKeyNSData, (__bridge CFDictionaryRef) attributes, nullptr); + if (_mPublicKey == nil) { + chip::Crypto::P256PublicKey publicKey = _mKeyPair.Pubkey(); + NSData * publicKeyNSData = [NSData dataWithBytes:publicKey.Bytes() length:publicKey.Length()]; + NSDictionary * attributes = @{ + (__bridge NSString *) kSecAttrKeyClass : (__bridge NSString *) kSecAttrKeyClassPublic, + (NSString *) kSecAttrKeyType : (NSString *) kSecAttrKeyTypeECSECPrimeRandom, + (NSString *) kSecAttrKeySizeInBits : @Public_KeySize, + (NSString *) kSecAttrLabel : kCHIPToolKeychainLabel, + (NSString *) kSecAttrApplicationTag : @CHIPPlugin_CAKeyTag, + }; + _mPublicKey = SecKeyCreateWithData((__bridge CFDataRef) publicKeyNSData, (__bridge CFDictionaryRef) attributes, nullptr); + } + return _mPublicKey; } - (CHIP_ERROR)Deserialize:(chip::Crypto::P256SerializedKeypair &)input @@ -136,4 +140,11 @@ - (CHIP_ERROR)initSerializedKeyFromValue:(NSData *)value serializedKey:(chip::Cr return CHIP_NO_ERROR; } +- (void)dealloc +{ + if (_mPublicKey) { + CFRelease(_mPublicKey); + } +} + @end diff --git a/scripts/tests/chiptest/lsan-mac-suppressions.txt b/scripts/tests/chiptest/lsan-mac-suppressions.txt new file mode 100644 index 00000000000000..68ac691dec325d --- /dev/null +++ b/scripts/tests/chiptest/lsan-mac-suppressions.txt @@ -0,0 +1,35 @@ +# Looks like some Objective C class bits are leaked, which is probably OK since +# those are singletons. +leak:realizeClassWithoutSwift +leak:objc_initializeClassPair_internal + +# TODO: Under [NSManagedObjectContext executeFetchRequest] there are managed object bits that seem to be leaky. +leak:class_addMethod +leak:class_addIvar + +# TODO: Leaks of blocks from dispatch source handlers that need to be investigated. +leak:_Block_copy + +# TODO: OpenSSL random byte generation creates some sort of pools that we seem to never clean up. This seems to be happening a _lot_. +leak:drbg_ctr_init +leak:rand_pool_new +leak:RAND_priv_bytes +leak:drbg_bytes + +# TODO: OpenSSL ERR_get_state seems to leak. +leak:ERR_get_state + +# TODO: BLE initialization allocates some UUIDs and strings that seem to leak. +leak:[BleConnection initWithDiscriminator:] +leak:[CBXpcConnection initWithDelegate:queue:options:sessionType:] + +# TODO: ClusterBase::SubscribeAttribute creates a subscription that cannot +# actually be shut down in any way. +leak:ClusterBase::SubscribeAttribute + +# TODO: Figure out how we are managing to leak NSData while using ARC! +leak:[CHIPToolKeypair signMessageECDSA_RAW:] + +# TODO: Figure out how we are managing to leak NSData while using ARC, though +# this may just be a leak deep inside the CFPreferences stuff somewhere. +leak:[CHIPToolPersistentStorageDelegate storageDataForKey:]