This is an archive of the discontinued LLVM Phabricator instance.

[libc] Improve testing of mem functions
ClosedPublic

Authored by gchatelet on Oct 27 2022, 10:07 AM.

Details

Summary

This patch extracts the testing logic from op_tests.cpp into
memory_check_utils.h so we can reuse it for mem* function integration
tests.

This makes testing consistent and thorough.
For instance this catches a bug that got unnoticed during submission of
D136595 and D135134. Integration test for memcmp was only testing a
single size.

This also leverages ASAN to make sure that data is not read / written
outside permitted boundaries

Diff Detail

Event Timeline

gchatelet created this revision.Oct 27 2022, 10:07 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 27 2022, 10:07 AM
gchatelet requested review of this revision.Oct 27 2022, 10:07 AM

Fix documentation and make code less convoluted

courbet added inline comments.Oct 28 2022, 1:00 AM
libc/test/src/string/bcmp_test.cpp
45

SizeSweep or AllSizes would be clearer IMO (here and below).

46

kMaxSize (here and below)

courbet added inline comments.Oct 28 2022, 1:05 AM
libc/test/src/string/memory_utils/memory_check_utils.h
34

poisons

89

Maybe call this ReferenceCopy top make it clear that this is assumed to be correct.

89

Can we mark this with attribute nobuiltin to make it clear that this is the implementation we get ? Or maybe even add an #ifndef NOBUILTIN #error "should be built with -fno-builtin" (I'm not sure whether clang exports such a define, but if it does...).

It would be a shame if we were to compare memcpy against memcpy :)

gchatelet marked 5 inline comments as done.Oct 31 2022, 2:57 PM
gchatelet updated this revision to Diff 472146.Oct 31 2022, 2:59 PM
  • fixup! [libc] Improve testing of mem functions

I'm submitting this patch. Let me know if you have any other comments.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 2 2022, 1:55 AM
This revision was automatically updated to reflect the committed changes.