Skip to content

CDRIVER-4264 Address build failures on VS 2013 variants on Evergreen #932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jan 24, 2022
Merged

CDRIVER-4264 Address build failures on VS 2013 variants on Evergreen #932

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eae9c12
Format bson-atomic.c
eramongodb Jan 21, 2022
5b0dbb3
Remove test suite signal handlers for C90 compatibility
eramongodb Jan 20, 2022
9f9f48b
Replace snprintf with bson_snprintf
eramongodb Jan 20, 2022
77bbfee
Address missing _InterlockedExchangePointer on x86 with VS 2013
eramongodb Jan 21, 2022
7af0dbe
Fix typos in comments
eramongodb Jan 24, 2022
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
35 changes: 24 additions & 11 deletions src/libbson/src/bson/bson-atomic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ bson_memory_barrier (void)
}

/**
* Some platforms do not support compiler intrinsics for atomic operations.
* Some platforms do not support compiler ics for atomic operations.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Thank you for the catch.

* We emulate that here using a spin lock and regular arithmetic operations
*/
static int8_t gEmulAtomicLock = 0;
Expand Down Expand Up @@ -195,8 +195,8 @@ _bson_emul_atomic_int32_compare_exchange_weak (volatile int32_t *p,

int
_bson_emul_atomic_int_fetch_add (volatile int *p,
int n,
enum bson_memory_order _unused)
int n,
enum bson_memory_order _unused)
{
int ret;
_lock_emul_atomic ();
Expand All @@ -208,8 +208,8 @@ _bson_emul_atomic_int_fetch_add (volatile int *p,

int
_bson_emul_atomic_int_exchange (volatile int *p,
int n,
enum bson_memory_order _unused)
int n,
enum bson_memory_order _unused)
{
int ret;
_lock_emul_atomic ();
Expand All @@ -221,9 +221,9 @@ _bson_emul_atomic_int_exchange (volatile int *p,

int
_bson_emul_atomic_int_compare_exchange_strong (volatile int *p,
int expect_value,
int new_value,
enum bson_memory_order _unused)
int expect_value,
int new_value,
enum bson_memory_order _unused)
{
int ret;
_lock_emul_atomic ();
Expand All @@ -237,11 +237,24 @@ _bson_emul_atomic_int_compare_exchange_strong (volatile int *p,

int
_bson_emul_atomic_int_compare_exchange_weak (volatile int *p,
int expect_value,
int new_value,
enum bson_memory_order order)
int expect_value,
int new_value,
enum bson_memory_order order)
{
/* We're emulating. We can't do a weak version. */
return _bson_emul_atomic_int_compare_exchange_strong (
p, expect_value, new_value, order);
}

void *
_bson_emul_atomic_ptr_exchange (void *volatile *p,
void *n,
enum bson_memory_order _unused)
{
void *ret;
_lock_emul_atomic ();
ret = *p;
*p = n;
_unlock_emul_atomic ();
return ret;
}
17 changes: 16 additions & 1 deletion src/libbson/src/bson/bson-atomic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ enum bson_memory_order {
#define BSON_EMULATE_INT
#endif

/* CDRIVER-4264 Contrary to documentation, VS 2013 targeting x86 does not
* correctly/consistently provide _IntrinsicPointerExchange. */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* correctly/consistently provide _IntrinsicPointerExchange. */
* correctly/consistently provide _InterlockedExchangePointer. */

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you. I keep mistakening "Interlocked" as "Intrinsic". Will fix. 😑

#if defined(_MSC_VER) && _MSC_VER < 1900 && defined(_M_IX86)
#define BSON_EMULATE_PTR
#endif

#define DEF_ATOMIC_OP( \
MSVC_Intrinsic, GNU_Intrinsic, GNU_Legacy_Intrinsic, Order, ...) \
do { \
Expand Down Expand Up @@ -433,6 +439,11 @@ _bson_emul_atomic_int_compare_exchange_weak (int volatile *val,
int new_value,
enum bson_memory_order);

BSON_EXPORT (void *)
_bson_emul_atomic_ptr_exchange (void *volatile *val,
void *v,
enum bson_memory_order);

BSON_EXPORT (void)
bson_thrd_yield (void);

Expand Down Expand Up @@ -608,8 +619,10 @@ bson_atomic_ptr_exchange (void *volatile *ptr,
void *new_value,
enum bson_memory_order ord)
{
#if defined(BSON_EMULATE_PTR)
return _bson_emul_atomic_ptr_exchange (ptr, new_value, ord);
#elif defined(BSON_USE_LEGACY_GCC_ATOMICS)
/* The older __sync_val_compare_and_swap also takes oldval */
#if defined(BSON_USE_LEGACY_GCC_ATOMICS)
DEF_ATOMIC_OP (_InterlockedExchangePointer,
,
__sync_val_compare_and_swap,
Expand Down Expand Up @@ -750,6 +763,8 @@ BSON_EXPORT (int32_t) bson_atomic_int_add (volatile int32_t *p, int32_t n);
BSON_GNUC_DEPRECATED_FOR ("bson_atomic_int64_fetch_add")
BSON_EXPORT (int64_t) bson_atomic_int64_add (volatile int64_t *p, int64_t n);


#undef BSON_EMULATE_PTR
#undef BSON_EMULATE_INT32
#undef BSON_EMULATE_INT

Expand Down
49 changes: 0 additions & 49 deletions src/libmongoc/tests/TestSuite.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,45 +113,6 @@ static BSON_ONCE_FUN (_test_suite_ensure_mutex_once)
}


static void
_handle_signal (int signum)
{
const char *s = "\nProcess was interrupted by the delivery of a signal.\n";
const char *sigstr;
size_t n;
switch (signum) {
case SIGABRT:
sigstr = "SIGABRT - Abnormal termination";
break;
case SIGINT:
sigstr = "SIGINT - Interrupted";
break;
case SIGTERM:
sigstr = "SIGTERM - Termination requested";
break;
case SIGSEGV:
sigstr = "SIGSEGV - Access violation";
break;
default:
sigstr = "(Unknown signal delivered)";
}
#ifdef BSON_OS_UNIX
/* On POSIX these APIs are signal-safe */
n = write (STDERR_FILENO, s, strlen (s));
n = write (STDERR_FILENO, " ", 2);
n = write (STDERR_FILENO, sigstr, strlen (sigstr));
n = write (STDERR_FILENO, "\n", 1);
fsync (STDERR_FILENO);
#else
/* On Windows these APIs are signal-safe */
fprintf (stderr, "\n%s\n %s\n", s, sigstr);
fflush (stderr);
#endif
_Exit (signum);
(void) n;
}


void
TestSuite_Init (TestSuite *suite, const char *name, int argc, char **argv)
{
Expand All @@ -168,11 +129,6 @@ TestSuite_Init (TestSuite *suite, const char *name, int argc, char **argv)
suite->ctest_run = NULL;
_mongoc_array_init (&suite->match_patterns, sizeof (char *));

suite->prev_sigabrt = signal (SIGABRT, _handle_signal);
suite->prev_sigint = signal (SIGINT, _handle_signal);
suite->prev_sigterm = signal (SIGTERM, _handle_signal);
suite->prev_sigsegv = signal (SIGSEGV, _handle_signal);

for (i = 1; i < argc; i++) {
if (0 == strcmp ("-d", argv[i])) {
suite->flags |= TEST_DEBUGOUTPUT;
Expand Down Expand Up @@ -1100,11 +1056,6 @@ TestSuite_Destroy (TestSuite *suite)
}

_mongoc_array_destroy (&suite->match_patterns);

signal (SIGABRT, suite->prev_sigabrt);
signal (SIGINT, suite->prev_sigint);
signal (SIGTERM, suite->prev_sigterm);
signal (SIGSEGV, suite->prev_sigsegv);
}


Expand Down
5 changes: 0 additions & 5 deletions src/libmongoc/tests/TestSuite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,11 +674,6 @@ struct _TestSuite {
int silent;
bson_string_t *mock_server_log_buf;
FILE *mock_server_log;

void (*prev_sigabrt) (int);
void (*prev_sigint) (int);
void (*prev_sigterm) (int);
void (*prev_sigsegv) (int);
};


Expand Down
2 changes: 1 addition & 1 deletion src/libmongoc/tests/json-test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1930,7 +1930,7 @@ _install_json_test_suite_with_check (TestSuite *suite,
char joined[PATH_MAX];
char resolved[PATH_MAX];

snprintf (joined, PATH_MAX, "%s/%s", base, subdir);
bson_snprintf (joined, PATH_MAX, "%s/%s", base, subdir);
ASSERT (realpath (joined, resolved));

if (suite->ctest_run) {
Expand Down
38 changes: 17 additions & 21 deletions src/libmongoc/tests/test-mongoc-uri.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,6 @@
#include "test-libmongoc.h"
#include "test-conveniences.h"

#ifdef _WIN32
#define snprintf _snprintf
#endif

static void
test_mongoc_uri_new (void)
{
Expand Down Expand Up @@ -1579,7 +1575,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_t *uri;
bson_error_t err;

snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://CN=client,OU=kerneluser,O=10Gen,L=New York City,"
"ST=New York,[email protected]/?"
Expand Down Expand Up @@ -1617,7 +1613,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true&%s=key.pem&%s=ca.pem",
tls,
Expand Down Expand Up @@ -1650,7 +1646,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (
bson_snprintf (
url_buffer, sizeof (url_buffer), "mongodb://localhost/?%s=true", tls);
uri = mongoc_uri_new (url_buffer);

Expand All @@ -1670,7 +1666,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true&%s=pa$$word!&%s=encrypted.pem",
tls,
Expand Down Expand Up @@ -1703,7 +1699,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true&%s=true",
tls,
Expand All @@ -1730,7 +1726,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=foo.pem",
tlsCertificateKeyFile);
Expand All @@ -1740,7 +1736,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=foo.pem",
tlsCAFile);
Expand All @@ -1750,7 +1746,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true",
tlsAllowInvalidCertificates);
Expand All @@ -1764,7 +1760,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true",
tlsAllowInvalidHostnames);
Expand All @@ -1778,7 +1774,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=false&%s=foo.pem",
tls,
Expand All @@ -1789,7 +1785,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=false&%s=foo.pem",
tls,
Expand All @@ -1800,7 +1796,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=false&%s=true",
tls,
Expand All @@ -1815,7 +1811,7 @@ test_mongoc_uri_tls_ssl (const char *tls,
mongoc_uri_destroy (uri);


snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=false&%s=false",
tls,
Expand All @@ -1837,7 +1833,7 @@ test_mongoc_uri_tls_ssl (const char *tls,

/* Mixing options okay so long as they match */
capture_logs (true);
snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true&%s=true",
tls,
Expand All @@ -1849,7 +1845,7 @@ test_mongoc_uri_tls_ssl (const char *tls,

/* Same option with different values okay, latter overrides */
capture_logs (true);
snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true&%s=false",
tls,
Expand All @@ -1869,7 +1865,7 @@ test_mongoc_uri_tls_ssl (const char *tls,

/* Mixing options not okay if values differ */
capture_logs (false);
snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb://localhost/?%s=true&%s=false",
tls,
Expand All @@ -1892,7 +1888,7 @@ test_mongoc_uri_tls_ssl (const char *tls,

/* No conflict appears with implicit tls=true via SRV */
capture_logs (false);
snprintf (url_buffer,
bson_snprintf (url_buffer,
sizeof (url_buffer),
"mongodb+srv://a.b.c/?%s=foo.pem",
tlsCAFile);
Expand Down