libc: strlcpy/strlcat shouldn't bzero the rest of buf

[gburgessiv]

When running Bionic's testsuite over llvm-libc, tests broke because e.g.,

const char *str = "abc";
char buf[7]{"111111"};
strlcpy(buf, str, 7);
ASSERT_EQ(buf, {'1', '1', '1', '\0', '\0', '\0', '\0'});

On my machine (Debian w/ glibc and clang-16), a printf loop over buf gets unrolled into a series of const printf at compile-time:

printf("%d\n", '1');
printf("%d\n", '1');
printf("%d\n", '1');
printf("%d\n", 0);
printf("%d\n", '1');
printf("%d\n", '1');
printf("%d\n", 0);

Seems best to match existing precedent here.

[llvmbot]

@llvm/pr-subscribers-libc

Author: George Burgess IV (gburgessiv)

Changes

When running Bionic's testsuite over llvm-libc, tests broke because e.g.,

const char *str = "abc";
char buf[7]{"111111"};
strlcpy(buf, str, 7);
ASSERT_EQ(buf, {'1', '1', '1', '\0', '\0', '\0', '\0'});

On my machine (Debian w/ glibc and clang-16), a printf loop over buf gets unrolled into a series of const printf at compile-time:

printf("%d\n", '1');
printf("%d\n", '1');
printf("%d\n", '1');
printf("%d\n", 0);
printf("%d\n", '1');
printf("%d\n", '1');
printf("%d\n", 0);

Seems best to match existing precedent here.

---

Full diff: https://github.com/llvm/llvm-project/pull/114259.diff

3 Files Affected:

• (modified) libc/src/string/string_utils.h (+1-1)
• (modified) libc/test/src/string/strlcat_test.cpp (+9)
• (modified) libc/test/src/string/strlcpy_test.cpp (+1-2)

diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
index 78381e46e480dd..240b28f15718a8 100644
--- a/libc/src/string/string_utils.h
+++ b/libc/src/string/string_utils.h
@@ -221,7 +221,7 @@ LIBC_INLINE size_t strlcpy(char *__restrict dst, const char *__restrict src,
     return len;
   size_t n = len < size - 1 ? len : size - 1;
   inline_memcpy(dst, src, n);
-  inline_bzero(dst + n, size - n);
+  dst[n] = '\0';
   return len;
 }
 
diff --git a/libc/test/src/string/strlcat_test.cpp b/libc/test/src/string/strlcat_test.cpp
index 1ffa4b0e921e2b..5757fc92b39d2a 100644
--- a/libc/test/src/string/strlcat_test.cpp
+++ b/libc/test/src/string/strlcat_test.cpp
@@ -27,6 +27,15 @@ TEST(LlvmLibcStrlcatTest, Smaller) {
   EXPECT_STREQ(buf, "abcd");
 }
 
+TEST(LlvmLibcStrlcatTest, SmallerNoOverwriteAfter0) {
+  const char *str = "cd";
+  char buf[8]{"ab\0\0efg"};
+
+  EXPECT_EQ(LIBC_NAMESPACE::strlcat(buf, str, 8), size_t(4));
+  EXPECT_STREQ(buf, "abcd");
+  EXPECT_STREQ(buf + 5, "fg");
+}
+
 TEST(LlvmLibcStrlcatTest, No0) {
   const char *str = "cd";
   char buf[7]{"ab"};
diff --git a/libc/test/src/string/strlcpy_test.cpp b/libc/test/src/string/strlcpy_test.cpp
index 5a1e30c12963f3..ecf0e925a265c3 100644
--- a/libc/test/src/string/strlcpy_test.cpp
+++ b/libc/test/src/string/strlcpy_test.cpp
@@ -25,6 +25,5 @@ TEST(LlvmLibcStrlcpyTest, Smaller) {
 
   EXPECT_EQ(LIBC_NAMESPACE::strlcpy(buf, str, 7), size_t(3));
   EXPECT_STREQ(buf, "abc");
-  for (const char *p = buf + 3; p < buf + 7; p++)
-    EXPECT_EQ(*p, '\0');
+  EXPECT_STREQ(buf + 4, "11");
 }

[nickdesaulniers]

Looks like this mistake has existed since the introduction of this helper b118330

Thanks for the patch!

[gburgessiv]

Thanks for reviewing! I'll press the button once buildkite checks pass.