[libc-commits] [libc] [libc] Fix readlink tests on 32-bit systems (PR #98168)

[Mikhail R. Gadelha via libc-commits]

https://github.com/mikhailramalho created https://github.com/llvm/llvm-project/pull/98168

This patch changes the call sizeof(<Cstring>) to call malloc with the actual lenght of the string to allocate space for buf.

The test case worked in 64-bit systems because the sizeof(CString) was big enough (19 bytes) to hold the contents of FILENAME, but in 32-bit systems sizeof(CString) is only 12 bytes long, causing a truncation of FILENAME when placing it in buf.

I also changed the vla to a malloc instead to silence a "variable length arrays are a C99 feature [-Wvla-extension]" warning.

>From dea35afcf91b3b35b4fcbe7dd7a3e23272e6cb2c Mon Sep 17 00:00:00 2001
From: "Mikhail R. Gadelha" <mikhail at igalia.com>
Date: Tue, 9 Jul 2024 12:10:18 -0300
Subject: [PATCH] [libc] Fix readlink tests on 32-bit systems

This patch changes the call sizeof(<Cstring>) to call malloc with the
actual lenght of the string to allocate space for buf.

The test case worked in 64-bit systems because the sizeof(CString) was
big enough (19 bytes) to hold the contents of FILENAME, but in 32-bit
systems sizeof(CString) is only 12 bytes long, causing a truncation of
FILENAME when placing it in buf.

I also changed the vla to a malloc instead to silence a "variable
length arrays are a C99 feature [-Wvla-extension]" warning.
---
 libc/test/src/unistd/CMakeLists.txt      |  2 ++
 libc/test/src/unistd/readlink_test.cpp   | 12 ++++++++----
 libc/test/src/unistd/readlinkat_test.cpp | 16 ++++++++++------
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/libc/test/src/unistd/CMakeLists.txt b/libc/test/src/unistd/CMakeLists.txt
index 1a1e01e50f4e8..332455b791aee 100644
--- a/libc/test/src/unistd/CMakeLists.txt
+++ b/libc/test/src/unistd/CMakeLists.txt
@@ -262,6 +262,7 @@ add_libc_unittest(
     libc.include.unistd
     libc.src.errno.errno
     libc.src.unistd.readlink
+    libc.src.string.string_utils
     libc.src.unistd.symlink
     libc.src.unistd.unlink
     libc.src.__support.CPP.string_view
@@ -279,6 +280,7 @@ add_libc_unittest(
     libc.include.unistd
     libc.src.errno.errno
     libc.src.unistd.readlinkat
+    libc.src.string.string_utils
     libc.src.unistd.symlink
     libc.src.unistd.unlink
     libc.src.__support.CPP.string_view
diff --git a/libc/test/src/unistd/readlink_test.cpp b/libc/test/src/unistd/readlink_test.cpp
index 49ab9c24f4024..e29a6bd3e374b 100644
--- a/libc/test/src/unistd/readlink_test.cpp
+++ b/libc/test/src/unistd/readlink_test.cpp
@@ -8,6 +8,7 @@
 
 #include "src/__support/CPP/string_view.h"
 #include "src/errno/libc_errno.h"
+#include "src/string/string_utils.h"
 #include "src/unistd/readlink.h"
 #include "src/unistd/symlink.h"
 #include "src/unistd/unlink.h"
@@ -30,17 +31,20 @@ TEST(LlvmLibcReadlinkTest, CreateAndUnlink) {
   //   3. Cleanup the symlink created in step #1.
   ASSERT_THAT(LIBC_NAMESPACE::symlink(LINK_VAL, LINK), Succeeds(0));
 
-  char buf[sizeof(LINK_VAL)];
-  ssize_t len = LIBC_NAMESPACE::readlink(LINK, buf, sizeof(buf));
+  size_t buf_len = LIBC_NAMESPACE::internal::string_length(FILENAME);
+  char *buf = static_cast<char *>(malloc(buf_len));
+  ssize_t len = LIBC_NAMESPACE::readlink(LINK, buf, buf_len);
   ASSERT_ERRNO_SUCCESS();
   ASSERT_EQ(cpp::string_view(buf, len), cpp::string_view(LINK_VAL));
 
   ASSERT_THAT(LIBC_NAMESPACE::unlink(LINK), Succeeds(0));
+  free(buf);
 }
 
 TEST(LlvmLibcReadlinkTest, ReadlinkInNonExistentPath) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
-  char buf[8];
-  ASSERT_THAT(LIBC_NAMESPACE::readlink("non-existent-link", buf, sizeof(buf)),
+  constexpr auto len = 8;
+  char buf[len];
+  ASSERT_THAT(LIBC_NAMESPACE::readlink("non-existent-link", buf, len),
               Fails(ENOENT));
 }
diff --git a/libc/test/src/unistd/readlinkat_test.cpp b/libc/test/src/unistd/readlinkat_test.cpp
index 7e1ded5f8a4a1..f2924d3b8f6c5 100644
--- a/libc/test/src/unistd/readlinkat_test.cpp
+++ b/libc/test/src/unistd/readlinkat_test.cpp
@@ -8,6 +8,7 @@
 
 #include "src/__support/CPP/string_view.h"
 #include "src/errno/libc_errno.h"
+#include "src/string/string_utils.h"
 #include "src/unistd/readlinkat.h"
 #include "src/unistd/symlink.h"
 #include "src/unistd/unlink.h"
@@ -32,18 +33,21 @@ TEST(LlvmLibcReadlinkatTest, CreateAndUnlink) {
   //   3. Cleanup the symlink created in step #1.
   ASSERT_THAT(LIBC_NAMESPACE::symlink(LINK_VAL, LINK), Succeeds(0));
 
-  char buf[sizeof(LINK_VAL)];
-  ssize_t len = LIBC_NAMESPACE::readlinkat(AT_FDCWD, LINK, buf, sizeof(buf));
+  size_t buf_len = LIBC_NAMESPACE::internal::string_length(FILENAME);
+  char *buf = static_cast<char *>(malloc(buf_len));
+  ssize_t len = LIBC_NAMESPACE::readlinkat(AT_FDCWD, LINK, buf, buf_len);
   ASSERT_ERRNO_SUCCESS();
   ASSERT_EQ(cpp::string_view(buf, len), cpp::string_view(LINK_VAL));
 
   ASSERT_THAT(LIBC_NAMESPACE::unlink(LINK), Succeeds(0));
+  free(buf);
 }
 
 TEST(LlvmLibcReadlinkatTest, ReadlinkInNonExistentPath) {
   using LIBC_NAMESPACE::testing::ErrnoSetterMatcher::Fails;
-  char buf[8];
-  ASSERT_THAT(LIBC_NAMESPACE::readlinkat(AT_FDCWD, "non-existent-link", buf,
-                                         sizeof(buf)),
-              Fails(ENOENT));
+  constexpr auto len = 8;
+  char buf[len];
+  ASSERT_THAT(
+      LIBC_NAMESPACE::readlinkat(AT_FDCWD, "non-existent-link", buf, len),
+      Fails(ENOENT));
 }