Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .evergreen/config_generator/components/loadbalanced.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ def tasks():
env={
'CC': _COMPILER,
'CFLAGS': '-fno-omit-frame-pointer',
'EXTRA_CONFIGURE_FLAGS': '-DENABLE_EXTRA_ALIGNMENT=OFF',
'SSL': 'OPENSSL'
},
working_dir='mongoc',
Expand Down
1 change: 0 additions & 1 deletion .evergreen/config_generator/components/mock_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ def variants():
'CC': 'gcc',
'ASAN': 'on',
'CFLAGS': '-fno-omit-frame-pointer',
'EXTRA_CONFIGURE_FLAGS': '-DENABLE_EXTRA_ALIGNMENT=OFF',
'SANITIZE': 'address,undefined',
}
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ def variants():
'ASAN': 'on',
'CFLAGS': '-fno-omit-frame-pointer',
'CHECK_LOG': 'ON',
'EXTRA_CONFIGURE_FLAGS': '-DENABLE_EXTRA_ALIGNMENT=OFF',
'SANITIZE': 'address,undefined',
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def variants():
expansions = {
'CFLAGS': '-fno-omit-frame-pointer',
'CHECK_LOG': 'ON',
'EXTRA_CONFIGURE_FLAGS': '-DENABLE_EXTRA_ALIGNMENT=OFF -DENABLE_SHM_COUNTERS=OFF',
'EXTRA_CONFIGURE_FLAGS': '-DENABLE_SHM_COUNTERS=OFF',
'SANITIZE': 'thread',
}

Expand Down
4 changes: 2 additions & 2 deletions .evergreen/generated_configs/legacy-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ tasks:
shell: bash
script: |-
set -o errexit
env EXTRA_CONFIGURE_FLAGS="-DENABLE_EXTRA_ALIGNMENT=OFF" SNAPPY="OFF" ZLIB="BUNDLED" ZSTD="OFF" .evergreen/scripts/compile.sh
env SNAPPY="OFF" ZLIB="BUNDLED" ZSTD="OFF" .evergreen/scripts/compile.sh
- func: upload-build
- name: debug-compile-nosasl-nossl
tags:
Expand Down Expand Up @@ -1311,7 +1311,7 @@ tasks:
shell: bash
script: |-
set -o errexit
env SANITIZE=address SASL=AUTO SSL=OPENSSL EXTRA_CONFIGURE_FLAGS='-DENABLE_EXTRA_ALIGNMENT=OFF' .evergreen/scripts/compile.sh
env SANITIZE=address SASL=AUTO SSL=OPENSSL .evergreen/scripts/compile.sh
- func: prepare-kerberos
- func: run auth tests
vars:
Expand Down
1 change: 0 additions & 1 deletion .evergreen/generated_configs/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2949,7 +2949,6 @@ tasks:
env:
CC: gcc
CFLAGS: -fno-omit-frame-pointer
EXTRA_CONFIGURE_FLAGS: -DENABLE_EXTRA_ALIGNMENT=OFF
SSL: OPENSSL
args:
- -c
Expand Down
4 changes: 1 addition & 3 deletions .evergreen/generated_configs/variants.yml
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ buildvariants:
ASAN: "on"
CC: gcc
CFLAGS: -fno-omit-frame-pointer
EXTRA_CONFIGURE_FLAGS: -DENABLE_EXTRA_ALIGNMENT=OFF
SANITIZE: address,undefined
tasks:
- name: mock-server-test
Expand All @@ -194,7 +193,6 @@ buildvariants:
ASAN: "on"
CFLAGS: -fno-omit-frame-pointer
CHECK_LOG: "ON"
EXTRA_CONFIGURE_FLAGS: -DENABLE_EXTRA_ALIGNMENT=OFF
SANITIZE: address,undefined
tasks:
- name: .sanitizers-matrix-asan
Expand All @@ -203,7 +201,7 @@ buildvariants:
expansions:
CFLAGS: -fno-omit-frame-pointer
CHECK_LOG: "ON"
EXTRA_CONFIGURE_FLAGS: -DENABLE_EXTRA_ALIGNMENT=OFF -DENABLE_SHM_COUNTERS=OFF
EXTRA_CONFIGURE_FLAGS: -DENABLE_SHM_COUNTERS=OFF
SANITIZE: thread
tasks:
- name: .sanitizers-matrix-tsan
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ class CompileWithClientSideEncryptionAsan(CompileTask):
CFLAGS="-fno-omit-frame-pointer",
COMPILE_LIBMONGOCRYPT="ON",
CHECK_LOG="ON",
EXTRA_CONFIGURE_FLAGS="-DENABLE_EXTRA_ALIGNMENT=OFF",
PATH="/usr/lib/llvm-3.8/bin:$PATH",
)
cls_tags: ClassVar[Sequence[str]] = ["client-side-encryption"]
Expand Down Expand Up @@ -191,7 +190,6 @@ def __init__(
"debug-compile-no-align",
tags=["debug-compile"],
compression="zlib",
EXTRA_CONFIGURE_FLAGS="-DENABLE_EXTRA_ALIGNMENT=OFF",
),
CompileTask("debug-compile-nosasl-nossl", tags=["debug-compile", "nosasl", "nossl"], SSL="OFF"),
CompileTask("debug-compile-lto", CFLAGS="-flto"),
Expand Down Expand Up @@ -697,7 +695,7 @@ def pre_commands(self) -> Iterable[Value]:
func("find-cmake-latest"),
shell_mongoc(
"""
env SANITIZE=address SASL=AUTO SSL=OPENSSL EXTRA_CONFIGURE_FLAGS='-DENABLE_EXTRA_ALIGNMENT=OFF' .evergreen/scripts/compile.sh
env SANITIZE=address SASL=AUTO SSL=OPENSSL .evergreen/scripts/compile.sh
""",
add_expansions_to_env=True,
),
Expand Down
15 changes: 0 additions & 15 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -185,19 +185,6 @@ mongo_bool_setting(
]]
)

# Deprecated options:
mongo_bool_setting(
ENABLE_EXTRA_ALIGNMENT
"[Deprecated] Enable extra alignment on libbson types"
DEFAULT VALUE ON
DEVEL VALUE OFF
VALIDATE CODE [[
if(ENABLE_EXTRA_ALIGNMENT AND MONGO_SANITIZE MATCHES "undefined")
message(WARNING "ENABLE_EXTRA_ALIGNMENT=“${ENABLE_EXTRA_ALIGNMENT}” will create conflicts with UndefinedBehaviorSanitizer")
endif()
]]
)

# Announce the build configuration. Used by `mlib_build_config_is()` in `mlib/config.h`
add_compile_definitions(_MLIB_BUILD_CONFIG=$<CONFIG>)

Expand Down Expand Up @@ -302,8 +289,6 @@ if ( (ENABLE_BUILD_DEPENDECIES STREQUAL OFF) AND (NOT CMAKE_CURRENT_SOURCE_DIR S
set (ENABLE_BUILD_DEPENDECIES ON)
endif ()

mongo_pick(BSON_EXTRA_ALIGN 1 0 ENABLE_EXTRA_ALIGNMENT)

mongo_pick(MONGOC_ENABLE_RDTSCP 1 0 ENABLE_RDTSCP)

mongo_pick(MONGOC_ENABLE_STATIC_BUILD 1 0 ENABLE_STATIC)
Expand Down
1 change: 0 additions & 1 deletion Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ build:
RUN cmake -S "$source_dir" -B "$build_dir" -G "Ninja Multi-Config" \
-D ENABLE_MAINTAINER_FLAGS=ON \
-D ENABLE_SHM_COUNTERS=ON \
-D ENABLE_EXTRA_ALIGNMENT=OFF \
-D ENABLE_SASL=$(echo $sasl | __str upper) \
-D ENABLE_SNAPPY=ON \
-D ENABLE_SRV=ON \
Expand Down
4 changes: 4 additions & 0 deletions src/libbson/NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ libbson 2.0.0 (Unreleased)
* Compiling with `BSON_MEMCHECK` defined now has no effect.
* Remove deprecated integral comparison interfaces: `bson_in_range_*` and `bson_cmp_*`.
* Remove deprecated atomic and threading interfaces: `bson_atomic_*` and `bson_thrd_yield`.
* The deprecated `ENABLE_EXTRA_ALIGNMENT` CMake option is removed.
* `bson_t` and `bson_iter_t` are now aligned to the size of a pointer.
* `BSON_ALIGNED_BEGIN` and `BSON_ALIGNED_END` now unconditionally apply their requested alignment.


libbson 1.30.2
==============
Expand Down
3 changes: 1 addition & 2 deletions src/libbson/doc/bson_t.rst
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,11 @@ Synopsis
#define BSON_APPEND_VALUE(b, key, val) \
bson_append_value (b, key, (int) strlen (key), (val))

BSON_ALIGNED_BEGIN (128)
typedef struct {
uint32_t flags; /* Internal flags for the bson_t. */
uint32_t len; /* Length of BSON data. */
uint8_t padding[120]; /* Padding for stack allocation. */
} bson_t BSON_ALIGNED_END (128);
} bson_t;

Description
-----------
Expand Down
8 changes: 0 additions & 8 deletions src/libbson/src/bson/bson-config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,6 @@
#endif


/*
* Define to 1 if you want extra aligned types in libbson
*/
#define BSON_EXTRA_ALIGN @BSON_EXTRA_ALIGN@
#if BSON_EXTRA_ALIGN != 1
# undef BSON_EXTRA_ALIGN
#endif


/*
* Define to 1 if you have rand_r available on your platform.
Expand Down
10 changes: 0 additions & 10 deletions src/libbson/src/bson/bson-macros.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,23 +162,13 @@
#define BSON_ALIGN_OF_PTR (BSON_ALIGNOF (void *))
#endif

#ifdef BSON_EXTRA_ALIGN
#if defined(_MSC_VER)
#define BSON_ALIGNED_BEGIN(_N) __declspec (align (_N))
#define BSON_ALIGNED_END(_N)
#else
#define BSON_ALIGNED_BEGIN(_N)
#define BSON_ALIGNED_END(_N) __attribute__ ((aligned (_N)))
#endif
#else
#if defined(_MSC_VER)
#define BSON_ALIGNED_BEGIN(_N) __declspec (align (BSON_ALIGN_OF_PTR))
#define BSON_ALIGNED_END(_N)
#else
#define BSON_ALIGNED_BEGIN(_N)
#define BSON_ALIGNED_END(_N) __attribute__ ((aligned ((_N) > BSON_ALIGN_OF_PTR ? BSON_ALIGN_OF_PTR : (_N))))
#endif
#endif


#define bson_str_empty(s) (!s[0])
Expand Down
8 changes: 4 additions & 4 deletions src/libbson/src/bson/bson-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,18 @@ typedef enum {
#define BSON_INLINE_DATA_SIZE 120


BSON_ALIGNED_BEGIN (128)
BSON_ALIGNED_BEGIN (BSON_ALIGN_OF_PTR)
typedef struct {
bson_flags_t flags;
uint32_t len;
uint8_t data[BSON_INLINE_DATA_SIZE];
} bson_impl_inline_t BSON_ALIGNED_END (128);
} bson_impl_inline_t BSON_ALIGNED_END (BSON_ALIGN_OF_PTR);


BSON_STATIC_ASSERT2 (impl_inline_t, sizeof (bson_impl_inline_t) == 128);


BSON_ALIGNED_BEGIN (128)
BSON_ALIGNED_BEGIN (BSON_ALIGN_OF_PTR)
typedef struct {
bson_flags_t flags; /* flags describing the bson_t */
/* len is part of the public bson_t declaration. It is not
Expand All @@ -71,7 +71,7 @@ typedef struct {
size_t alloclen; /* length of buffer that we own. */
bson_realloc_func realloc; /* our realloc implementation */
void *realloc_func_ctx; /* context for our realloc func */
} bson_impl_alloc_t BSON_ALIGNED_END (128);
} bson_impl_alloc_t BSON_ALIGNED_END (BSON_ALIGN_OF_PTR);


BSON_STATIC_ASSERT2 (impl_alloc_t, sizeof (bson_impl_alloc_t) <= 128);
Expand Down
8 changes: 4 additions & 4 deletions src/libbson/src/bson/bson-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ typedef struct _bson_json_opts_t bson_json_opts_t;
*
* This structure is meant to fit in two sequential 64-byte cachelines.
*/
BSON_ALIGNED_BEGIN (128) typedef struct _bson_t {
BSON_ALIGNED_BEGIN (BSON_ALIGN_OF_PTR) typedef struct _bson_t {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

uint32_t flags; /* Internal flags for the bson_t. */
uint32_t len; /* Length of BSON data. */
uint8_t padding[120]; /* Padding for stack allocation. */
} bson_t BSON_ALIGNED_END (128);
} bson_t BSON_ALIGNED_END (BSON_ALIGN_OF_PTR);

/**
* BSON_INITIALIZER:
Expand Down Expand Up @@ -342,7 +342,7 @@ typedef struct _bson_value_t {
* This structure is safe to discard on the stack. No cleanup is necessary
* after using it.
*/
BSON_ALIGNED_BEGIN (128)
BSON_ALIGNED_BEGIN (BSON_ALIGN_OF_PTR)
typedef struct {
const uint8_t *raw; /* The raw buffer being iterated. */
uint32_t len; /* The length of raw. */
Expand All @@ -356,7 +356,7 @@ typedef struct {
uint32_t next_off; /* The offset of the next field. */
uint32_t err_off; /* The offset of the error. */
bson_value_t value; /* Internal value for various state. */
} bson_iter_t BSON_ALIGNED_END (128);
} bson_iter_t BSON_ALIGNED_END (BSON_ALIGN_OF_PTR);


/**
Expand Down