From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS22989 209.51.188.0/24 X-Spam-Status: No, score=-3.7 required=3.0 tests=AWL,BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 18E681F463 for ; Sun, 5 Jan 2020 08:16:09 +0000 (UTC) Received: from localhost ([::1]:40020 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1io14x-0001W2-JA for normalperson@yhbt.net; Sun, 05 Jan 2020 03:16:07 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:57298) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1io14s-0001Uc-74 for bug-gnulib@gnu.org; Sun, 05 Jan 2020 03:16:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1io14p-0007rU-VP for bug-gnulib@gnu.org; Sun, 05 Jan 2020 03:16:01 -0500 Received: from mo6-p00-ob.smtp.rzone.de ([2a01:238:20a:202:5300::3]:28721) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1io14p-0007kj-3n for bug-gnulib@gnu.org; Sun, 05 Jan 2020 03:15:59 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1578212155; s=strato-dkim-0002; d=clisp.org; h=References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: X-RZG-CLASS-ID:X-RZG-AUTH:From:Subject:Sender; bh=efWgyi3H3XnGBvr/qc44ydmZAdleEbMzD1f65F2UFZI=; b=bQY5p6GfYpzW3/wuxkiyKRdezXL5GU0YDxCi/DY6722zHqjn3H90P5HtjuoGVBsJo9 ZT0LBmLzAGWiOE7KvoOHx7VHItg8MT/5TwD+dnTG0RZWtTmlbUzQyAAGcwAYGYnzHaQp 0UYMSWwtpiAATQj2yT34jvQXuJNRslzyGzJTL/dT7cdoBXhBHMfIwd+swevF2TjEY44F 2HWKehMrEj2lt4W/zoxoa3ArQah/Koio5ktwga5D/DA6vnb4yKl79/zVuSkNivvVK+gj JjCQFKH4k+YOA7KfZ8SvnwMnHi5R5lvVTjgvMs6EEq6yesHJgwjK4YXb9dKd4rWxWWYZ +j+g== X-RZG-AUTH: ":Ln4Re0+Ic/6oZXR1YgKryK8brlshOcZlIWs+iCP5vnk6shH+AHjwLuWOH6fzxfs=" X-RZG-CLASS-ID: mo00 Received: from bruno.haible.de by smtp.strato.de (RZmta 46.1.3 DYNA|AUTH) with ESMTPSA id R06a06w058FpRmG (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (curve X9_62_prime256v1 with 256 ECDH bits, eq. 3072 bits RSA)) (Client did not present a certificate); Sun, 5 Jan 2020 09:15:51 +0100 (CET) From: Bruno Haible To: bug-gnulib@gnu.org Subject: Re: [PATCH] stdlib: avoid canonicalize_file_name contradiction Date: Sun, 05 Jan 2020 09:15:50 +0100 Message-ID: <4223468.KeoCiBS5fI@omega> User-Agent: KMail/5.1.3 (Linux/4.4.0-170-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="nextPart2394880.hl3tbnxEgN" Content-Transfer-Encoding: 7Bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a01:238:20a:202:5300::3 X-BeenThere: bug-gnulib@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Gnulib discussion list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jim Meyering Errors-To: bug-gnulib-bounces+normalperson=yhbt.net@gnu.org Sender: "bug-gnulib" This is a multi-part message in MIME format. --nextPart2394880.hl3tbnxEgN Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Hi Jim, > stdlib: avoid canonicalize_file_name contradiction > * lib/stdlib.in.h (canonicalize_file_name): Remove the nonnull > attribute from its declaration. I'm not sure this is right. The patch removes a diagnostic (from GCC and possibly other static analyzers) when some code is, by mistake, passing NULL to canonicalize_file_name. The question is: Is passing NULL to canonicalize_file_name valid? If not, then the nonnull attribute should stay. On one hand, in glibc's stdlib.h we have: extern char *canonicalize_file_name (const char *__name) __THROW __nonnull ((1)) __wur; On the other hand, in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93156#c3 you say "Note that at least some versions of canonicalize_file_name *can* take a NULL argument". What are there versions? Even if Cygwin and/or Solaris 11 have a function of the same name which allows passing NULL, portable code should not pass NULL since that would not work on glibc systems. Therefore the diagnostic is useful. > tests/test-canonicalize-lgpl.c > passes null_ptr () to it, which (via this contradiction) would > provoke a segfault from GCC 10. See a small reproducer and > discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93156 I would prefer to modify the tests, to avoid GCC's over-optimization. There are two ways to do that: * Use 'volatile', as shown by Andrew Pinski in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93156#c2 I am not in favour of this, because the semantics of 'volatile' changes over time. Currently there is a proposed change in http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2244.htm#dr_476 . * The usual technique for this, that we already use in lib/canonicalize-lgpl.c lib/getaddrinfo.c lib/getdelim.c lib/glob.c lib/random_r.c lib/setenv.c lib/tsearch.c lib/unsetenv.c , is to add a line #define _GL_ARG_NONNULL(params) before '#include '. Does the attached patch fix the problem for you? Bruno --nextPart2394880.hl3tbnxEgN Content-Disposition: attachment; filename="0001-tests-Avoid-GCC-over-optimization-caused-by-_GL_ARG_.patch" Content-Transfer-Encoding: 7Bit Content-Type: text/x-patch; charset="UTF-8"; name="0001-tests-Avoid-GCC-over-optimization-caused-by-_GL_ARG_.patch" >From 826a16e3da3d42454417e433774dda6e5003ad04 Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Sun, 5 Jan 2020 09:13:25 +0100 Subject: [PATCH] tests: Avoid GCC over-optimization caused by _GL_ARG_NONNULL attributes. Reported by Jim Meyering in . * tests/test-canonicalize.c (_GL_ARG_NONNULL): Define to empty. * tests/test-canonicalize-lgpl.c (_GL_ARG_NONNULL): Likewise. * tests/test-ptsname_r.c (_GL_ARG_NONNULL): Likewise. --- ChangeLog | 9 +++++++++ tests/test-canonicalize-lgpl.c | 5 +++++ tests/test-canonicalize.c | 5 +++++ tests/test-ptsname_r.c | 5 +++++ 4 files changed, 24 insertions(+) diff --git a/ChangeLog b/ChangeLog index 6d9ea9e..556549c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2020-01-05 Bruno Haible + + tests: Avoid GCC over-optimization caused by _GL_ARG_NONNULL attributes. + Reported by Jim Meyering in + . + * tests/test-canonicalize.c (_GL_ARG_NONNULL): Define to empty. + * tests/test-canonicalize-lgpl.c (_GL_ARG_NONNULL): Likewise. + * tests/test-ptsname_r.c (_GL_ARG_NONNULL): Likewise. + 2020-01-04 Bruno Haible mbsnrtoc32s: Add tests. diff --git a/tests/test-canonicalize-lgpl.c b/tests/test-canonicalize-lgpl.c index b46ad87..2c743af 100644 --- a/tests/test-canonicalize-lgpl.c +++ b/tests/test-canonicalize-lgpl.c @@ -16,6 +16,11 @@ /* Written by Bruno Haible , 2007. */ +/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc + may "optimize" the null_ptr function, when its result gets passed to a + function that has an argument declared as _GL_ARG_NONNULL. */ +#define _GL_ARG_NONNULL(params) + #include #include diff --git a/tests/test-canonicalize.c b/tests/test-canonicalize.c index e7138a6..1df3092 100644 --- a/tests/test-canonicalize.c +++ b/tests/test-canonicalize.c @@ -16,6 +16,11 @@ /* Written by Bruno Haible , 2007. */ +/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc + may "optimize" the null_ptr function, when its result gets passed to a + function that has an argument declared as _GL_ARG_NONNULL. */ +#define _GL_ARG_NONNULL(params) + #include #include "canonicalize.h" diff --git a/tests/test-ptsname_r.c b/tests/test-ptsname_r.c index 19e33ea..9e1471c 100644 --- a/tests/test-ptsname_r.c +++ b/tests/test-ptsname_r.c @@ -14,6 +14,11 @@ You should have received a copy of the GNU General Public License along with this program. If not, see . */ +/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc + may "optimize" the null_ptr function, when its result gets passed to a + function that has an argument declared as _GL_ARG_NONNULL. */ +#define _GL_ARG_NONNULL(params) + #include #include -- 2.7.4 --nextPart2394880.hl3tbnxEgN--