Skip to content

Commit

Permalink
Add ubsan build variant to build_examples script (#23953)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
arkq authored and pull[bot] committed May 19, 2023
1 parent 9cb7b5e commit 2192092
Show file tree
Hide file tree
Showing 16 changed files with 53 additions and 32 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/examples-tizen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
14 changes: 9 additions & 5 deletions build/config/compiler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,6 @@ config("sanitize_address") {

config("sanitize_thread") {
cflags = [ "-fsanitize=thread" ]

ldflags = cflags
}

Expand All @@ -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
}

Expand Down
3 changes: 3 additions & 0 deletions scripts/build/build/targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion scripts/build/builders/host.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down
3 changes: 3 additions & 0 deletions scripts/build/builders/tizen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions scripts/build/testdata/all_targets_linux_x64.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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}
2 changes: 1 addition & 1 deletion src/ble/BleLayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>((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))
Expand Down
4 changes: 2 additions & 2 deletions src/ble/CHIPBleServiceData.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint16_t>(DeviceDiscriminatorAndAdvVersion[1] << 8u & ~kDiscriminatorMask);
auto advVersion = static_cast<uint16_t>(DeviceDiscriminatorAndAdvVersion[1] << 8u & ~kDiscriminatorMask);
deviceDiscriminator = static_cast<uint16_t>(advVersion | (deviceDiscriminator & kDiscriminatorMask));
chip::Encoding::LittleEndian::Put16(DeviceDiscriminatorAndAdvVersion, deviceDiscriminator);
}

Expand Down
6 changes: 3 additions & 3 deletions src/lib/asn1/ASN1Reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@ CHIP_ERROR ASN1Reader::GetGeneralizedTime(ASN1UniversalTime & outTime)
static uint8_t ReverseBits(uint8_t v)
{
// swap adjacent bits
v = static_cast<uint8_t>((v >> 1) & 0x55) | static_cast<uint8_t>((v & 0x55) << 1);
v = static_cast<uint8_t>(static_cast<uint8_t>((v >> 1) & 0x55) | static_cast<uint8_t>((v & 0x55) << 1));
// swap adjacent bit pairs
v = static_cast<uint8_t>((v >> 2) & 0x33) | static_cast<uint8_t>((v & 0x33) << 2);
v = static_cast<uint8_t>(static_cast<uint8_t>((v >> 2) & 0x33) | static_cast<uint8_t>((v & 0x33) << 2));
// swap nibbles
v = static_cast<uint8_t>(v >> 4) | static_cast<uint8_t>(v << 4);
v = static_cast<uint8_t>(static_cast<uint8_t>(v >> 4) | static_cast<uint8_t>(v << 4));
return v;
}

Expand Down
6 changes: 3 additions & 3 deletions src/lib/asn1/ASN1Writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>((v >> 1) & 0x55) | static_cast<uint8_t>((v & 0x55) << 1);
v = static_cast<uint8_t>(static_cast<uint8_t>((v >> 1) & 0x55) | static_cast<uint8_t>((v & 0x55) << 1));
// swap adjacent bit pairs
v = static_cast<uint8_t>((v >> 2) & 0x33) | static_cast<uint8_t>((v & 0x33) << 2);
v = static_cast<uint8_t>(static_cast<uint8_t>((v >> 2) & 0x33) | static_cast<uint8_t>((v & 0x33) << 2));
// swap nibbles
v = static_cast<uint8_t>(v >> 4) | static_cast<uint8_t>(v << 4);
v = static_cast<uint8_t>(static_cast<uint8_t>(v >> 4) | static_cast<uint8_t>(v << 4));
return v;
}

Expand Down
15 changes: 12 additions & 3 deletions src/lib/core/CHIPKeyIds.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>((keyId & kMask_RootKeyNumber) >> kShift_RootKeyNumber);
}

/**
* Get application group epoch key number that was used to derive specified application key.
Expand All @@ -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<uint8_t>((keyId & kMask_EpochKeyNumber) >> kShift_EpochKeyNumber);
}

/**
* Get application group local number that was used to derive specified application key.
Expand All @@ -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<uint8_t>((keyId & kMask_GroupLocalNumber) >> kShift_GroupLocalNumber);
}

/**
* Construct application group root key ID given root key number.
Expand Down
2 changes: 1 addition & 1 deletion src/lib/core/CHIPTLVTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(1 << fieldSize) : 0;
return static_cast<uint8_t>((fieldSize != kTLVFieldSize_0Byte) ? (1 << fieldSize) : 0);
}

} // namespace TLV
Expand Down
6 changes: 3 additions & 3 deletions src/lib/support/Base64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(*in >> 2);
val2 = (*in << 4) & 0x3F;
val2 = static_cast<uint8_t>((*in << 4) & 0x3F);
in++;
inLen--;
if (inLen > 0)
{
val2 = static_cast<uint8_t>(val2 | *in >> 4);
val3 = (*in << 2) & 0x3F;
val3 = static_cast<uint8_t>((*in << 2) & 0x3F);
in++;
inLen--;
if (inLen > 0)
{
val3 = static_cast<uint8_t>(val3 | *in >> 6);
val4 = *in & 0x3F;
val4 = static_cast<uint8_t>(*in & 0x3F);
in++;
inLen--;
}
Expand Down
6 changes: 2 additions & 4 deletions src/lib/support/BufferWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(x & 0xff));
x >>= 8;
size--;
}
Expand All @@ -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<uint8_t>((x >> (size * 8)) & 0xff));
}
return *this;
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/support/SerializableIntegerSet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint16_t>(serialized.size()) / static_cast<uint16_t>(sizeof(uint64_t));
mNextAvailable = static_cast<uint16_t>(serialized.size() / sizeof(uint64_t));

// Our serialized data is always little-endian; swap to native.
SwapByteOrderIfNeeded();
Expand Down
3 changes: 2 additions & 1 deletion src/setup_payload/QRCodeSetupPayloadGenerator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t>(1 << index % 8);
const uint8_t mask = static_cast<uint8_t>(1 << index % 8);
bits[index / 8] = static_cast<uint8_t>(bits[index / 8] | mask);
}
index++;
input >>= 1;
Expand Down

0 comments on commit 2192092

Please sign in to comment.