[libc-commits] [PATCH] D136865: [libc] Improve testing of mem functions

Clement Courbet via Phabricator via libc-commits libc-commits at lists.llvm.org
Fri Oct 28 01:05:47 PDT 2022


courbet added inline comments.


================
Comment at: libc/test/src/string/memory_utils/memory_check_utils.h:33
+// Simple structure to allocate a buffer of a particular size.
+// When ASAN is present it also poison the whole memory.
+// This is a utility class to be used by Buffer below, do not use directly.
----------------
`poisons`


================
Comment at: libc/test/src/string/memory_utils/memory_check_utils.h:88
+// Copy one span to another.
+static inline void Copy(cpp::span<char> dst, const cpp::span<char> src) {
+  assert(dst.size() == src.size());
----------------
Maybe call this `ReferenceCopy` top make it clear that this is assumed to be correct.


================
Comment at: libc/test/src/string/memory_utils/memory_check_utils.h:88
+// Copy one span to another.
+static inline void Copy(cpp::span<char> dst, const cpp::span<char> src) {
+  assert(dst.size() == src.size());
----------------
courbet wrote:
> Maybe call this `ReferenceCopy` top make it clear that this is assumed to be correct.
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 :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136865/new/

https://reviews.llvm.org/D136865



More information about the libc-commits mailing list