Skip to content

Fixing unit tests for mem-cpy, -move and -set functions #112

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

Closed
wants to merge 15 commits into from
Closed

Fixing unit tests for mem-cpy, -move and -set functions #112

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
665c46b
mod: Disable memcpy interposition by default (#103)
db7 Oct 2, 2025
9cc3c28
cmake: Add option to measure test coverage
db7 Sep 20, 2025
31dc5b4
Fixing unit tests for mem-cpy, -move and -set functions
Oct 20, 2025
7a7c69a
memmove test - fixed
Oct 20, 2025
661e73d
Mocking in clang did not run with the functions implemented for testi…
Oct 20, 2025
f1e6208
Removed the mock/fake functions from memcpy_test as Clang overrid…
Oct 23, 2025
a32ba0c
Merge branch 'main' into main
georgeta-igna Oct 23, 2025
dec8ed9
Allocated memory for the destination arg of each mem handling function
Oct 23, 2025
bff191d
Removed the mock functions from memcpy_test as clang overwrites the r…
Oct 23, 2025
5935f18
Adding back real_sym function in memcpy_test.c to match with the call…
Oct 23, 2025
043e4db
Adding back symbol and enabled for the interpose calls to compile
Oct 23, 2025
0547e80
Fixing the asan test
Oct 24, 2025
3e652bf
Removed the validation for ret field
Oct 24, 2025
f40b0e7
Changes for building under linux alpine.
Oct 27, 2025
23496e5
Merge branch 'main' into main
georgeta-igna Oct 27, 2025
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
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

add_compile_options(-Wall -Wextra -Werror)

option(DICE_INTERPOSE_MEMCPY "Build dice-memcpy (memcpy, memset, memmove)" off)
option(DICE_INTERPOSE_MEMCPY "Build dice-memcpy (memcpy, memset, memmove)" on)

# LTO setup
option(DICE_LTO "Build Dice with LTO" on)
Expand Down
136 changes: 35 additions & 101 deletions test/interpose/memcpy_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,34 +13,6 @@
#include <dice/pubsub.h>
#include <dice/events/memcpy.h>

static void *symbol;
/* we need to declare this as noinline, otherwise the optimization of the
* compiler gets rid of the symbol. */
static __attribute__((noinline)) void
enable(void *foo)
{
symbol = foo;
}
static __attribute__((noinline)) void
disable(void)
{
symbol = NULL;
}
static inline bool
enabled(void)
{
return symbol != NULL;
}

void *
real_sym(const char *name, const char *ver)
{
(void)ver;
if (!enabled())
return _real_sym(name, ver);
return symbol;
}

/* Expects struct to match this:
*
* struct memcpy_event {
Expand Down Expand Up @@ -72,151 +44,113 @@ struct memmove_event E_memmove;
*/
struct memset_event E_memset;

/* mock implementation of functions */
void *
fake_memcpy(void *dest ,const void *src ,size_t num)
{
/* check that every argument is as expected */
ensure(dest == E_memcpy.dest);
ensure(src == E_memcpy.src);
ensure(num == E_memcpy.num);
/* return expected value */
return E_memcpy.ret;
}
void *
fake_memmove(void *dest ,const void *src ,size_t count)
{
/* check that every argument is as expected */
ensure(dest == E_memmove.dest);
ensure(src == E_memmove.src);
ensure(count == E_memmove.count);
/* return expected value */
return E_memmove.ret;
}
void *
fake_memset(void *ptr, int value, size_t num)
{
/* check that every argument is as expected */
ensure(ptr == E_memset.ptr);
ensure(value == E_memset.value);
ensure(num == E_memset.num);
/* return expected value */
return E_memset.ret;
}

#define ASSERT_FIELD_EQ(E, field) \
ensure(memcmp(&ev->field, &(E)->field, sizeof(__typeof((E)->field))) == 0);
ensure(memcmp(&ev->field, &(E)->field, sizeof((E)->field)) == 0);

PS_SUBSCRIBE(INTERCEPT_BEFORE, EVENT_MEMCPY, {
if (!enabled())
return PS_STOP_CHAIN;
struct memcpy_event *ev = EVENT_PAYLOAD(ev);
log_printf("ev->dest %p\n", &ev->dest);
log_printf("&(&E_memcpy)->dest %p\n", &(&E_memcpy)->dest);
log_printf("memcmp(&ev->dest, &(&E_memcpy)->dest, sizeof((&E_memcpy)->dest)) == 0 %d\n", memcmp(&ev->dest, &(&E_memcpy)->dest, sizeof(__typeof((&E_memcpy)->dest))) == 0);
Copy link

Choose a reason for hiding this comment

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

redundant typeof() ?

Copy link

Choose a reason for hiding this comment

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

should we really commit these log messages?

If yes, I recommend defining a macro to avoid writing the same text twice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, I am not planning to merge with these. I want to understand better why for the arm with clang the next ensure fails: https://github.com/open-s4c/dice/actions/runs/18745327427/job/53480906689

ASSERT_FIELD_EQ(&E_memcpy, dest);
ASSERT_FIELD_EQ(&E_memcpy, src);
Copy link

Choose a reason for hiding this comment

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

can we add some check that the interceptor really got called? Or is that already checked elsewhere?

ASSERT_FIELD_EQ(&E_memcpy, num);
})

PS_SUBSCRIBE(INTERCEPT_AFTER, EVENT_MEMCPY, {
if (!enabled())
return PS_STOP_CHAIN;
struct memcpy_event *ev = EVENT_PAYLOAD(ev);
log_printf("ev->dest %p\n", &ev->dest);
log_printf("&(&E_memcpy)->dest %p\n", &(&E_memcpy)->dest);
log_printf("memcmp(&ev->dest, &(&E_memcpy)->dest, sizeof((&E_memcpy)->dest)) == 0 %d\n", memcmp(&ev->dest, &(&E_memcpy)->dest, sizeof(__typeof((&E_memcpy)->dest))) == 0);
ASSERT_FIELD_EQ(&E_memcpy, dest);
ASSERT_FIELD_EQ(&E_memcpy, src);
ASSERT_FIELD_EQ(&E_memcpy, num);
ASSERT_FIELD_EQ(&E_memcpy, ret);
ensure(E_memcpy.dest == E_memcpy.ret);
})
PS_SUBSCRIBE(INTERCEPT_BEFORE, EVENT_MEMMOVE, {
if (!enabled())
return PS_STOP_CHAIN;
struct memmove_event *ev = EVENT_PAYLOAD(ev);
ASSERT_FIELD_EQ(&E_memmove, dest);
ASSERT_FIELD_EQ(&E_memmove, src);
ASSERT_FIELD_EQ(&E_memmove, count);
})

PS_SUBSCRIBE(INTERCEPT_AFTER, EVENT_MEMMOVE, {
if (!enabled())
return PS_STOP_CHAIN;
struct memmove_event *ev = EVENT_PAYLOAD(ev);
ASSERT_FIELD_EQ(&E_memmove, dest);
ASSERT_FIELD_EQ(&E_memmove, src);
ASSERT_FIELD_EQ(&E_memmove, count);
ASSERT_FIELD_EQ(&E_memmove, ret);
})
PS_SUBSCRIBE(INTERCEPT_BEFORE, EVENT_MEMSET, {
if (!enabled())
return PS_STOP_CHAIN;
struct memset_event *ev = EVENT_PAYLOAD(ev);
ASSERT_FIELD_EQ(&E_memset, ptr);
ASSERT_FIELD_EQ(&E_memset, value);
ASSERT_FIELD_EQ(&E_memset, num);
})

PS_SUBSCRIBE(INTERCEPT_AFTER, EVENT_MEMSET, {
if (!enabled())
return PS_STOP_CHAIN;
struct memset_event *ev = EVENT_PAYLOAD(ev);
ASSERT_FIELD_EQ(&E_memset, ptr);
ASSERT_FIELD_EQ(&E_memset, value);
ASSERT_FIELD_EQ(&E_memset, num);
ASSERT_FIELD_EQ(&E_memset, ret);
Copy link

Choose a reason for hiding this comment

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

whitespace?

})


static void
event_init(void *ptr, size_t n)
{
char *buf = ptr;
for (size_t i = 0; i < n; i++)
buf[i] = rand() % 256;
}

/* test case */
/* test cases */

static void
test_memcpy(void)
{
/* initialize event with random content */
event_init(&E_memcpy, sizeof(struct memcpy_event));
/* call memcpy with arguments */
enable(fake_memcpy);
E_memcpy.dest = malloc(10);
char hello[] = "Hello!";
E_memcpy.src= hello;
E_memcpy.num = strlen(E_memcpy.src) + 1;
E_memcpy.ret = E_memcpy.dest;
void * ret = //
memcpy( //
E_memcpy.dest, //
E_memcpy.src, //
E_memcpy.num );
ensure(ret == E_memcpy.ret);
disable();
ensure(ret == E_memcpy.dest);
ensure(strcmp(E_memcpy.dest, E_memcpy.src) == 0);
free(E_memcpy.dest);
E_memcpy.dest = NULL;
}
static void
test_memmove(void)
{
/* initialize event with random content */
event_init(&E_memmove, sizeof(struct memmove_event));
/* call memmove with arguments */
enable(fake_memmove);
E_memmove.dest = malloc(10);
char hello[] = "Hi there!";
E_memmove.src= hello;
E_memmove.count = 2;
E_memmove.ret = E_memmove.dest;
void * ret = //
memmove( //
E_memmove.dest, //
E_memmove.src, //
E_memmove.count );
ensure(ret == E_memmove.ret);
disable();
ensure(ret == E_memmove.dest);
log_printf("memmove res %s\n", (char *)E_memmove.dest);
ensure(strncmp((char *)E_memmove.dest, "Hi", 2) == 0);
free(E_memmove.dest);
E_memmove.dest = NULL;
}
static void
test_memset(void)
{
/* initialize event with random content */
event_init(&E_memset, sizeof(struct memset_event));
/* call memset with arguments */
enable(fake_memset);
E_memset.ptr = malloc(5);
E_memset.value= 3;
E_memset.num = 2;
Copy link

Choose a reason for hiding this comment

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

maybe better to use compound literal for this initialization?

image

E_memset.ret = E_memset.ptr;
Copy link

Choose a reason for hiding this comment

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

I think they are always equal, so we should probably remove this .ret field and just compare with E_memset.ptr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

E_memset.ret is null without this and it is checked somewhere in the main test: ensure(ret == E_memset.ptr);

Copy link

Choose a reason for hiding this comment

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

I mean there's no point to have the field at all, it can be removed from the struct. Then you can still ensure(ret == E_memset.ptr);

void * ret = //
memset( //
E_memset.ptr, //
E_memset.value, //
E_memset.num );
ensure(ret == E_memset.ret);
disable();
ensure(ret == E_memset.ptr);
free(E_memset.ptr);
E_memset.ptr = NULL;
}

int
Expand Down
Loading