-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Create a poor-developer's msan for libc wide read functions. #170586
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.
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
base: main
Are you sure you want to change the base?
Conversation
Most libcs optimize functions like strlen by reading in chunks larger than a single character. As part of "the implementation", they can legally do this as long as they are careful not to read invalid memory. However, such tricks prevents those functions from being tested under the various sanitizers. This PR creates a test framework that can report when one of these functions read or write in an invalid way without using the sanitizers.
|
@llvm/pr-subscribers-libc Author: None (Sterling-Augustine) ChangesMost libcs optimize functions like strlen by reading in chunks larger than a single character. As part of "the implementation", they can legally do this as long as they are careful not to read invalid memory. However, such tricks prevents those functions from being tested under the various sanitizers. This PR creates a test framework that can report when one of these functions read or write in an invalid way without using the sanitizers. Full diff: https://github.com/llvm/llvm-project/pull/170586.diff 1 Files Affected:
diff --git a/libc/test/src/strings/CMakeLists.txt b/libc/test/src/strings/CMakeLists.txt
index 5f70dc024f6ce..0ccd0dc302943 100644
--- a/libc/test/src/strings/CMakeLists.txt
+++ b/libc/test/src/strings/CMakeLists.txt
@@ -110,5 +110,15 @@ add_libc_test(
libc.src.strings.strncasecmp_l
)
+add_libc_test(
+ wide_read_memory_test
+ SUITE
+ libc-strings-tests
+ SRCS
+ wide_read_memory_test.cpp
+ DEPENDS
+ libc.src.string.strlen
+)
+
add_libc_multi_impl_test(bcmp libc-strings-tests SRCS bcmp_test.cpp)
add_libc_multi_impl_test(bzero libc-strings-tests SRCS bzero_test.cpp)
|
🐧 Linux x64 Test Results✅ The build succeeded and no tests ran. This is expected in some build configurations. |
| SUITE | ||
| libc-strings-tests | ||
| SRCS | ||
| wide_read_memory_test.cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file supposed to be committed? It's missing.
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- libc/test/src/strings/wide_read_memory_test.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/libc/test/src/strings/wide_read_memory_test.cpp b/libc/test/src/strings/wide_read_memory_test.cpp
index e353d49dd..5e2e02bb9 100644
--- a/libc/test/src/strings/wide_read_memory_test.cpp
+++ b/libc/test/src/strings/wide_read_memory_test.cpp
@@ -27,14 +27,13 @@
namespace LIBC_NAMESPACE_DECL {
-
class LlvmLibcWideAccessMemoryTest : public testing::Test {
char *page0_;
char *page1_;
char *page2_;
size_t page_size;
- public:
+public:
void SetUp() override {
page_size = getpagesize();
page0_ =
@@ -65,7 +64,7 @@ class LlvmLibcWideAccessMemoryTest : public testing::Test {
// 512-byte arrays. The termination condition, eg, end-of string or character
// being searched for, should be near the end of the data.
template <typename TestFunc>
- void TestMemoryAccess(const std::vector<char>& buf, TestFunc func) {
+ void TestMemoryAccess(const std::vector<char> &buf, TestFunc func) {
// Run func on data near the start boundary of valid memory.
for (unsigned long offset = 0;
offset < std::alignment_of<std::max_align_t>::value; ++offset) {
@@ -76,7 +75,7 @@ class LlvmLibcWideAccessMemoryTest : public testing::Test {
// Run func on data near the end boundary of valid memory.
for (unsigned long offset = 0;
offset < std::alignment_of<std::max_align_t>::value; ++offset) {
- char *test_addr = page2_ - buf.size() - offset - 1;
+ char *test_addr = page2_ - buf.size() - offset - 1;
assert(test_addr + buf.size() < page2_);
BasicMemCopy(test_addr, buf.data(), buf.size());
func(test_addr);
@@ -89,10 +88,10 @@ TEST_F(LlvmLibcWideAccessMemoryTest, StringLength) {
std::vector<char> buf(1536, 'a');
// Make sure it is null terminated.
buf.push_back('\0');
- this->TestMemoryAccess(buf, [this, buf](const char* test_data) {
+ this->TestMemoryAccess(buf, [this, buf](const char *test_data) {
// -1 for the null character.
ASSERT_EQ(internal::string_length(test_data), size_t(buf.size() - 1));
});
}
-}
+} // namespace LIBC_NAMESPACE_DECL
|
| #include <type_traits> | ||
| #include <vector> | ||
|
|
||
| #include <assert.h> | ||
| #include <sys/mman.h> | ||
| #include <unistd.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to replace these with our internal implementations / headers. I see we already have mprotect, mmap, munmap. You might need to add getpagesize entrypoint (in a separate PR). Also you can use our inline_memcpy instead of BasicMemCopy.
One good thing with using those is that with correct dependency, this test will be skipped automatically for any build configurations where those syscalls are not available / implemented.
Most libcs optimize functions like strlen by reading in chunks larger than a single character. As part of "the implementation", they can legally do this as long as they are careful not to read invalid memory.
However, such tricks prevents those functions from being tested under the various sanitizers.
This PR creates a test framework that can report when one of these functions read or write in an invalid way without using the sanitizers.