[libc-commits] [libc] f6ba352 - [libc] Add nullptr check option to printf %s

[Michael Jones via libc-commits]

Author: Michael Jones
Date: 2023-08-07T14:47:09-07:00
New Revision: f6ba352988b2734cbbd21df98e5e5536d3cad98f

URL: https://github.com/llvm/llvm-project/commit/f6ba352988b2734cbbd21df98e5e5536d3cad98f
DIFF: https://github.com/llvm/llvm-project/commit/f6ba352988b2734cbbd21df98e5e5536d3cad98f.diff

LOG: [libc] Add nullptr check option to printf %s

Some printf implementations perform a null check on pointers passed to
%s. While that's not in the standard, this patch adds it as an option
for compatibility. It also puts a similar check in %n behind the same
flag.

Reviewed By: lntue

Differential Revision: https://reviews.llvm.org/D156923

Added: 
    

Modified: 
    libc/src/stdio/printf_core/printf_config.h
    libc/src/stdio/printf_core/string_converter.h
    libc/src/stdio/printf_core/write_int_converter.h
    libc/test/src/stdio/sprintf_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/stdio/printf_core/printf_config.h b/libc/src/stdio/printf_core/printf_config.h
index 2c0bd11cdecf1f..e1d9654f3affe4 100644
--- a/libc/src/stdio/printf_core/printf_config.h
+++ b/libc/src/stdio/printf_core/printf_config.h
@@ -39,4 +39,6 @@
 
 // TODO(michaelrj): Move the other printf configuration options into this file.
 
+// LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
+
 #endif // LLVM_LIBC_SRC_STDIO_PRINTF_CORE_PRINTF_CONFIG_H

diff  --git a/libc/src/stdio/printf_core/string_converter.h b/libc/src/stdio/printf_core/string_converter.h
index 137bac6e76bea9..460f195f5d9032 100644
--- a/libc/src/stdio/printf_core/string_converter.h
+++ b/libc/src/stdio/printf_core/string_converter.h
@@ -22,9 +22,15 @@ namespace printf_core {
 
 LIBC_INLINE int convert_string(Writer *writer, const FormatSection &to_conv) {
   size_t string_len = 0;
+  const char *str_ptr = reinterpret_cast<const char *>(to_conv.conv_val_ptr);
 
-  for (char *cur_str = reinterpret_cast<char *>(to_conv.conv_val_ptr);
-       cur_str[string_len]; ++string_len) {
+#ifndef LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
+  if (str_ptr == nullptr) {
+    str_ptr = "null";
+  }
+#endif // LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
+
+  for (const char *cur_str = (str_ptr); cur_str[string_len]; ++string_len) {
     ;
   }
 
@@ -42,8 +48,7 @@ LIBC_INLINE int convert_string(Writer *writer, const FormatSection &to_conv) {
     RET_IF_RESULT_NEGATIVE(writer->write(' ', padding_spaces));
   }
 
-  RET_IF_RESULT_NEGATIVE(writer->write(
-      {reinterpret_cast<const char *>(to_conv.conv_val_ptr), string_len}));
+  RET_IF_RESULT_NEGATIVE(writer->write({(str_ptr), string_len}));
 
   // If the padding is on the right side, write the spaces last.
   if (padding_spaces > 0 &&

diff  --git a/libc/src/stdio/printf_core/write_int_converter.h b/libc/src/stdio/printf_core/write_int_converter.h
index 6b26c9bf394af4..82049426f7eff7 100644
--- a/libc/src/stdio/printf_core/write_int_converter.h
+++ b/libc/src/stdio/printf_core/write_int_converter.h
@@ -22,13 +22,11 @@ namespace printf_core {
 LIBC_INLINE int convert_write_int(Writer *writer,
                                   const FormatSection &to_conv) {
 
-  // This is an additional check added by LLVM-libc. The reason it returns -3 is
-  // because printf uses negative return values for errors, and -1 and -2 are
-  // already in use by the file_writer class for file errors.
-  // TODO: Remove this. It's better to crash than to provide an incorrect
-  // result.
+#ifndef LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
+  // This is an additional check added by LLVM-libc.
   if (to_conv.conv_val_ptr == nullptr)
     return NULLPTR_WRITE_ERROR;
+#endif // LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
 
   int written = writer->get_chars_written();
 

diff  --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 8fc4997b0268b0..98bda95d0fdab2 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -96,6 +96,12 @@ TEST(LlvmLibcSPrintfTest, StringConv) {
                                  "isn't", 12, 10, "important. Ever.");
   EXPECT_EQ(written, 26);
   ASSERT_STREQ(buff, " beginning is   important.");
+
+#ifndef LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
+  written = __llvm_libc::sprintf(buff, "%s", nullptr);
+  EXPECT_EQ(written, 4);
+  ASSERT_STREQ(buff, "null");
+#endif // LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
 }
 
 TEST(LlvmLibcSPrintfTest, IntConv) {
@@ -2781,8 +2787,10 @@ TEST(LlvmLibcSPrintfTest, WriteIntConv) {
   EXPECT_EQ(test_val, 8);
   ASSERT_STREQ(buff, "87654321");
 
+#ifndef LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
   written = __llvm_libc::sprintf(buff, "abc123%n", nullptr);
   EXPECT_LT(written, 0);
+#endif // LIBC_COPT_PRINTF_NO_NULLPTR_CHECKS
 }
 #endif // LIBC_COPT_PRINTF_DISABLE_WRITE_INT