Skip to content

Conversation

@georgeta-igna
Copy link
Collaborator

No description provided.

db7 and others added 2 commits October 2, 2025 09:33
* mod: Disable memcpy interposition by default

Signed-off-by: Diogo Behrens <[email protected]>

* cicd: Trigger pipeline on pull requests

Signed-off-by: Diogo Behrens <[email protected]>

* cicd: Enable memcpy interposition where safe

Signed-off-by: Diogo Behrens <[email protected]>

---------

Signed-off-by: Diogo Behrens <[email protected]>
…ng. enable function did not replace the actual call to functions. For this reason I used #define mem_func fake_mem_func to tell the preprocessor to replace every call to mem func with calls to the fake functions.
@db7
Copy link
Member

db7 commented Oct 21, 2025

This is not passing in alpine

gigna and others added 2 commits October 23, 2025 12:21
…the intended return values of the mocks.

    Signed-off-by: gigna <[email protected]>
Signed-off-by: georgeta-igna <[email protected]>
DIvanov503
DIvanov503 previously approved these changes Oct 23, 2025
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);

Choose a reason for hiding this comment

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

redundant typeof() ?

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_memset, ptr);
ASSERT_FIELD_EQ(&E_memset, value);
ASSERT_FIELD_EQ(&E_memset, num);
ASSERT_FIELD_EQ(&E_memset, ret);

Choose a reason for hiding this comment

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

whitespace?

E_memset.ptr = malloc(5);
E_memset.value= 3;
E_memset.num = 2;
E_memset.ret = E_memset.ptr;

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

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

enable(fake_memset);
E_memset.ptr = malloc(5);
E_memset.value= 3;
E_memset.num = 2;

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

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

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?

@georgeta-igna
Copy link
Collaborator Author

Replaced by #122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants