[libc-commits] [PATCH] D156923: [libc] Add nullptr check option to printf %s

[Michael Jones via Phabricator via libc-commits]

michaelrj created this revision.
michaelrj added reviewers: lntue, sivachandra.
Herald added projects: libc-project, All.
Herald added a subscriber: libc-commits.
michaelrj requested review of this revision.
Herald added a subscriber: wangpc.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156923

Files:
  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


Index: libc/test/src/stdio/sprintf_test.cpp
===================================================================
--- libc/test/src/stdio/sprintf_test.cpp
+++ libc/test/src/stdio/sprintf_test.cpp
@@ -96,6 +96,12 @@
                                  "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 @@
   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
 
Index: libc/src/stdio/printf_core/write_int_converter.h
===================================================================
--- libc/src/stdio/printf_core/write_int_converter.h
+++ libc/src/stdio/printf_core/write_int_converter.h
@@ -22,13 +22,11 @@
 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();
 
Index: libc/src/stdio/printf_core/string_converter.h
===================================================================
--- libc/src/stdio/printf_core/string_converter.h
+++ libc/src/stdio/printf_core/string_converter.h
@@ -22,9 +22,15 @@
 
 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 @@
     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 &&
Index: libc/src/stdio/printf_core/printf_config.h
===================================================================
--- libc/src/stdio/printf_core/printf_config.h
+++ 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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D156923.546551.patch
Type: text/x-patch
Size: 3453 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libc-commits/attachments/20230802/9df41c9f/attachment.bin>