From 1d80bc309f0ad44a12a506878d82f0c5fc0d5872 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Fri, 9 Dec 2022 14:27:23 +0100 Subject: [PATCH] Add ubsan build variant to build_examples script (#23953) * Add ubsan build variant to build_examples script * Fix warnings reported by gcc when compiling with UBSAN * Build Tizen chip-toll-ubsan on CI to verify non-UB * Update targets testdata for Linux x64 --- .github/workflows/examples-tizen.yaml | 2 +- build/config/compiler/BUILD.gn | 14 +++++++++----- scripts/build/build/targets.py | 3 +++ scripts/build/builders/host.py | 5 ++++- scripts/build/builders/tizen.py | 3 +++ scripts/build/testdata/all_targets_linux_x64.txt | 6 +++--- src/ble/BleLayer.cpp | 2 +- src/ble/CHIPBleServiceData.h | 4 ++-- src/lib/asn1/ASN1Reader.cpp | 6 +++--- src/lib/asn1/ASN1Writer.cpp | 6 +++--- src/lib/core/CHIPKeyIds.h | 15 ++++++++++++--- src/lib/core/CHIPTLVTypes.h | 2 +- src/lib/support/Base64.cpp | 6 +++--- src/lib/support/BufferWriter.cpp | 6 ++---- src/lib/support/SerializableIntegerSet.cpp | 2 +- src/setup_payload/QRCodeSetupPayloadGenerator.cpp | 3 ++- 16 files changed, 53 insertions(+), 32 deletions(-) diff --git a/.github/workflows/examples-tizen.yaml b/.github/workflows/examples-tizen.yaml index dfd53af82bcaa6..259733a008237c 100644 --- a/.github/workflows/examples-tizen.yaml +++ b/.github/workflows/examples-tizen.yaml @@ -61,7 +61,7 @@ jobs: --enable-flashbundle \ --target tizen-arm-all-clusters \ --target tizen-arm-all-clusters-minimal-no-wifi \ - --target chip-tool \ + --target chip-tool-ubsan \ --target light \ --target light-no-ble-no-wifi \ build \ diff --git a/build/config/compiler/BUILD.gn b/build/config/compiler/BUILD.gn index 93276a4666fa27..f00770a3c6e8c6 100644 --- a/build/config/compiler/BUILD.gn +++ b/build/config/compiler/BUILD.gn @@ -385,7 +385,6 @@ config("sanitize_address") { config("sanitize_thread") { cflags = [ "-fsanitize=thread" ] - ldflags = cflags } @@ -403,11 +402,16 @@ config("sanitize_undefined_behavior") { "-fvisibility=hidden", "-fsanitize=undefined", "-fsanitize=float-divide-by-zero", - "-fsanitize=unsigned-integer-overflow", - "-fsanitize=implicit-conversion", - "-fsanitize=nullability", - "-fno-sanitize=vptr,function", ] + if (is_clang) { + cflags += [ + "-fsanitize=unsigned-integer-overflow", + "-fsanitize=implicit-conversion", + "-fsanitize=nullability", + "-fno-sanitize=vptr,function", + ] + } + ldflags = cflags } diff --git a/scripts/build/build/targets.py b/scripts/build/build/targets.py index a7d024b643ad8a..bb0a41cd08636c 100755 --- a/scripts/build/build/targets.py +++ b/scripts/build/build/targets.py @@ -71,6 +71,7 @@ def BuildHostFakeTarget(): target.AppendModifier("boringssl", crypto_library=HostCryptoLibrary.BORINGSSL).ExceptIfRe('-boringssl') target.AppendModifier("asan", use_asan=True).ExceptIfRe("-tsan") target.AppendModifier("tsan", use_tsan=True).ExceptIfRe("-asan") + target.AppendModifier("ubsan", use_ubsan=True) target.AppendModifier("libfuzzer", use_tsan=True).OnlyIfRe("-clang") target.AppendModifier('coverage', use_coverage=True).OnlyIfRe('-(chip-tool|all-clusters)') target.AppendModifier('dmalloc', use_dmalloc=True) @@ -139,6 +140,7 @@ def BuildHostTarget(): target.AppendModifier("boringssl", crypto_library=HostCryptoLibrary.BORINGSSL).ExceptIfRe('-boringssl') target.AppendModifier("asan", use_asan=True).ExceptIfRe("-tsan") target.AppendModifier("tsan", use_tsan=True).ExceptIfRe("-asan") + target.AppendModifier("ubsan", use_ubsan=True) target.AppendModifier("libfuzzer", use_tsan=True).OnlyIfRe("-clang") target.AppendModifier('coverage', use_coverage=True).OnlyIfRe('-(chip-tool|all-clusters)') target.AppendModifier('dmalloc', use_dmalloc=True) @@ -459,6 +461,7 @@ def BuildTizenTarget(): target.AppendModifier(name="no-ble", enable_ble=False) target.AppendModifier(name="no-wifi", enable_wifi=False) target.AppendModifier(name="asan", use_asan=True) + target.AppendModifier(name="ubsan", use_ubsan=True) return target diff --git a/scripts/build/builders/host.py b/scripts/build/builders/host.py index 07bd62a9850884..b05a034e41b6d0 100644 --- a/scripts/build/builders/host.py +++ b/scripts/build/builders/host.py @@ -222,7 +222,7 @@ class HostBuilder(GnBuilder): def __init__(self, root, runner, app: HostApp, board=HostBoard.NATIVE, enable_ipv4=True, enable_ble=True, enable_wifi=True, - enable_thread=True, use_tsan=False, use_asan=False, + enable_thread=True, use_tsan=False, use_asan=False, use_ubsan=False, separate_event_loop=True, use_libfuzzer=False, use_clang=False, interactive_mode=True, extra_tests=False, use_platform_mdns=False, enable_rpcs=False, @@ -260,6 +260,9 @@ def __init__(self, root, runner, app: HostApp, board=HostBoard.NATIVE, if use_asan: self.extra_gn_options.append('is_asan=true') + if use_ubsan: + self.extra_gn_options.append('is_ubsan=true') + if use_dmalloc: self.extra_gn_options.append('chip_config_memory_debug_checks=true') self.extra_gn_options.append('chip_config_memory_debug_dmalloc=true') diff --git a/scripts/build/builders/tizen.py b/scripts/build/builders/tizen.py index 28d91ee7faf218..71ae18e9f1cce6 100644 --- a/scripts/build/builders/tizen.py +++ b/scripts/build/builders/tizen.py @@ -82,6 +82,7 @@ def __init__(self, enable_wifi: bool = True, use_asan: bool = False, use_tsan: bool = False, + use_ubsan: bool = False, ): super(TizenBuilder, self).__init__( root=os.path.join(root, app.value.source), @@ -110,6 +111,8 @@ def __init__(self, self.extra_gn_options.append('is_asan=true') if use_tsan: raise Exception("TSAN sanitizer not supported by Tizen toolchain") + if use_ubsan: + self.extra_gn_options.append('is_ubsan=true') def GnBuildArgs(self): # Make sure that required ENV variables are defined diff --git a/scripts/build/testdata/all_targets_linux_x64.txt b/scripts/build/testdata/all_targets_linux_x64.txt index 3bce2a8dd07b14..2fbf8d8813520f 100644 --- a/scripts/build/testdata/all_targets_linux_x64.txt +++ b/scripts/build/testdata/all_targets_linux_x64.txt @@ -6,8 +6,8 @@ cyw30739-cyw930739m2evb_01-{light,lock,ota-requestor}[-no-progress-logging] efr32-{brd4161a,brd4187c,brd4163a,brd4164a,brd4166a,brd4170a,brd4186a,brd4187a,brd4304a}-{window-covering,switch,unit-test,light,lock}[-rpc][-with-ota-requestor][-sed][-low-power][-shell][-no_logging][-openthread_mtd][-enable_heap_monitoring][-no_openthread_cli][-show_qr_code][-wifi][-rs911x][-wf200][-wifi_ipv4][-additional_data_advertising][-use_ot_lib][-use_ot_coap_lib][-no-version] esp32-{m5stack,c3devkit,devkitc,qemu}-{all-clusters,all-clusters-minimal,ota-provider,ota-requestor,shell,light,lock,bridge,temperature-measurement,ota-requestor,tests}[-rpc][-ipv6only] genio-lighting-app -linux-fake-tests[-mbedtls][-boringssl][-asan][-tsan][-libfuzzer][-coverage][-dmalloc][-clang] -linux-{x64,arm64}-{rpc-console,all-clusters,all-clusters-minimal,chip-tool,thermostat,java-matter-controller,minmdns,light,lock,shell,ota-provider,ota-requestor,python-bindings,tv-app,tv-casting-app,bridge,dynamic-bridge,tests,chip-cert,address-resolve-tool}[-nodeps][-platform-mdns][-minmdns-verbose][-libnl][-same-event-loop][-no-interactive][-ipv6only][-no-ble][-no-wifi][-no-thread][-mbedtls][-boringssl][-asan][-tsan][-libfuzzer][-coverage][-dmalloc][-clang][-test][-rpc] +linux-fake-tests[-mbedtls][-boringssl][-asan][-tsan][-ubsan][-libfuzzer][-coverage][-dmalloc][-clang] +linux-{x64,arm64}-{rpc-console,all-clusters,all-clusters-minimal,chip-tool,thermostat,java-matter-controller,minmdns,light,lock,shell,ota-provider,ota-requestor,python-bindings,tv-app,tv-casting-app,bridge,dynamic-bridge,tests,chip-cert,address-resolve-tool}[-nodeps][-platform-mdns][-minmdns-verbose][-libnl][-same-event-loop][-no-interactive][-ipv6only][-no-ble][-no-wifi][-no-thread][-mbedtls][-boringssl][-asan][-tsan][-ubsan][-libfuzzer][-coverage][-dmalloc][-clang][-test][-rpc] linux-x64-efr32-test-runner[-clang] imx-{chip-tool,lighting-app,thermostat,all-clusters-app,all-clusters-minimal-app,ota-provider-app}[-release] infineon-psoc6-{lock,light,all-clusters,all-clusters-minimal}[-ota][-updateimage] @@ -17,6 +17,6 @@ mw320-all-clusters-app nrf-{nrf5340dk,nrf52840dk,nrf52840dongle}-{all-clusters,all-clusters-minimal,lock,light,light-switch,shell,pump,pump-controller,window-covering}[-rpc] nrf-native-posix-64-tests qpg-qpg6105-{lock,light,shell,persistent-storage} -tizen-arm-{all-clusters,all-clusters-minimal,chip-tool,light}[-no-ble][-no-wifi][-asan] +tizen-arm-{all-clusters,all-clusters-minimal,chip-tool,light}[-no-ble][-no-wifi][-asan][-ubsan] telink-tlsr9518adk80d-{all-clusters,all-clusters-minimal,light,light-switch,ota-requestor,thermostat} openiotsdk-{shell,lock} diff --git a/src/ble/BleLayer.cpp b/src/ble/BleLayer.cpp index 3b2db861743de8..b560e6dc511592 100644 --- a/src/ble/BleLayer.cpp +++ b/src/ble/BleLayer.cpp @@ -735,7 +735,7 @@ BleTransportProtocolVersion BleLayer::GetHighestSupportedProtocolVersion(const B shift_width ^= 4; uint8_t version = reqMsg.mSupportedProtocolVersions[(i / 2)]; - version = (version >> shift_width) & 0x0F; // Grab just the nibble we want. + version = static_cast((version >> shift_width) & 0x0F); // Grab just the nibble we want. if ((version >= CHIP_BLE_TRANSPORT_PROTOCOL_MIN_SUPPORTED_VERSION) && (version <= CHIP_BLE_TRANSPORT_PROTOCOL_MAX_SUPPORTED_VERSION) && (version > retVersion)) diff --git a/src/ble/CHIPBleServiceData.h b/src/ble/CHIPBleServiceData.h index c39a30ed8a72c1..965ae74307448b 100644 --- a/src/ble/CHIPBleServiceData.h +++ b/src/ble/CHIPBleServiceData.h @@ -94,8 +94,8 @@ struct ChipBLEDeviceIdentificationInfo void SetDeviceDiscriminator(uint16_t deviceDiscriminator) { // Discriminator is 12-bit long, so don't overwrite bits 12th through 15th - deviceDiscriminator &= kDiscriminatorMask; - deviceDiscriminator |= static_cast(DeviceDiscriminatorAndAdvVersion[1] << 8u & ~kDiscriminatorMask); + auto advVersion = static_cast(DeviceDiscriminatorAndAdvVersion[1] << 8u & ~kDiscriminatorMask); + deviceDiscriminator = static_cast(advVersion | (deviceDiscriminator & kDiscriminatorMask)); chip::Encoding::LittleEndian::Put16(DeviceDiscriminatorAndAdvVersion, deviceDiscriminator); } diff --git a/src/lib/asn1/ASN1Reader.cpp b/src/lib/asn1/ASN1Reader.cpp index 14e583b6007a5f..6039e1ae084561 100644 --- a/src/lib/asn1/ASN1Reader.cpp +++ b/src/lib/asn1/ASN1Reader.cpp @@ -208,11 +208,11 @@ CHIP_ERROR ASN1Reader::GetGeneralizedTime(ASN1UniversalTime & outTime) static uint8_t ReverseBits(uint8_t v) { // swap adjacent bits - v = static_cast((v >> 1) & 0x55) | static_cast((v & 0x55) << 1); + v = static_cast(static_cast((v >> 1) & 0x55) | static_cast((v & 0x55) << 1)); // swap adjacent bit pairs - v = static_cast((v >> 2) & 0x33) | static_cast((v & 0x33) << 2); + v = static_cast(static_cast((v >> 2) & 0x33) | static_cast((v & 0x33) << 2)); // swap nibbles - v = static_cast(v >> 4) | static_cast(v << 4); + v = static_cast(static_cast(v >> 4) | static_cast(v << 4)); return v; } diff --git a/src/lib/asn1/ASN1Writer.cpp b/src/lib/asn1/ASN1Writer.cpp index 30a4fe1778fd64..aea03a559bb914 100644 --- a/src/lib/asn1/ASN1Writer.cpp +++ b/src/lib/asn1/ASN1Writer.cpp @@ -133,11 +133,11 @@ CHIP_ERROR ASN1Writer::PutOctetString(uint8_t cls, uint8_t tag, chip::TLV::TLVRe static uint8_t ReverseBits(uint8_t v) { // swap adjacent bits - v = static_cast((v >> 1) & 0x55) | static_cast((v & 0x55) << 1); + v = static_cast(static_cast((v >> 1) & 0x55) | static_cast((v & 0x55) << 1)); // swap adjacent bit pairs - v = static_cast((v >> 2) & 0x33) | static_cast((v & 0x33) << 2); + v = static_cast(static_cast((v >> 2) & 0x33) | static_cast((v & 0x33) << 2)); // swap nibbles - v = static_cast(v >> 4) | static_cast(v << 4); + v = static_cast(static_cast(v >> 4) | static_cast(v << 4)); return v; } diff --git a/src/lib/core/CHIPKeyIds.h b/src/lib/core/CHIPKeyIds.h index 9cfb4f899498be..3abbe90d674005 100644 --- a/src/lib/core/CHIPKeyIds.h +++ b/src/lib/core/CHIPKeyIds.h @@ -269,7 +269,10 @@ class ChipKeyId * @return root key number. * */ - static uint8_t GetRootKeyNumber(uint32_t keyId) { return (keyId & kMask_RootKeyNumber) >> kShift_RootKeyNumber; } + static uint8_t GetRootKeyNumber(uint32_t keyId) + { + return static_cast((keyId & kMask_RootKeyNumber) >> kShift_RootKeyNumber); + } /** * Get application group epoch key number that was used to derive specified application key. @@ -278,7 +281,10 @@ class ChipKeyId * @return epoch key number. * */ - static uint8_t GetEpochKeyNumber(uint32_t keyId) { return (keyId & kMask_EpochKeyNumber) >> kShift_EpochKeyNumber; } + static uint8_t GetEpochKeyNumber(uint32_t keyId) + { + return static_cast((keyId & kMask_EpochKeyNumber) >> kShift_EpochKeyNumber); + } /** * Get application group local number that was used to derive specified application key. @@ -287,7 +293,10 @@ class ChipKeyId * @return application group local number. * */ - static uint8_t GetAppGroupLocalNumber(uint32_t keyId) { return (keyId & kMask_GroupLocalNumber) >> kShift_GroupLocalNumber; } + static uint8_t GetAppGroupLocalNumber(uint32_t keyId) + { + return static_cast((keyId & kMask_GroupLocalNumber) >> kShift_GroupLocalNumber); + } /** * Construct application group root key ID given root key number. diff --git a/src/lib/core/CHIPTLVTypes.h b/src/lib/core/CHIPTLVTypes.h index 8cf7fc27a23756..e6fb238660ac31 100644 --- a/src/lib/core/CHIPTLVTypes.h +++ b/src/lib/core/CHIPTLVTypes.h @@ -204,7 +204,7 @@ inline uint8_t TLVFieldSizeToBytes(TLVFieldSize fieldSize) { // We would like to assert fieldSize < 7, but that gives us fatal // -Wtautological-constant-out-of-range-compare warnings... - return (fieldSize != kTLVFieldSize_0Byte) ? static_cast(1 << fieldSize) : 0; + return static_cast((fieldSize != kTLVFieldSize_0Byte) ? (1 << fieldSize) : 0); } } // namespace TLV diff --git a/src/lib/support/Base64.cpp b/src/lib/support/Base64.cpp index b1877860f598a9..8c6bfe2e7c237d 100644 --- a/src/lib/support/Base64.cpp +++ b/src/lib/support/Base64.cpp @@ -123,19 +123,19 @@ uint16_t Base64Encode(const uint8_t * in, uint16_t inLen, char * out, Base64ValT uint8_t val1, val2, val3, val4; val1 = static_cast(*in >> 2); - val2 = (*in << 4) & 0x3F; + val2 = static_cast((*in << 4) & 0x3F); in++; inLen--; if (inLen > 0) { val2 = static_cast(val2 | *in >> 4); - val3 = (*in << 2) & 0x3F; + val3 = static_cast((*in << 2) & 0x3F); in++; inLen--; if (inLen > 0) { val3 = static_cast(val3 | *in >> 6); - val4 = *in & 0x3F; + val4 = static_cast(*in & 0x3F); in++; inLen--; } diff --git a/src/lib/support/BufferWriter.cpp b/src/lib/support/BufferWriter.cpp index a8db80c2a91a65..1d7da851ea0ba1 100644 --- a/src/lib/support/BufferWriter.cpp +++ b/src/lib/support/BufferWriter.cpp @@ -57,8 +57,7 @@ LittleEndian::BufferWriter & LittleEndian::BufferWriter::EndianPut(uint64_t x, s { while (size > 0) { - uint8_t c = x & 0xff; - Put(c); + Put(static_cast(x & 0xff)); x >>= 8; size--; } @@ -69,8 +68,7 @@ BigEndian::BufferWriter & BigEndian::BufferWriter::EndianPut(uint64_t x, size_t { while (size-- > 0) { - uint8_t c = (x >> (size * 8)) & 0xff; - Put(c); + Put(static_cast((x >> (size * 8)) & 0xff)); } return *this; } diff --git a/src/lib/support/SerializableIntegerSet.cpp b/src/lib/support/SerializableIntegerSet.cpp index 19b305467cff4a..7a16aea943a5ec 100644 --- a/src/lib/support/SerializableIntegerSet.cpp +++ b/src/lib/support/SerializableIntegerSet.cpp @@ -27,7 +27,7 @@ CHIP_ERROR SerializableU64SetBase::Deserialize(ByteSpan serialized) { VerifyOrReturnError(serialized.size() <= MaxSerializedSize(), CHIP_ERROR_INVALID_ARGUMENT); memcpy(mData, serialized.data(), serialized.size()); - mNextAvailable = static_cast(serialized.size()) / static_cast(sizeof(uint64_t)); + mNextAvailable = static_cast(serialized.size() / sizeof(uint64_t)); // Our serialized data is always little-endian; swap to native. SwapByteOrderIfNeeded(); diff --git a/src/setup_payload/QRCodeSetupPayloadGenerator.cpp b/src/setup_payload/QRCodeSetupPayloadGenerator.cpp index c6531482a56344..386000ba452a35 100644 --- a/src/setup_payload/QRCodeSetupPayloadGenerator.cpp +++ b/src/setup_payload/QRCodeSetupPayloadGenerator.cpp @@ -53,7 +53,8 @@ static CHIP_ERROR populateBits(uint8_t * bits, size_t & offset, uint64_t input, { if (input & 1) { - bits[index / 8] |= static_cast(1 << index % 8); + const uint8_t mask = static_cast(1 << index % 8); + bits[index / 8] = static_cast(bits[index / 8] | mask); } index++; input >>= 1;