UBsan warns on access to 0 sized arrays in union

[m-gupta]

Bugzilla Link	44608
Version	unspecified
OS	Linux
CC	@cmtice,@dwblaikie,@gburgessiv,@kcc,@vitalybuka

Extended Description

Copied from Chrome OS bug
https://bugs.chromium.org/p/chromium/issues/detail?id=1043405

One of the programs in Chrome OS uses a 0 sized array inside a union.
ubsan errors on oob accesses to this member. Given that this array is 0 sized, should accesses to it be treated differently?

Relevant struct snippet from
https://chromium.git.corp.google.com/chromiumos/platform/vboot_reference/+/f5367d598a985520a8c935f68ac90d295c7b8d8e/firmware/2lib/include/2sha.h

struct vb2_hash {
	/* enum vb2_hash_algorithm. Fixed width for serialization.
	   Single byte to avoid endianness issues. */
	uint8_t algo;
	/* Padding to align and to match existing CBFS attribute. */
	uint8_t reserved[3];
	/* The actual digest. Can add new types here as required. */
	union {
		uint8_t raw[0]; // triggers ubsan oob checks
#if VB2_SUPPORT_SHA1
		uint8_t sha1[VB2_SHA1_DIGEST_SIZE];
#endif
#if VB2_SUPPORT_SHA256
		uint8_t sha256[VB2_SHA256_DIGEST_SIZE];
#endif
#if VB2_SUPPORT_SHA512
		uint8_t sha512[VB2_SHA512_DIGEST_SIZE];
#endif
	} bytes;  /* This has a name so that it's easy to sizeof(). */
};

[Endilll]

CC @AaronBallman

[llvmbot]

@llvm/issue-subscribers-clang-codegen

Author: None (m-gupta)

| | | | --- | --- | | Bugzilla Link | [44608](https://llvm.org/bz44608) | | Version | unspecified | | OS | Linux | | CC | @cmtice,@dwblaikie,@gburgessiv,@kcc,@vitalybuka |

Extended Description

Copied from Chrome OS bug
https://bugs.chromium.org/p/chromium/issues/detail?id=1043405

One of the programs in Chrome OS uses a 0 sized array inside a union.
ubsan errors on oob accesses to this member. Given that this array is 0 sized, should accesses to it be treated differently?

Relevant struct snippet from
https://chromium.git.corp.google.com/chromiumos/platform/vboot_reference/+/f5367d598a985520a8c935f68ac90d295c7b8d8e/firmware/2lib/include/2sha.h

struct vb2_hash {
	/* enum vb2_hash_algorithm. Fixed width for serialization.
	   Single byte to avoid endianness issues. */
	uint8_t algo;
	/* Padding to align and to match existing CBFS attribute. */
	uint8_t reserved[3];
	/* The actual digest. Can add new types here as required. */
	union {
		uint8_t raw[0]; // triggers ubsan oob checks
#if VB2_SUPPORT_SHA1
		uint8_t sha1[VB2_SHA1_DIGEST_SIZE];
#endif
#if VB2_SUPPORT_SHA256
		uint8_t sha256[VB2_SHA256_DIGEST_SIZE];
#endif
#if VB2_SUPPORT_SHA512
		uint8_t sha512[VB2_SHA512_DIGEST_SIZE];
#endif
	} bytes;  /* This has a name so that it's easy to sizeof(). */
};

[AaronBallman]

I think any attempted read or write of a zero-sized object should be caught by UBSan because that access is trying to do something out of bounds. CC @zygoloid for additional opinions

[zygoloid]

We have explicit code to test whether we're indexing a flexible array member and disable the UBSan check, and we unconditionally treat an array of 0 elements in a union as being a flexible array member. I also can't reproduce the problem with a simple testcase. So maybe this is fixed already?

[AaronBallman]

Your simple test case doesn't reproduce an issue in any version of Clang, so perhaps the issue requires a particular test case?

[zygoloid]

It looks to me like that testcase reproduces the problem with Clang 10 and before, and not with Clang 11 onwards. So I think this was fixed in Clang 11.