CDRIVER-2813 remove BSON_EXTRA_ALIGNMENT option

[kevinAlbs]

Summary

• Remove CMake option ENABLE_EXTRA_ALIGNMENT.
• Align bson_t and bson_iter_t to the size of a pointer.

Verified with this patch build: https://spruce.mongodb.com/version/67dc0db1caab950007b0f764

Motivation

Addresses long-standing ask to remove default over-alignment: CDRIVER-2813.

Removing alignment specifiers entirely was considered. However, removing the alignment specifier on bson_t resulted in a runtime error in calls to BSON_ALIGNED_ALLOC (bson_t):

Failure to allocate memory in bson_aligned_alloc()

I expect this is due to bson_t having alignment of 4. bson_t does not contain a pointer.

From man aligned_alloc (emphasis mine):

In addition, aligned_alloc() returns a NULL pointer and sets errno to EINVAL if size is not an integral multiple of alignment, or if alignment is not a power of 2 at least as large as sizeof(void *).

To limit the risk of causing runtime errors in user upgrades, this PR proposes keeping existing alignment specifiers. Only the over-alignment of bson_t and bson_iter_t is changed.

Alignment changes

The following were printed on my 64-bit machine to compare alignment changes.

Prior to this PR. With ENABLE_EXTRA_ALIGNMENT=ON:

alignof(bson_t):           128
alignof(bson_iter_t):      128
alignof(bson_visitor_t):   8
alignof(bson_reader_t):    8
alignof(bson_value_t):     8
alignof(bson_error_t):     8

With all alignment specifiers removed (not done in this PR):

alignof(bson_t):           4
alignof(bson_iter_t):      8
alignof(bson_visitor_t):   8
alignof(bson_reader_t):    4
alignof(bson_value_t):     8
alignof(bson_error_t):     4

With this PR:

alignof(bson_t):           8
alignof(bson_iter_t):      8
alignof(bson_visitor_t):   8
alignof(bson_reader_t):    8
alignof(bson_value_t):     8
alignof(bson_error_t):     8

[vector-of-bool]

Could we instead fix bson_aligned_alloc to increase the alignment size if it is below pointer alignment?

[kevinAlbs]

I am concerned removing the alignment specifier could break existing code allocating without libbson functions. Example:

bson_t *b = aligned_alloc (alignof(bson_t), sizeof(bson_t));

Upgrading to 2.0 may make this a runtime error. Maybe (hopefully) it is unlikely. I see no usage on GitHub code search for align.*bson_t. Though there is use of malloc.*bson_t.

AFAIK types being aligned to a pointer size (or 8) is less of an issue. If there is little observed benefit, it might not be worth the risk (even if small).

[eramongodb]

Polymorphic types such as bson_t and bson_reader_t require a pointer alignment specifier in their "base" type due to pointer data members in their "derived" types (bson_impl_alloc_t and bson_reader_handle_t respectively). The alignment specifier is probably preferable to awkwardly inserting a stub pointer data member in the "base" type.

[eramongodb]

Suggest also removing the debug-compile-no-align tasks from the legacy config.

[vector-of-bool]

LGTM

[eramongodb]

Note: partially reverting mongodb/mongo-cxx-driver#1329 (reverting only the addition and use of bsoncxx::aligned_storage<T>) reveals a single benign warning when compiled with MSVC and /W4 (analogous to -Wpadded warnings for GCC and Clang):

warning C4324: 'bsoncxx::v_noabi::builder::core::impl::frame': structure was padded due to alignment specifier

Runtime alignment errors are not observed when running C++ Driver UBSan tasks with the partial revert using the changes in this PR branch. 👍