Reducing Warnings - Misaligned Address of Over-Aligned Types

[eramongodb]

Description

Resolves CDRIVER-4434. This PR resolves ASan runtime errors regarding misalignment of over-aligned types, particularly when the C Driver is configured with ENABLE_EXTRA_ALIGNMENT=ON (which should probably be deprecated as was done for ENABLE_AUTOMATIC_INIT_AND_CLEANUP=ON).

This has impacted end users even with ENABLE_EXTRA_ALIGNMENT=OFF as reported in CDRIVER-1051.

As a drive-by improvement, the debug-compile-aws and link-sample-program-based tasks were updated to correctly configure C99 standard conformance options for correct detection of (the lack of) C11 features, which was interfering with selection of aligned_alloc alternatives.

BSON_ALIGNOF

Essential to the scope of this PR was introducing a utility to query the alignment of a given type (to avoid the use of magic numbers or defining numerous BSON_ALIGN_OF_<Type> constants). The C11 _Alignof operator is used if available; otherwise, compiler extensions are utilized instead.

BSON_ALIGN_OF_PTR was updated to be defined in terms of BSON_ALIGNOF instead of sizeof (should have no behavioral difference).

Note: the special-casing of _MSC_VER for BSON_ALIGN_OF_PTR are preserved as __declspec (align (N)) only supports integer literals for N, prohibiting the use of BSON_ALIGNOF within BSON_ALIGNED_* macros (which was initially attempted but subsequently reverted to preserve compatibility with existing platforms). This also prevented redefining the extended alignment of bson_impl_inline_t and bson_impl_alloc_t in terms of bson_t.

aligned_alloc

Empowered by the addition of BSON_ALIGNOF, bson_aligned_alloc and bson_aligned_alloc0 were added to bson-memory.h and an .aligned_alloc member added to bson_mem_vtable_t to support correctly aligned allocations.

bson_aligned_alloc0 was added primarily for convenience as an alignment-aware equivalent to bson_malloc0.

bson_aligned_alloc is designed to behave as a stand-in for C11 aligned_alloc. Although POSIX conformance requires alignment to be a power of 2 and a multiple of sizeof (void *) and num_bytes to be a multiple of alignment, bson_aligned_alloc defers actual assertions to the appropriate implementation-defined function that is invoked rather than explicitly (potentially redundantly) asserting the requirements. When neither C11 or posix_memalign are available, the implementation defers to malloc. (Note, for MSVC, the alternative _aligned_alloc cannot satisfy C11 aligned_alloc requirements as it modifies errno and requires the returned memory be freed by a dedicated _aligned_free function instead of free.)

Every instance of a call to bson_malloc or bson_malloc0 allocating a type with extended alignment requirements (as detected by ASan) was replaced with a call to bson_aligned_alloc or bson_aligned_alloc0 respectively. For nearly all instances, this transformation was trivial. Exceptions that required additional effort are noted below.

Note: as _Alignof does not support expressions as arguments, corresponding sizeof were converted from expressions to the explicit type for symmetry:

- // runtime error: misaligned address
- bson_t *bson = bson_malloc (sizeof (*bson));

- // error: '_Alignof' applied to an expression is a GNU extension
- bson_t *bson = bson_aligned_alloc (BSON_ALIGNOF (*bson), sizeof (*bson));

+ // OK: correct alignment.
+ bson_t *bson = bson_aligned_alloc (BSON_ALIGNOF (bson_t), sizeof (bson_t));

mongoc_array_aligned_init

To support correct alignment of elements of mongoc_array_t, a mongoc_array_aligned_init function was added. Existing calls to mongoc_array_init that do not require extended alignment support are thus left unmodified.

As there is not an aligned equivalent to realloc, if the array was initialized with alignment requirements, storage extension performs an bson_aligned_alloc + memmove to maintain alignment requirements. If the array was not initialized with alignment requirements, realloc is used as was done prior.

pool_node::data

This struct presented numerous challenges w.r.t. correct alignment due to the use of a flexible member array to allocate elements (aka items) inline within the pool_node object. This inline allocation is required to allow for the owning pool_node object to be obtained given a pointer to the element.

Given this requirement, several adjustments to mongoc_ts_pool were made accordingly:

_pool_node_data_offset

Rather than using pool_node::data as the location of the element (aka item), an offset is applied such that the element is correctly aligned in memory. This required extending the allocation size of pool_node to pad pool_node::data accordingly:

[ next | owner_pool | (padding) | element ]
^                   ^           ^
|                   |           |
pool_node *         |           first byte of aligned element
                    pool_node::data

The difference (padding) between the address of the element and the address of pool_node::data is expressed as _pool_node_data_offset.

Given a call to <PoolName>_new_with_params, the alignment is detected automatically via BSON_ALIGNOF. However, the alignment may be set manually via a direct call to <PoolName>_new, including opting out of alignment requirements by setting .element_alignment = 0 (as is done in test_ts_pool_simple). A non-zero alignment that is too small to satisfy bson_aligned_alloc requirements is automatically promoted to a compatible alignment on initialization of the pool; otherwise, any pool whose element type has an alignment less than void * would be forced to go out of its way to explicitly set the alignment.

_pool_node_from_item

The owning pool_node object must be obtainable from a pointer to an element in pool_node::data. This operation was abstracted as _pool_node_from_item, which requires _pool_node_data_offset to be known for the given element, which in turn requires access to the pool::element_alignment for the owning pool object. This operation forced some functions to be modified to request a pointer to the pool object:

• mongoc_ts_pool_return (elem) -> mongoc_ts_pool_return (pool, elem)
• mongoc_ts_pool_drop (elem) -> mongoc_ts_pool_drop (pool, elem)

For all instances of calls to these two functions (note: all non-test instances were restricted to mongoc-topology.c), the corresponding pool was easily obtainable and thus were modified accordingly.

[eramongodb]

Addition of the new debug-compile-ubsan-with-extra-alignment task exposed more misaligned allocations than local testing. It may be worth eventually expanding the set of variants being tested with UBSan for better coverage.

[kevinAlbs]

Nicely done. LGTM

[vector-of-bool]

pain.

Not because of these changes, but because they are necessary.

LGTM, but with a remark about deduplication that may be useful at some point.

[vector-of-bool]

This pattern of aligned_alloc(alignof(T), sizeof(T)) is pretty repetitive. It might be useful to have a bson_alloc_new(T) function-like macro.

[eramongodb]

Good call. Added BSON_ALIGNED_ALLOC and BSON_ALIGNED_ALLOC0 convenience macros to avoid type repetition.

[vector-of-bool]

LGTM. One possible improvement on the alloc macro, otherwise good.

[vector-of-bool]

If you insert an explicit cast-to-T*, that can catch incompatible-pointer assignments. i.e. double* d = BSON_ALIGNED_ALLOC0(int)