diff mbox series

Fix special case for C2x strtol binary constant handling (BZ# 30371)

Message ID 20230525112032.4041628-1-adhemerval.zanella@linaro.org
State Committed
Commit 95c9a6e806226cbf174c92efc021a0d464f170a4
Headers
Series Fix special case for C2x strtol binary constant handling (BZ# 30371) |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm pending Patch applied
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 pending Patch applied

Commit Message

Adhemerval Zanella May 25, 2023, 11:20 a.m. UTC 
  When the base is 0 or 2 and the first two characters are '0' and 'b',
but the rest are no binary digits.  In this case this is no error,
and strtol must return 0 and ENDPTR points to the 'x' or 'b'.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 stdlib/strtol_l.c                | 10 ++--
 stdlib/tst-strtol-binary-c11.c   |  1 +
 stdlib/tst-strtol-binary-c2x.c   |  1 +
 stdlib/tst-strtol-binary-gnu11.c |  1 +
 stdlib/tst-strtol-binary-gnu2x.c |  1 +
 stdlib/tst-strtol-binary-main.c  | 86 +++++++++++++++++++-------------
 wcsmbs/tst-wcstol-binary-c11.c   |  1 +
 wcsmbs/tst-wcstol-binary-c2x.c   |  1 +
 wcsmbs/tst-wcstol-binary-gnu11.c |  1 +
 wcsmbs/tst-wcstol-binary-gnu2x.c |  1 +
 10 files changed, 65 insertions(+), 39 deletions(-)

Comments

Florian Weimer May 25, 2023, 11:33 a.m. UTC | #1 
* Adhemerval Zanella via Libc-alpha:

> diff --git a/stdlib/tst-strtol-binary-main.c b/stdlib/tst-strtol-binary-main.c
> index ece3100298..54cda5cd03 100644
> --- a/stdlib/tst-strtol-binary-main.c
> +++ b/stdlib/tst-strtol-binary-main.c

> +#define CHECK_RES(ARG, RES, EP, EXPECTED, EXPECTED_EP)			\
>    do									\
>      {									\
>        if (TEST_C2X)							\
>  	{								\
>  	  TEST_COMPARE ((RES), EXPECTED);				\
> -	  TEST_COMPARE (*(EP), 0);					\
> +	  TEST_VERIFY ((EP) == EXPECTED_EP);				\
>  	}								\
>        else								\
>  	{								\

You could add TEST_COMPARE_STRING (EP, EXPECTED_EP); as a debugging aid.
Patch looks okay.

Reviewed-by: Florian Weimer <fweimer@redhat.com>

Thanks,
Florian
Adhemerval Zanella May 25, 2023, 12:22 p.m. UTC | #2 
On 25/05/23 08:33, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/stdlib/tst-strtol-binary-main.c b/stdlib/tst-strtol-binary-main.c
>> index ece3100298..54cda5cd03 100644
>> --- a/stdlib/tst-strtol-binary-main.c
>> +++ b/stdlib/tst-strtol-binary-main.c
> 
>> +#define CHECK_RES(ARG, RES, EP, EXPECTED, EXPECTED_EP)			\
>>    do									\
>>      {									\
>>        if (TEST_C2X)							\
>>  	{								\
>>  	  TEST_COMPARE ((RES), EXPECTED);				\
>> -	  TEST_COMPARE (*(EP), 0);					\
>> +	  TEST_VERIFY ((EP) == EXPECTED_EP);				\
>>  	}								\
>>        else								\
>>  	{								\
> 
> You could add TEST_COMPARE_STRING (EP, EXPECTED_EP); as a debugging aid.
> Patch looks okay.

I though about it, but since the interface explicit state the the result
should point to the string itself I don't think it adds much.

> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
Thanks.
Bruno Haible May 25, 2023, 1:24 p.m. UTC | #3 
The patch looks good to me as well.
Thanks for handling this, Adhemerval.

Bruno
diff mbox series

Patch

diff --git a/stdlib/strtol_l.c b/stdlib/strtol_l.c
index 3424c3feab..548b46aa52 100644
--- a/stdlib/strtol_l.c
+++ b/stdlib/strtol_l.c
@@ -526,11 +526,15 @@  INTERNAL (__strtol_l) (const STRING_TYPE *nptr, STRING_TYPE **endptr,
 noconv:
   /* We must handle a special case here: the base is 0 or 16 and the
      first two characters are '0' and 'x', but the rest are no
-     hexadecimal digits.  This is no error case.  We return 0 and
-     ENDPTR points to the `x`.  */
+     hexadecimal digits.  Likewise when the base is 0 or 2 and the
+     first two characters are '0' and 'b', but the rest are no binary
+     digits.  This is no error case.  We return 0 and ENDPTR points to
+     the 'x' or 'b'.  */
   if (endptr != NULL)
     {
-      if (save - nptr >= 2 && TOUPPER (save[-1]) == L_('X')
+      if (save - nptr >= 2
+	  && (TOUPPER (save[-1]) == L_('X')
+	      || (bin_cst && TOUPPER (save[-1]) == L_('B')))
 	  && save[-2] == L_('0'))
 	*endptr = (STRING_TYPE *) &save[-1];
       else
diff --git a/stdlib/tst-strtol-binary-c11.c b/stdlib/tst-strtol-binary-c11.c
index 6e58bb2599..8b8c31cdd0 100644
--- a/stdlib/tst-strtol-binary-c11.c
+++ b/stdlib/tst-strtol-binary-c11.c
@@ -20,6 +20,7 @@ 
 #undef _GNU_SOURCE
 
 #define CHAR char
+#define WIDE 0
 #define FNPFX strto
 #define L_(C) C
 #define TEST_C2X 0
diff --git a/stdlib/tst-strtol-binary-c2x.c b/stdlib/tst-strtol-binary-c2x.c
index b9ccfda759..e75f0881b6 100644
--- a/stdlib/tst-strtol-binary-c2x.c
+++ b/stdlib/tst-strtol-binary-c2x.c
@@ -23,6 +23,7 @@ 
 #define _ISOC2X_SOURCE
 
 #define CHAR char
+#define WIDE 0
 #define FNPFX strto
 #define L_(C) C
 #define TEST_C2X 1
diff --git a/stdlib/tst-strtol-binary-gnu11.c b/stdlib/tst-strtol-binary-gnu11.c
index a029591c8b..7dc81317df 100644
--- a/stdlib/tst-strtol-binary-gnu11.c
+++ b/stdlib/tst-strtol-binary-gnu11.c
@@ -25,6 +25,7 @@ 
 #define __GLIBC_USE_C2X_STRTOL 0
 
 #define CHAR char
+#define WIDE 0
 #define FNPFX strto
 #define L_(C) C
 #define TEST_C2X 0
diff --git a/stdlib/tst-strtol-binary-gnu2x.c b/stdlib/tst-strtol-binary-gnu2x.c
index 0a7fdd4d4d..96db2414b9 100644
--- a/stdlib/tst-strtol-binary-gnu2x.c
+++ b/stdlib/tst-strtol-binary-gnu2x.c
@@ -18,6 +18,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #define CHAR char
+#define WIDE 0
 #define FNPFX strto
 #define L_(C) C
 #define TEST_C2X 1
diff --git a/stdlib/tst-strtol-binary-main.c b/stdlib/tst-strtol-binary-main.c
index ece3100298..54cda5cd03 100644
--- a/stdlib/tst-strtol-binary-main.c
+++ b/stdlib/tst-strtol-binary-main.c
@@ -21,6 +21,7 @@ 
 #include <locale.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <wchar.h>
 
 #include <support/check.h>
@@ -32,13 +33,19 @@ 
 #define CONCAT(X, Y) CONCAT_ (X, Y)
 #define FNX(FN) CONCAT (FNPFX, FN)
 
-#define CHECK_RES(ARG, RES, EP, EXPECTED)				\
+#if WIDE
+# define STRCHR wcschr
+#else
+# define STRCHR strchr
+#endif
+
+#define CHECK_RES(ARG, RES, EP, EXPECTED, EXPECTED_EP)			\
   do									\
     {									\
       if (TEST_C2X)							\
 	{								\
 	  TEST_COMPARE ((RES), EXPECTED);				\
-	  TEST_COMPARE (*(EP), 0);					\
+	  TEST_VERIFY ((EP) == EXPECTED_EP);				\
 	}								\
       else								\
 	{								\
@@ -51,95 +58,102 @@ 
   while (0)
 
 static void
-one_check (const CHAR *s, long int expected_l, unsigned long int expected_ul,
-	   long long int expected_ll, unsigned long long int expected_ull)
+one_check (const CHAR *s, const CHAR *expected_p, long int expected_l,
+	   unsigned long int expected_ul, long long int expected_ll,
+	   unsigned long long int expected_ull)
 {
+  expected_p = expected_p == NULL ? STRCHR (s, '\0') : expected_p;
+
   CHAR *ep;
   long int ret_l;
   unsigned long int ret_ul;
   long long int ret_ll;
   unsigned long long int ret_ull;
   ret_l = FNX (l) (s, &ep, 0);
-  CHECK_RES (s, ret_l, ep, expected_l);
+  CHECK_RES (s, ret_l, ep, expected_l, expected_p);
   ret_l = FNX (l) (s, &ep, 2);
-  CHECK_RES (s, ret_l, ep, expected_l);
+  CHECK_RES (s, ret_l, ep, expected_l, expected_p);
   ret_ul = FNX (ul) (s, &ep, 0);
-  CHECK_RES (s, ret_ul, ep, expected_ul);
+  CHECK_RES (s, ret_ul, ep, expected_ul, expected_p);
   ret_ul = FNX (ul) (s, &ep, 2);
-  CHECK_RES (s, ret_ul, ep, expected_ul);
+  CHECK_RES (s, ret_ul, ep, expected_ul, expected_p);
   ret_ll = FNX (ll) (s, &ep, 0);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ll = FNX (ll) (s, &ep, 2);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ull = FNX (ull) (s, &ep, 0);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
   ret_ull = FNX (ull) (s, &ep, 2);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
   ret_ll = FNX (imax) (s, &ep, 0);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ll = FNX (imax) (s, &ep, 2);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ull = FNX (umax) (s, &ep, 0);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
   ret_ull = FNX (umax) (s, &ep, 2);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
 #if TEST_Q
   ret_ll = FNX (q) (s, &ep, 0);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ll = FNX (q) (s, &ep, 2);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ull = FNX (uq) (s, &ep, 0);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
   ret_ull = FNX (uq) (s, &ep, 2);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
 #endif
 #if TEST_LOCALE
   locale_t loc = xnewlocale (LC_NUMERIC_MASK, "C", (locale_t) 0);
   ret_l = FNX (l_l) (s, &ep, 0, loc);
-  CHECK_RES (s, ret_l, ep, expected_l);
+  CHECK_RES (s, ret_l, ep, expected_l, expected_p);
   ret_l = FNX (l_l) (s, &ep, 2, loc);
-  CHECK_RES (s, ret_l, ep, expected_l);
+  CHECK_RES (s, ret_l, ep, expected_l, expected_p);
   ret_ul = FNX (ul_l) (s, &ep, 0, loc);
-  CHECK_RES (s, ret_ul, ep, expected_ul);
+  CHECK_RES (s, ret_ul, ep, expected_ul, expected_p);
   ret_ul = FNX (ul_l) (s, &ep, 2, loc);
-  CHECK_RES (s, ret_ul, ep, expected_ul);
+  CHECK_RES (s, ret_ul, ep, expected_ul, expected_p);
   ret_ll = FNX (ll_l) (s, &ep, 0, loc);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ll = FNX (ll_l) (s, &ep, 2, loc);
-  CHECK_RES (s, ret_ll, ep, expected_ll);
+  CHECK_RES (s, ret_ll, ep, expected_ll, expected_p);
   ret_ull = FNX (ull_l) (s, &ep, 0, loc);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
   ret_ull = FNX (ull_l) (s, &ep, 2, loc);
-  CHECK_RES (s, ret_ull, ep, expected_ull);
+  CHECK_RES (s, ret_ull, ep, expected_ull, expected_p);
 #endif
 }
 
 static int
 do_test (void)
 {
-  one_check (L_("0b101"), 5, 5, 5, 5);
-  one_check (L_("0B101"), 5, 5, 5, 5);
-  one_check (L_("-0b11111"), -31, -31, -31, -31);
-  one_check (L_("-0B11111"), -31, -31, -31, -31);
-  one_check (L_("0b111111111111111111111111111111111"),
+  {
+    const CHAR *input = L_("0b");
+    one_check (input, input + 1, 0L, 0UL, 0LL, 0ULL);
+  }
+  one_check (L_("0b101"), NULL, 5, 5, 5, 5);
+  one_check (L_("0B101"), NULL, 5, 5, 5, 5);
+  one_check (L_("-0b11111"), NULL, -31, -31, -31, -31);
+  one_check (L_("-0B11111"), NULL, -31, -31, -31, -31);
+  one_check (L_("0b111111111111111111111111111111111"), NULL,
 	     LONG_MAX >= 0x1ffffffffLL ? (long int) 0x1ffffffffLL : LONG_MAX,
 	     (ULONG_MAX >= 0x1ffffffffULL
 	      ? (unsigned long int) 0x1ffffffffULL
 	      : ULONG_MAX),
 	     0x1ffffffffLL, 0x1ffffffffULL);
-  one_check (L_("0B111111111111111111111111111111111"),
+  one_check (L_("0B111111111111111111111111111111111"), NULL,
 	     LONG_MAX >= 0x1ffffffffLL ? (long int) 0x1ffffffffLL : LONG_MAX,
 	     (ULONG_MAX >= 0x1ffffffffULL
 	      ? (unsigned long int) 0x1ffffffffULL
 	      : ULONG_MAX),
 	     0x1ffffffffLL, 0x1ffffffffULL);
-  one_check (L_("-0b111111111111111111111111111111111"),
+  one_check (L_("-0b111111111111111111111111111111111"), NULL,
 	     LONG_MIN <= -0x1ffffffffLL ? (long int) -0x1ffffffffLL : LONG_MIN,
 	     (ULONG_MAX >= 0x1ffffffffULL
 	      ? (unsigned long int) -0x1ffffffffULL
 	      : ULONG_MAX),
 	     -0x1ffffffffLL, -0x1ffffffffULL);
-  one_check (L_("-0B111111111111111111111111111111111"),
+  one_check (L_("-0B111111111111111111111111111111111"), NULL,
 	     LONG_MIN <= -0x1ffffffffLL ? (long int) -0x1ffffffffLL : LONG_MIN,
 	     (ULONG_MAX >= 0x1ffffffffULL
 	      ? (unsigned long int) -0x1ffffffffULL
diff --git a/wcsmbs/tst-wcstol-binary-c11.c b/wcsmbs/tst-wcstol-binary-c11.c
index bff1d879f0..fdd79ec9e0 100644
--- a/wcsmbs/tst-wcstol-binary-c11.c
+++ b/wcsmbs/tst-wcstol-binary-c11.c
@@ -20,6 +20,7 @@ 
 #undef _GNU_SOURCE
 
 #define CHAR wchar_t
+#define WIDE 1
 #define FNPFX wcsto
 #define L_(C) L ## C
 #define TEST_C2X 0
diff --git a/wcsmbs/tst-wcstol-binary-c2x.c b/wcsmbs/tst-wcstol-binary-c2x.c
index 0f8ef44854..6c06dab9ba 100644
--- a/wcsmbs/tst-wcstol-binary-c2x.c
+++ b/wcsmbs/tst-wcstol-binary-c2x.c
@@ -23,6 +23,7 @@ 
 #define _ISOC2X_SOURCE
 
 #define CHAR wchar_t
+#define WIDE 1
 #define FNPFX wcsto
 #define L_(C) L ## C
 #define TEST_C2X 1
diff --git a/wcsmbs/tst-wcstol-binary-gnu11.c b/wcsmbs/tst-wcstol-binary-gnu11.c
index 189f217563..1a3d5d3d6c 100644
--- a/wcsmbs/tst-wcstol-binary-gnu11.c
+++ b/wcsmbs/tst-wcstol-binary-gnu11.c
@@ -25,6 +25,7 @@ 
 #define __GLIBC_USE_C2X_STRTOL 0
 
 #define CHAR wchar_t
+#define WIDE 1
 #define FNPFX wcsto
 #define L_(C) L ## C
 #define TEST_C2X 0
diff --git a/wcsmbs/tst-wcstol-binary-gnu2x.c b/wcsmbs/tst-wcstol-binary-gnu2x.c
index 707d4076f1..feda1b59dd 100644
--- a/wcsmbs/tst-wcstol-binary-gnu2x.c
+++ b/wcsmbs/tst-wcstol-binary-gnu2x.c
@@ -18,6 +18,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #define CHAR wchar_t
+#define WIDE 1
 #define FNPFX wcsto
 #define L_(C) L ## C
 #define TEST_C2X 1