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.

diff --git a/libgomp/config/linux/affinity.c b/libgomp/config/linux/affinity.c
index 69e72a8d1413e0cdacc0191440e4f95087c05caf..e11906abec4a7f561fe57b90d6169e7bce6b1b76 100644 (file)
--- 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 97b9e2faa879274aa5496e4a0b3410feb251267e..ad40de4c3811e1bb149a226e8039f5811ffc4b56 100644 (file)
--- 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 7eb895f6b6948dd2db53ebbf969e4d17233c98e7..b0acacb57838e3efaa4a897a6a710faad3334433 100644 (file)
--- 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;
     }
 }