From patchwork Fri Oct 15 14:46:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 1541763 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=uJ0aBsvX; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HW8JH0phhz9ssD for ; Sat, 16 Oct 2021 01:47:43 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EC05D385780B for ; Fri, 15 Oct 2021 14:47:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EC05D385780B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1634309261; bh=AXKW+5af5hVLxhT7ZLKGc5d+RqI+wHMXCJQDHymxxTc=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=uJ0aBsvXpb8BhiT2FCjbKnBcGcDldSBuxV841kiNcKbpJW7YSsH0xLCqtFkfjFclU ATMXByFYrh2df/5TDCDZjwXjye6ZLddHzQANW6gBOXLrLTDHbQay0ZLpnTc4N7b3wI EoEitD7G2a1uzq8KPa3W6YpSyYMl2bYoCJv/v3U0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 30027385780D for ; Fri, 15 Oct 2021 14:46:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 30027385780D Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-64-cOtENQl7P_-nd6FjjB-nbQ-1; Fri, 15 Oct 2021 10:46:38 -0400 X-MC-Unique: cOtENQl7P_-nd6FjjB-nbQ-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id D3B6E10A8E00; Fri, 15 Oct 2021 14:46:37 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.193.172]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 5871910013D7; Fri, 15 Oct 2021 14:46:37 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 19FEkY8Z424956 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Fri, 15 Oct 2021 16:46:35 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 19FEkX1L424955; Fri, 15 Oct 2021 16:46:33 +0200 Date: Fri, 15 Oct 2021 16:46:33 +0200 To: gcc-patches@gcc.gnu.org Subject: [committed] openmp: Fix up strtoul and strtoull uses in libgomp Message-ID: <20211015144633.GF304296@tucnak> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jakub Jelinek via Gcc-patches From: Jakub Jelinek Reply-To: Jakub Jelinek Cc: Tobias Burnus Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Hi! 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; Regtested on x86_64-linux and i686-linux, committed to trunk. 2021-10-15 Jakub Jelinek * 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. Jakub --- libgomp/env.c.jj 2021-10-14 22:04:30.594333475 +0200 +++ libgomp/env.c 2021-10-15 14:07:07.464919497 +0200 @@ -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, 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 *nega 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 *nega } 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 *nega 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 *nega ++env; if (*env == ':') { + char *end; ++env; while (isspace ((unsigned char) *env)) ++env; @@ -651,9 +655,10 @@ parse_one_place (char **envp, bool *nega 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 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, unsig 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, unsig 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; } } --- libgomp/config/linux/affinity.c.jj 2021-10-15 13:20:19.561484351 +0200 +++ libgomp/config/linux/affinity.c 2021-10-15 14:14:14.718752064 +0200 @@ -251,7 +251,7 @@ gomp_affinity_find_last_cache_level (cha 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, i } 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, i { 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 (unsigne 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 (unsigne 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++) { --- libgomp/config/rtems/proc.c.jj 2021-01-05 00:13:58.255297642 +0100 +++ libgomp/config/rtems/proc.c 2021-10-15 14:12:38.980134056 +0200 @@ -78,22 +78,25 @@ parse_thread_pools (char *env, unsigned { 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;