public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc/devel/omp/gcc-11] openmp: Fix up strtoul and strtoull uses in libgomp
@ 2021-10-16 15:51 Tobias Burnus
  0 siblings, 0 replies; only message in thread
From: Tobias Burnus @ 2021-10-16 15:51 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:7b6f0eea7f0e35ce738871a6539ea7f28a0db676

commit 7b6f0eea7f0e35ce738871a6539ea7f28a0db676
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Oct 15 23:32:18 2021 +0200

    openmp: Fix up strtoul and strtoull uses in libgomp
    
    Yesterday when working on numa_domains, I've noticed because of a bug
    in my patch a hang on a large NUMA machine.  I've fixed the bug, but
    also discovered that the hang was a result of making wrong assumptions
    about strtoul/strtoull.  All the uses were for portability setting
    errno = 0 before the calls and treating non-zero errno after the call
    as invalid input, but for the case where there are no valid digits at
    all strtoul may set errno to EINVAL, but doesn't have to and with
    glibc doesn't do that.  So, this patch goes through all the strtoul calls
    and next to errno != 0 checks adds also endptr == startptr check.
    Haven't done it in places where we immediately reject strtoul returning 0
    the same as we reject errno != 0, because strtoul must return 0 in the
    case where it sets endptr to the start pointer.  In some spots the code
    was using errno = 0; x = strtoul (p, &p, 10); if (errno) { /*invalid*/ }
    and those spots had to be changed to
    errno = 0; x = strtoul (p, &end, 10); if (errno || end == p) { /*invalid*/ }
    p = end;
    
    2021-10-15  Jakub Jelinek  <jakub@redhat.com>
    
            * env.c (parse_schedule): For strtoul or strtoull calls which don't
            clearly reject return value 0 as invalid handle the case where end
            pointer is the same as first argument as invalid.
            (parse_unsigned_long_1): Likewise.
            (parse_one_place): Likewise.
            (parse_places_var): Likewise.
            (parse_stacksize): Likewise.
            (parse_spincount): Likewise.
            (parse_affinity): Likewise.
            (parse_gomp_openacc_dim): Likewise.  Avoid strict aliasing violation.
            Make code valid C89.
            * config/linux/affinity.c (gomp_affinity_find_last_cache_level):
            For strtoul calls which don't clearly reject return value 0 as
            invalid handle the case where end pointer is the same as first
            argument as invalid.
            (gomp_affinity_init_level_1): Likewise.
            (gomp_affinity_init_numa_domains): Likewise.
            * config/rtems/proc.c (parse_thread_pools): Likewise.
    
    (cherry picked from commit c057ed9c52c6a63a1a692268f916b1a9131cd4b7)

Diff:
---
 libgomp/ChangeLog.omp           | 24 +++++++++++++++++++++
 libgomp/config/linux/affinity.c | 35 ++++++++++++++++++-------------
 libgomp/config/rtems/proc.c     | 11 ++++++----
 libgomp/env.c                   | 46 ++++++++++++++++++++++++-----------------
 4 files changed, 79 insertions(+), 37 deletions(-)

diff --git a/libgomp/ChangeLog.omp b/libgomp/ChangeLog.omp
index f7303f491cf..937eeb3e2cf 100644
--- a/libgomp/ChangeLog.omp
+++ b/libgomp/ChangeLog.omp
@@ -1,3 +1,27 @@
+2021-10-15  Tobias Burnus  <tobias@codesourcery.com>
+
+	Backported from master:
+	2021-10-15  Jakub Jelinek  <jakub@redhat.com>
+
+	* env.c (parse_schedule): For strtoul or strtoull calls which don't
+	clearly reject return value 0 as invalid handle the case where end
+	pointer is the same as first argument as invalid.
+	(parse_unsigned_long_1): Likewise.
+	(parse_one_place): Likewise.
+	(parse_places_var): Likewise.
+	(parse_stacksize): Likewise.
+	(parse_spincount): Likewise.
+	(parse_affinity): Likewise.
+	(parse_gomp_openacc_dim): Likewise.  Avoid strict aliasing violation.
+	Make code valid C89.
+	* config/linux/affinity.c (gomp_affinity_find_last_cache_level):
+	For strtoul calls which don't clearly reject return value 0 as
+	invalid handle the case where end pointer is the same as first
+	argument as invalid.
+	(gomp_affinity_init_level_1): Likewise.
+	(gomp_affinity_init_numa_domains): Likewise.
+	* config/rtems/proc.c (parse_thread_pools): Likewise.
+
 2021-10-15  Tobias Burnus  <tobias@codesourcery.com>
 
 	Backported from master:
diff --git a/libgomp/config/linux/affinity.c b/libgomp/config/linux/affinity.c
index 69e72a8d141..e11906abec4 100644
--- a/libgomp/config/linux/affinity.c
+++ b/libgomp/config/linux/affinity.c
@@ -251,7 +251,7 @@ gomp_affinity_find_last_cache_level (char *name, size_t prefix_len,
 	  char *p;
 	  errno = 0;
 	  val = strtoul (line, &p, 10);
-	  if (!errno && val >= maxval)
+	  if (!errno && p > line && val >= maxval)
 	    {
 	      ret = l;
 	      maxval = val;
@@ -303,7 +303,7 @@ gomp_affinity_init_level_1 (int level, int this_level, unsigned long count,
 	  }
 	if (getline (&line, &linelen, f) > 0)
 	  {
-	    char *p = line;
+	    char *p = line, *end;
 	    void *pl = gomp_places_list[gomp_places_list_len];
 	    if (level == this_level)
 	      gomp_affinity_init_place (pl);
@@ -311,16 +311,18 @@ gomp_affinity_init_level_1 (int level, int this_level, unsigned long count,
 	      {
 		unsigned long first, last;
 		errno = 0;
-		first = strtoul (p, &p, 10);
-		if (errno)
+		first = strtoul (p, &end, 10);
+		if (errno || end == p)
 		  break;
+		p = end;
 		last = first;
 		if (*p == '-')
 		  {
 		    errno = 0;
-		    last = strtoul (p + 1, &p, 10);
-		    if (errno || last < first)
+		    last = strtoul (p + 1, &end, 10);
+		    if (errno || end == p + 1 || last < first)
 		      break;
+		    p = end;
 		  }
 		for (; first <= last; first++)
 		  if (!CPU_ISSET_S (first, gomp_cpuset_size, copy))
@@ -383,18 +385,21 @@ gomp_affinity_init_numa_domains (unsigned long count, cpu_set_t *copy,
   while (*q && *q != '\n' && gomp_places_list_len < count)
     {
       unsigned long nfirst, nlast;
+      char *end;
 
       errno = 0;
-      nfirst = strtoul (q, &q, 10);
-      if (errno)
+      nfirst = strtoul (q, &end, 10);
+      if (errno || end == q)
 	break;
+      q = end;
       nlast = nfirst;
       if (*q == '-')
 	{
 	  errno = 0;
-	  nlast = strtoul (q + 1, &q, 10);
-	  if (errno || nlast < nfirst)
+	  nlast = strtoul (q + 1, &end, 10);
+	  if (errno || end == q + 1 || nlast < nfirst)
 	    break;
+	  q = end;
 	}
       for (; nfirst <= nlast; nfirst++)
 	{
@@ -413,16 +418,18 @@ gomp_affinity_init_numa_domains (unsigned long count, cpu_set_t *copy,
 		  bool seen = false;
 
 		  errno = 0;
-		  first = strtoul (p, &p, 10);
-		  if (errno)
+		  first = strtoul (p, &end, 10);
+		  if (errno || end == p)
 		    break;
+		  p = end;
 		  last = first;
 		  if (*p == '-')
 		    {
 		      errno = 0;
-		      last = strtoul (p + 1, &p, 10);
-		      if (errno || last < first)
+		      last = strtoul (p + 1, &end, 10);
+		      if (errno || end == p + 1 || last < first)
 			break;
+		      p = end;
 		    }
 		  for (; first <= last; first++)
 		    {
diff --git a/libgomp/config/rtems/proc.c b/libgomp/config/rtems/proc.c
index 97b9e2faa87..ad40de4c381 100644
--- a/libgomp/config/rtems/proc.c
+++ b/libgomp/config/rtems/proc.c
@@ -78,22 +78,25 @@ parse_thread_pools (char *env, unsigned long *count, unsigned long *priority,
 {
   size_t len;
   int i;
+  char *end;
 
   if (*env == ':')
     ++env;
 
   errno = 0;
-  *count = strtoul (env, &env, 10);
-  if (errno != 0)
+  *count = strtoul (env, &end, 10);
+  if (errno != 0 || end == env)
     gomp_fatal ("Invalid thread pool count");
+  env = end;
 
   if (*env == '$')
     {
       ++env;
       errno = 0;
-      *priority = strtoul (env, &env, 10);
-      if (errno != 0)
+      *priority = strtoul (env, &end, 10);
+      if (errno != 0 || end == env)
 	gomp_fatal ("Invalid thread pool priority");
+      env = end;
     }
   else
     *priority = -1;
diff --git a/libgomp/env.c b/libgomp/env.c
index 7eb895f6b69..b0acacb5783 100644
--- a/libgomp/env.c
+++ b/libgomp/env.c
@@ -183,7 +183,7 @@ parse_schedule (void)
 
   errno = 0;
   value = strtoul (env, &end, 10);
-  if (errno)
+  if (errno || end == env)
     goto invalid;
 
   while (isspace ((unsigned char) *end))
@@ -232,7 +232,7 @@ parse_unsigned_long_1 (const char *name, unsigned long *pvalue, bool allow_zero,
 
   errno = 0;
   value = strtoul (env, &end, 10);
-  if (errno || (long) value <= 0 - allow_zero)
+  if (errno || end == env || (long) value <= 0 - allow_zero)
     goto invalid;
 
   while (isspace ((unsigned char) *end))
@@ -570,6 +570,7 @@ parse_one_place (char **envp, bool *negatep, unsigned long *lenp,
 	  unsigned long this_num, this_len = 1;
 	  long this_stride = 1;
 	  bool this_negate = (*env == '!');
+	  char *end;
 	  if (this_negate)
 	    {
 	      if (gomp_places_list)
@@ -580,9 +581,10 @@ parse_one_place (char **envp, bool *negatep, unsigned long *lenp,
 	    }
 
 	  errno = 0;
-	  this_num = strtoul (env, &env, 10);
-	  if (errno)
+	  this_num = strtoul (env, &end, 10);
+	  if (errno || end == env)
 	    return false;
+	  env = end;
 	  while (isspace ((unsigned char) *env))
 	    ++env;
 	  if (*env == ':')
@@ -602,9 +604,10 @@ parse_one_place (char **envp, bool *negatep, unsigned long *lenp,
 		  while (isspace ((unsigned char) *env))
 		    ++env;
 		  errno = 0;
-		  this_stride = strtol (env, &env, 10);
-		  if (errno)
+		  this_stride = strtol (env, &end, 10);
+		  if (errno || end == env)
 		    return false;
+		  env = end;
 		  while (isspace ((unsigned char) *env))
 		    ++env;
 		}
@@ -636,6 +639,7 @@ parse_one_place (char **envp, bool *negatep, unsigned long *lenp,
     ++env;
   if (*env == ':')
     {
+      char *end;
       ++env;
       while (isspace ((unsigned char) *env))
 	++env;
@@ -651,9 +655,10 @@ parse_one_place (char **envp, bool *negatep, unsigned long *lenp,
 	  while (isspace ((unsigned char) *env))
 	    ++env;
 	  errno = 0;
-	  stride = strtol (env, &env, 10);
-	  if (errno)
+	  stride = strtol (env, &end, 10);
+	  if (errno || end == env)
 	    return false;
+	  env = end;
 	  while (isspace ((unsigned char) *env))
 	    ++env;
 	}
@@ -720,7 +725,7 @@ parse_places_var (const char *name, bool ignore)
 
 	  errno = 0;
 	  count = strtoul (env, &end, 10);
-	  if (errno)
+	  if (errno || end == env)
 	    goto invalid;
 	  env = end;
 	  while (isspace ((unsigned char) *env))
@@ -859,7 +864,7 @@ parse_stacksize (const char *name, unsigned long *pvalue)
 
   errno = 0;
   value = strtoul (env, &end, 10);
-  if (errno)
+  if (errno || end == env)
     goto invalid;
 
   while (isspace ((unsigned char) *end))
@@ -928,7 +933,7 @@ parse_spincount (const char *name, unsigned long long *pvalue)
 
   errno = 0;
   value = strtoull (env, &end, 10);
-  if (errno)
+  if (errno || end == env)
     goto invalid;
 
   while (isspace ((unsigned char) *end))
@@ -1080,7 +1085,7 @@ parse_affinity (bool ignore)
 
 	  errno = 0;
 	  cpu_beg = strtoul (env, &end, 0);
-	  if (errno || cpu_beg >= 65536)
+	  if (errno || end == env || cpu_beg >= 65536)
 	    goto invalid;
 	  cpu_end = cpu_beg;
 	  cpu_stride = 1;
@@ -1090,7 +1095,7 @@ parse_affinity (bool ignore)
 	    {
 	      errno = 0;
 	      cpu_end = strtoul (++env, &end, 0);
-	      if (errno || cpu_end >= 65536 || cpu_end < cpu_beg)
+	      if (errno || end == env || cpu_end >= 65536 || cpu_end < cpu_beg)
 		goto invalid;
 
 	      env = end;
@@ -1202,27 +1207,30 @@ parse_gomp_openacc_dim (void)
   /* The syntax is the same as for the -fopenacc-dim compilation option.  */
   const char *var_name = "GOMP_OPENACC_DIM";
   const char *env_var = getenv (var_name);
+  const char *pos = env_var;
+  int i;
+
   if (!env_var)
     return;
 
-  const char *pos = env_var;
-  int i;
   for (i = 0; *pos && i != GOMP_DIM_MAX; i++)
     {
+      char *eptr;
+      long val;
+
       if (i && *pos++ != ':')
 	break;
 
       if (*pos == ':')
 	continue;
 
-      const char *eptr;
       errno = 0;
-      long val = strtol (pos, (char **)&eptr, 10);
-      if (errno || val < 0 || (unsigned)val != val)
+      val = strtol (pos, &eptr, 10);
+      if (errno || eptr != pos || val < 0 || (unsigned)val != val)
 	break;
 
       goacc_default_dims[i] = (int)val;
-      pos = eptr;
+      pos = (const char *) eptr;
     }
 }


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-10-16 15:51 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-16 15:51 [gcc/devel/omp/gcc-11] openmp: Fix up strtoul and strtoull uses in libgomp Tobias Burnus

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).