Skip to content

Commit

Permalink
Add NOINLINE to a couple of test functions
Browse files Browse the repository at this point in the history
One would expect that inline or not, a function's behaviour must
not change. But it doesn't seem to be the case when the tests are
built with clang in Release mode. Both normal LLVM clang and Apple
clang.

It is not clear why, but the function
test_encode_uint_custom_size() behaves wrong when inlined.
Specifically in Release mode with clang. Even with ASAN it
works. Still, this specific build fails reliably.

Debugging is quite complicated due to the optimizations messing up
the code. What was found is that seems like when the function is
called 2 times on 2 different buffers, the second time it is like
if the function had no effect.

Here is what was tried:

* Any attempt to observe the buffers with prints made the tests
  pass. Even if the prints are added **after** the loop in
  test_compare_uint().

* In RelWithDebInfo it fails, even though prints in LLDB show the
  buffers being filled with correct data. However when the code
  enters mp_compare_uint(), the buffers again look the same.

* The attempts to replace with 2-dimensional array with an array
  of structs didn't change anything.

* The increase of MP_NUMBER_MAX_LEN and MP_NUMBER_CODEC_COUNT
  didn't have any effect either.

* Inlining of test_compare_uint() body into main() with the values
  causing the failure would somehow pass.

* Adding a printf("...") to each iteration in
  test_encode_uint_all_sizes() would pass.

Given that this doesn't seem to be anyhow related to memory
corruption (including stack overflow), the conclusion so far is
that it could be a compiler bug.
  • Loading branch information
Gerold103 committed Feb 4, 2025
1 parent a8c3c6b commit 66327ce
Showing 1 changed file with 17 additions and 2 deletions.
19 changes: 17 additions & 2 deletions test/msgpuck.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@
#define lengthof(array) (sizeof(array) / sizeof((array)[0]))
#endif

#if __has_attribute(noinline) || defined(__GNUC__)
# define NOINLINE __attribute__((noinline))
#else
# define NOINLINE
#endif

static char buf[BUF_MAXLEN + 1];
static char str[STRBIN_MAXLEN];
static char *data = buf + 1; /* use unaligned address to fail early */
Expand Down Expand Up @@ -742,7 +748,11 @@ test_next_on_maps(void)
return check_plan();
}

static bool
/**
* When inlined in Release in clang, this function behaves weird. Looking
* sometimes like if its call had no effect even though it did encoding.
*/
static NOINLINE bool
test_encode_uint_custom_size(char *buf, uint64_t val, int size)
{
char *pos;
Expand Down Expand Up @@ -779,7 +789,12 @@ test_encode_uint_custom_size(char *buf, uint64_t val, int size)
return false;
}

static bool
/**
* Unlike test_encode_uint_custom_size(), this one doesn't seem to behave
* strange when inlined, but perhaps it just wasn't used in all the same ways as
* the former.
*/
static NOINLINE bool
test_encode_int_custom_size(char *buf, int64_t val, int size)
{
char *pos;
Expand Down

0 comments on commit 66327ce

Please sign in to comment.