[libc] fix strtointeger hex prefix parsing

[michaelrj]

Summary

Fix edge case where "0x" would be considered a complete hexadecimal
number for purposes of str_end. Now the hexadecimal prefix needs a valid
digit after it, else just the 0 will be counted as the number.

[sivachandra]

While it is OK for this patch, I would suggest that the tests be templatized.

libc/src/__support/str_conv_utils.h

82

I think the logic here and on line 41/42 can be moved to a single function say, is_hex_start(...). Then, we can use it like this:

else if (base == 16 && is_hex_start(src)) {
  ...
}

infer_base will have to be modified to something like this:

if (is_hex_start(*src)) {
  (*src) += 2;
  return 16;
} else if (**src == '0') {
  ++(*src);
  return 8;
} else {
  return 10;
}

[michaelrj]

move the hex number identification to a function

libc/src/__support/str_conv_utils.h

 static inline int b36_char_to_int(char input) {
   if (isdigit(input))
     return input - '0';
   if (isalpha(input))
     return (input | 32) + 10 - 'a';
   return 0;
 }
 
+// checks if the next 3 characters of the string pointer are the start of a
+// hexadecimal number. Does not advance the string pointer.
+static inline bool is_hex_start(const char *__restrict src) {
+  return *src == '0' && (*(src + 1) | 32) == 'x' && isalnum(*(src + 2)) &&
+         b36_char_to_int(*(src + 2)) < 16;
+}
+
 // Takes the address of the string pointer and parses the base from the start of
 // it. This will advance the string pointer.
 static inline int infer_base(const char *__restrict *__restrict src) {
-  if (**src == '0') {
-    ++(*src);
-    if ((**src | 32) == 'x') {
-      ++(*src);
-      return 16;
-    }
+  if (is_hex_start(*src)) {
+    (*src) += 2;
+  } else if (**src == '0') {
+    ++(*src);
     return 8;
-  }
+  } else {
     return 10;
   }
+}
 
 // Takes a pointer to a string, a pointer to a string pointer, and the base to
 // convert to. This function is used as the backend for all of the string to int
 // functions.
 template <class T>
 static inline T strtointeger(const char *__restrict src,
                              char **__restrict str_end, int base) {
   unsigned long long result = 0;
 
   if (base < 0 || base == 1 || base > 36) {
     errno = EINVAL; // NOLINT
     return 0;
   }
 
   src = first_non_whitespace(src);
 
   char result_sign = '+';
   if (*src == '+' || *src == '-') {
     result_sign = *src;
     ++src;
   }
 
   if (base == 0) {
     base = infer_base(&src);
-  } else if (base == 16 && *src == '0' && (*(src + 1) | 32) == 'x') {
+  } else if (base == 16 && is_hex_start(src)) {
     src = src + 2;
   }
 
   constexpr bool is_unsigned = (__llvm_libc::cpp::NumericLimits<T>::min() == 0);
   const bool is_positive = (result_sign == '+');
   unsigned long long constexpr NEGATIVE_MAX =
       !is_unsigned ? static_cast<unsigned long long>(
                          __llvm_libc::cpp::NumericLimits<T>::max()) +
                          1

libc/test/src/stdlib/strtol_test.cpp

TEST(LlvmLibcStrToLTest, CleanBaseSixteenDecode) {
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - no_prefix, ptrdiff_t(6));
 
   const char *yes_prefix = "0x456def";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtol(yes_prefix, &str_end, 16), 0x456defl);
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - yes_prefix, ptrdiff_t(8));
+
+  const char *letter_after_prefix = "0xabc123";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtol(letter_after_prefix, &str_end, 16), 0xabc123l);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - letter_after_prefix, ptrdiff_t(8));
+}
+
+TEST(LlvmLibcStrToLTest, MessyBaseSixteenDecode) {
+  char *str_end = nullptr;
+
+  const char *just_prefix = "0x";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtol(just_prefix, &str_end, 16), 0l);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtol(just_prefix, &str_end, 0), 0l);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  const char *prefix_with_x_after = "0xx";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtol(prefix_with_x_after, &str_end, 16), 0l);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtol(prefix_with_x_after, &str_end, 0), 0l);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
 }
 
 TEST(LlvmLibcStrToLTest, AutomaticBaseSelection) {
   char *str_end = nullptr;
 
   const char *base_ten = "12345";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtol(base_ten, &str_end, 0), 12345l);

libc/test/src/stdlib/strtoll_test.cpp

TEST(LlvmLibcStrToLLTest, CleanBaseSixteenDecode) {
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - no_prefix, ptrdiff_t(6));
 
   const char *yes_prefix = "0x456def";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtoll(yes_prefix, &str_end, 16), 0x456defll);
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - yes_prefix, ptrdiff_t(8));
+
+  const char *letter_after_prefix = "0xabc123";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoll(letter_after_prefix, &str_end, 16),
+            0xabc123ll);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - letter_after_prefix, ptrdiff_t(8));
+}
+
+TEST(LlvmLibcStrToLLTest, MessyBaseSixteenDecode) {
+  char *str_end = nullptr;
+
+  const char *just_prefix = "0x";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoll(just_prefix, &str_end, 16), 0ll);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoll(just_prefix, &str_end, 0), 0ll);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  const char *prefix_with_x_after = "0xx";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoll(prefix_with_x_after, &str_end, 16), 0ll);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoll(prefix_with_x_after, &str_end, 0), 0ll);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
 }
 
 TEST(LlvmLibcStrToLLTest, AutomaticBaseSelection) {
   char *str_end = nullptr;
 
   const char *base_ten = "12345";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtoll(base_ten, &str_end, 0), 12345ll);

libc/test/src/stdlib/strtoul_test.cpp

TEST(LlvmLibcStrToULTest, CleanBaseSixteenDecode) {
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - no_prefix, ptrdiff_t(6));
 
   const char *yes_prefix = "0x456def";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtoul(yes_prefix, &str_end, 16), 0x456deful);
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - yes_prefix, ptrdiff_t(8));
+
+  const char *letter_after_prefix = "0xabc123";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoul(letter_after_prefix, &str_end, 16),
+            0xabc123ul);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - letter_after_prefix, ptrdiff_t(8));
+}
+
+TEST(LlvmLibcStrToULTest, MessyBaseSixteenDecode) {
+  char *str_end = nullptr;
+
+  const char *just_prefix = "0x";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoul(just_prefix, &str_end, 16), 0ul);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoul(just_prefix, &str_end, 0), 0ul);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  const char *prefix_with_x_after = "0xx";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoul(prefix_with_x_after, &str_end, 16), 0ul);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoul(prefix_with_x_after, &str_end, 0), 0ul);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
 }
 
 TEST(LlvmLibcStrToULTest, AutomaticBaseSelection) {
   char *str_end = nullptr;
 
   const char *base_ten = "12345";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtoul(base_ten, &str_end, 0), 12345ul);

libc/test/src/stdlib/strtoull_test.cpp

TEST(LlvmLibcStrToULLTest, CleanBaseSixteenDecode) {
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - no_prefix, ptrdiff_t(6));
 
   const char *yes_prefix = "0x456def";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtoull(yes_prefix, &str_end, 16), 0x456defull);
   ASSERT_EQ(errno, 0);
   EXPECT_EQ(str_end - yes_prefix, ptrdiff_t(8));
+
+  const char *letter_after_prefix = "0xabc123";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoull(letter_after_prefix, &str_end, 16),
+            0xabc123ull);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - letter_after_prefix, ptrdiff_t(8));
+}
+
+TEST(LlvmLibcStrToULLTest, MessyBaseSixteenDecode) {
+  char *str_end = nullptr;
+
+  const char *just_prefix = "0x";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoull(just_prefix, &str_end, 16), 0ull);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoull(just_prefix, &str_end, 0), 0ull);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - just_prefix, ptrdiff_t(1));
+
+  const char *prefix_with_x_after = "0xx";
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoull(prefix_with_x_after, &str_end, 16), 0ull);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
+
+  errno = 0;
+  ASSERT_EQ(__llvm_libc::strtoull(prefix_with_x_after, &str_end, 0), 0ull);
+  ASSERT_EQ(errno, 0);
+  EXPECT_EQ(str_end - prefix_with_x_after, ptrdiff_t(1));
 }
 
 TEST(LlvmLibcStrToULLTest, AutomaticBaseSelection) {
   char *str_end = nullptr;
 
   const char *base_ten = "12345";
   errno = 0;
   ASSERT_EQ(__llvm_libc::strtoull(base_ten, &str_end, 0), 12345ull);