[libc-commits] [libc] 26f1770 - [libc] clean up ctype negative handling

Michael Jones via libc-commits libc-commits at lists.llvm.org
Mon Apr 10 10:00:27 PDT 2023


Author: Michael Jones
Date: 2023-04-10T10:00:22-07:00
New Revision: 26f1770e538ece427d5069b897f9e686746797a8

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

LOG: [libc] clean up ctype negative handling

The ctype functions will sometimes be passed negative numbers, such as
EOF. Some of the previous implementations didn't handle these properly.
Additionally, the tests did not check any negative numbers. These
problems have been fixed.

This patch fixes https://github.com/llvm/llvm-project/issues/62000

Reviewed By: lntue

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

Added: 
    

Modified: 
    libc/src/ctype/isblank.cpp
    libc/src/ctype/iscntrl.cpp
    libc/test/src/ctype/CMakeLists.txt
    libc/test/src/ctype/isalnum_test.cpp
    libc/test/src/ctype/isalpha_test.cpp
    libc/test/src/ctype/isascii_test.cpp
    libc/test/src/ctype/isblank_test.cpp
    libc/test/src/ctype/iscntrl_test.cpp
    libc/test/src/ctype/isdigit_test.cpp
    libc/test/src/ctype/isgraph_test.cpp
    libc/test/src/ctype/islower_test.cpp
    libc/test/src/ctype/isprint_test.cpp
    libc/test/src/ctype/ispunct_test.cpp
    libc/test/src/ctype/isspace_test.cpp
    libc/test/src/ctype/isupper_test.cpp
    libc/test/src/ctype/isxdigit_test.cpp
    libc/test/src/ctype/toascii_test.cpp
    libc/test/src/ctype/tolower_test.cpp
    libc/test/src/ctype/toupper_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/ctype/isblank.cpp b/libc/src/ctype/isblank.cpp
index eed838c8ac8db..438eff7f0a62d 100644
--- a/libc/src/ctype/isblank.cpp
+++ b/libc/src/ctype/isblank.cpp
@@ -15,8 +15,7 @@ namespace __llvm_libc {
 // TODO: Currently restricted to default locale.
 // These should be extended using locale information.
 LLVM_LIBC_FUNCTION(int, isblank, (int c)) {
-  const unsigned char ch = static_cast<unsigned char>(c);
-  return static_cast<int>(ch == ' ' || ch == '\t');
+  return static_cast<int>(c == ' ' || c == '\t');
 }
 
 } // namespace __llvm_libc

diff  --git a/libc/src/ctype/iscntrl.cpp b/libc/src/ctype/iscntrl.cpp
index 6c471a58102cd..b203e4e35f4e2 100644
--- a/libc/src/ctype/iscntrl.cpp
+++ b/libc/src/ctype/iscntrl.cpp
@@ -15,7 +15,7 @@ namespace __llvm_libc {
 // TODO: Currently restricted to default locale.
 // These should be extended using locale information.
 LLVM_LIBC_FUNCTION(int, iscntrl, (int c)) {
-  const unsigned char ch = static_cast<unsigned char>(c);
+  const unsigned ch = static_cast<unsigned>(c);
   return static_cast<int>(ch < 0x20 || ch == 0x7f);
 }
 

diff  --git a/libc/test/src/ctype/CMakeLists.txt b/libc/test/src/ctype/CMakeLists.txt
index 83ae6be202061..eadec223c8ea3 100644
--- a/libc/test/src/ctype/CMakeLists.txt
+++ b/libc/test/src/ctype/CMakeLists.txt
@@ -1,7 +1,7 @@
 add_libc_testsuite(libc_ctype_unittests)
 
 add_libc_unittest(
-  isalnum
+  isalnum_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -11,7 +11,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isalpha
+  isalpha_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -21,7 +21,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isascii
+  isascii_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -31,7 +31,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isblank
+  isblank_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -41,7 +41,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  iscntrl
+  iscntrl_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -51,7 +51,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isdigit
+  isdigit_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -61,7 +61,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isgraph
+  isgraph_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -71,7 +71,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  islower
+  islower_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -81,7 +81,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isprint
+  isprint_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -91,7 +91,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  ispunct
+  ispunct_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -101,7 +101,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isspace
+  isspace_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -111,7 +111,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isupper
+  isupper_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -121,7 +121,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  isxdigit
+  isxdigit_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -131,7 +131,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  toascii
+  toascii_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -141,7 +141,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  tolower
+  tolower_test
   SUITE
     libc_ctype_unittests
   SRCS
@@ -151,7 +151,7 @@ add_libc_unittest(
 )
 
 add_libc_unittest(
-  toupper
+  toupper_test
   SUITE
     libc_ctype_unittests
   SRCS

diff  --git a/libc/test/src/ctype/isalnum_test.cpp b/libc/test/src/ctype/isalnum_test.cpp
index 1cf5e6fbb2ff2..d44263a6ea7d1 100644
--- a/libc/test/src/ctype/isalnum_test.cpp
+++ b/libc/test/src/ctype/isalnum_test.cpp
@@ -13,7 +13,7 @@
 TEST(LlvmLibcIsAlNum, DefaultLocale) {
   // Loops through all characters, verifying that numbers and letters
   // return non-zero integer and everything else returns a zero.
-  for (int c = 0; c < 255; ++c) {
+  for (int c = -255; c < 255; ++c) {
     if (('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') ||
         ('0' <= c && c <= '9'))
       EXPECT_NE(__llvm_libc::isalnum(c), 0);

diff  --git a/libc/test/src/ctype/isalpha_test.cpp b/libc/test/src/ctype/isalpha_test.cpp
index 3fccaf650cdf9..916aae3ecd05e 100644
--- a/libc/test/src/ctype/isalpha_test.cpp
+++ b/libc/test/src/ctype/isalpha_test.cpp
@@ -13,7 +13,7 @@
 TEST(LlvmLibcIsAlpha, DefaultLocale) {
   // Loops through all characters, verifying that letters return a
   // non-zero integer and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if (('a' <= ch && ch <= 'z') || ('A' <= ch && ch <= 'Z'))
       EXPECT_NE(__llvm_libc::isalpha(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isascii_test.cpp b/libc/test/src/ctype/isascii_test.cpp
index ba450501ba0bc..80b0b1c162675 100644
--- a/libc/test/src/ctype/isascii_test.cpp
+++ b/libc/test/src/ctype/isascii_test.cpp
@@ -14,8 +14,8 @@ TEST(LlvmLibcIsAscii, DefaultLocale) {
   // Loops through all characters, verifying that ascii characters
   //    (which are all 7 bit unsigned integers)
   // return a non-zero integer and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
-    if (ch <= 0x7f)
+  for (int ch = -255; ch < 255; ++ch) {
+    if (0 <= ch && ch <= 0x7f)
       EXPECT_NE(__llvm_libc::isascii(ch), 0);
     else
       EXPECT_EQ(__llvm_libc::isascii(ch), 0);

diff  --git a/libc/test/src/ctype/isblank_test.cpp b/libc/test/src/ctype/isblank_test.cpp
index 85ea0384a19eb..f34e0cb37be97 100644
--- a/libc/test/src/ctype/isblank_test.cpp
+++ b/libc/test/src/ctype/isblank_test.cpp
@@ -12,7 +12,7 @@
 TEST(LlvmLibcIsBlank, DefaultLocale) {
   // Loops through all characters, verifying that space and horizontal tab
   // return a non-zero integer and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if (ch == ' ' || ch == '\t')
       EXPECT_NE(__llvm_libc::isblank(ch), 0);
     else

diff  --git a/libc/test/src/ctype/iscntrl_test.cpp b/libc/test/src/ctype/iscntrl_test.cpp
index 9398ad003f7ad..89b0c9a149079 100644
--- a/libc/test/src/ctype/iscntrl_test.cpp
+++ b/libc/test/src/ctype/iscntrl_test.cpp
@@ -12,7 +12,7 @@
 TEST(LlvmLibcIsCntrl, DefaultLocale) {
   // Loops through all characters, verifying that control characters
   // return a non-zero integer, all others return zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if ((0 <= ch && ch <= 0x1f /*US*/) || ch == 0x7f /*DEL*/)
       EXPECT_NE(__llvm_libc::iscntrl(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isdigit_test.cpp b/libc/test/src/ctype/isdigit_test.cpp
index ae734189eeb8c..fd49475f5bc0c 100644
--- a/libc/test/src/ctype/isdigit_test.cpp
+++ b/libc/test/src/ctype/isdigit_test.cpp
@@ -13,7 +13,7 @@
 TEST(LlvmLibcIsDigit, DefaultLocale) {
   // Loops through all characters, verifying that numbers return a
   // non-zero integer and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if ('0' <= ch && ch <= '9')
       EXPECT_NE(__llvm_libc::isdigit(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isgraph_test.cpp b/libc/test/src/ctype/isgraph_test.cpp
index 51e376c5d5dd7..7c3ed36d9ec9e 100644
--- a/libc/test/src/ctype/isgraph_test.cpp
+++ b/libc/test/src/ctype/isgraph_test.cpp
@@ -12,7 +12,7 @@
 TEST(LlvmLibcIsGraph, DefaultLocale) {
   // Loops through all characters, verifying that graphical characters
   // return a non-zero integer, everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if ('!' <= ch && ch <= '~') // A-Z, a-z, 0-9, punctuation.
       EXPECT_NE(__llvm_libc::isgraph(ch), 0);
     else

diff  --git a/libc/test/src/ctype/islower_test.cpp b/libc/test/src/ctype/islower_test.cpp
index 81d213e90d168..c399cc4e6d59a 100644
--- a/libc/test/src/ctype/islower_test.cpp
+++ b/libc/test/src/ctype/islower_test.cpp
@@ -12,7 +12,7 @@
 TEST(LlvmLibcIsLower, DefaultLocale) {
   // Loops through all characters, verifying that lowercase letters
   // return a non-zero integer and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if ('a' <= ch && ch <= 'z')
       EXPECT_NE(__llvm_libc::islower(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isprint_test.cpp b/libc/test/src/ctype/isprint_test.cpp
index 4fed40b1442d9..d7278fadc10fd 100644
--- a/libc/test/src/ctype/isprint_test.cpp
+++ b/libc/test/src/ctype/isprint_test.cpp
@@ -10,7 +10,7 @@
 #include "test/UnitTest/Test.h"
 
 TEST(LlvmLibcIsPrint, DefaultLocale) {
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if (' ' <= ch && ch <= '~') // A-Z, a-z, 0-9, punctuation, space.
       EXPECT_NE(__llvm_libc::isprint(ch), 0);
     else

diff  --git a/libc/test/src/ctype/ispunct_test.cpp b/libc/test/src/ctype/ispunct_test.cpp
index dddba4702ae2d..3b82e1958d768 100644
--- a/libc/test/src/ctype/ispunct_test.cpp
+++ b/libc/test/src/ctype/ispunct_test.cpp
@@ -25,7 +25,7 @@ static inline int is_punctuation_character(int c) {
 TEST(LlvmLibcIsPunct, DefaultLocale) {
   // Loops through all characters, verifying that punctuation characters
   // return a non-zero integer, and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if (is_punctuation_character(ch))
       EXPECT_NE(__llvm_libc::ispunct(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isspace_test.cpp b/libc/test/src/ctype/isspace_test.cpp
index 9bfac1c564747..9777bb24fc1a7 100644
--- a/libc/test/src/ctype/isspace_test.cpp
+++ b/libc/test/src/ctype/isspace_test.cpp
@@ -19,7 +19,7 @@ TEST(LlvmLibcIsSpace, DefaultLocale) {
   //    0x0b     |   vertical tab
   //    0x0d     |   carriage return
   //    0x20     |   space
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if (ch == 0x20 || (0x09 <= ch && ch <= 0x0d))
       EXPECT_NE(__llvm_libc::isspace(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isupper_test.cpp b/libc/test/src/ctype/isupper_test.cpp
index 0d0ff1e4746da..5407c763528e7 100644
--- a/libc/test/src/ctype/isupper_test.cpp
+++ b/libc/test/src/ctype/isupper_test.cpp
@@ -12,7 +12,7 @@
 TEST(LlvmLibcIsUpper, DefaultLocale) {
   // Loops through all characters, verifying that uppercase letters
   // return a non-zero integer and everything else returns zero.
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if ('A' <= ch && ch <= 'Z')
       EXPECT_NE(__llvm_libc::isupper(ch), 0);
     else

diff  --git a/libc/test/src/ctype/isxdigit_test.cpp b/libc/test/src/ctype/isxdigit_test.cpp
index 64a0e05a9ab63..b98d3bde6d064 100644
--- a/libc/test/src/ctype/isxdigit_test.cpp
+++ b/libc/test/src/ctype/isxdigit_test.cpp
@@ -10,7 +10,7 @@
 #include "test/UnitTest/Test.h"
 
 TEST(LlvmLibcIsXDigit, DefaultLocale) {
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     if (('0' <= ch && ch <= '9') || ('a' <= ch && ch <= 'f') ||
         ('A' <= ch && ch <= 'F'))
       EXPECT_NE(__llvm_libc::isxdigit(ch), 0);

diff  --git a/libc/test/src/ctype/toascii_test.cpp b/libc/test/src/ctype/toascii_test.cpp
index 88f862a6836ee..d5dde8c85d617 100644
--- a/libc/test/src/ctype/toascii_test.cpp
+++ b/libc/test/src/ctype/toascii_test.cpp
@@ -15,8 +15,8 @@ TEST(LlvmLibcToAscii, DefaultLocale) {
   //    (which are all 7 bit unsigned integers)
   // return themself, and that all other characters return themself
   // mod 128 (which is equivalent to & 0x7f)
-  for (int ch = 0; ch < 255; ++ch) {
-    if (ch <= 0x7f)
+  for (int ch = -255; ch < 255; ++ch) {
+    if (0 <= ch && ch <= 0x7f)
       EXPECT_EQ(__llvm_libc::toascii(ch), ch);
     else
       EXPECT_EQ(__llvm_libc::toascii(ch), ch & 0x7f);

diff  --git a/libc/test/src/ctype/tolower_test.cpp b/libc/test/src/ctype/tolower_test.cpp
index dc184c85ee982..2c2b7d966202a 100644
--- a/libc/test/src/ctype/tolower_test.cpp
+++ b/libc/test/src/ctype/tolower_test.cpp
@@ -10,7 +10,7 @@
 #include "test/UnitTest/Test.h"
 
 TEST(LlvmLibcToLower, DefaultLocale) {
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     // This follows pattern 'A' + 32 = 'a'.
     if ('A' <= ch && ch <= 'Z')
       EXPECT_EQ(__llvm_libc::tolower(ch), ch + 32);

diff  --git a/libc/test/src/ctype/toupper_test.cpp b/libc/test/src/ctype/toupper_test.cpp
index 402c742a99ad0..d0879d48e77fd 100644
--- a/libc/test/src/ctype/toupper_test.cpp
+++ b/libc/test/src/ctype/toupper_test.cpp
@@ -10,7 +10,7 @@
 #include "test/UnitTest/Test.h"
 
 TEST(LlvmLibcToUpper, DefaultLocale) {
-  for (int ch = 0; ch < 255; ++ch) {
+  for (int ch = -255; ch < 255; ++ch) {
     // This follows pattern 'a' - 32 = 'A'.
     if ('a' <= ch && ch <= 'z')
       EXPECT_EQ(__llvm_libc::toupper(ch), ch - 32);


        


More information about the libc-commits mailing list