From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 4099C3858D20 for ; Tue, 30 May 2023 07:41:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4099C3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1685432476; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FvhDSa/ICnQPwEWQj4A0S8xT3M2WC+I2i9SnCAjXM0k=; b=Zn6pI6sFTn0ZzUu7aUFCW8fexvJV7GLnxE5aScBQQHnJGOib6SrJYoS2XEvDMYRCGYmIo3 n0v01D+L+J7F8P3hbkSvJZD2uulOuzxpKcYHeQV+1W71PVPe0a+Prn+BokqU7eRafkL1MF grT1+tDJ4+6+HyyCFqJUWZj/z2Tyjuo= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-235-4PXWHUcKO36vf09FIyP-rQ-1; Tue, 30 May 2023 03:41:13 -0400 X-MC-Unique: 4PXWHUcKO36vf09FIyP-rQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 5044A800B2A; Tue, 30 May 2023 07:41:13 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.2.16.38]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B73CA112132C; Tue, 30 May 2023 07:41:12 +0000 (UTC) From: Florian Weimer To: Adhemerval Zanella Netto Cc: Sergey Bugaev , libc-alpha@sourceware.org Subject: Re: [PATCH v2 3/3] io: Add FORTIFY_SOURCE check for fcntl arguments References: <20230528172013.73111-1-bugaevc@gmail.com> <20230528172013.73111-4-bugaevc@gmail.com> <31457dbb-a805-262f-4b62-be0b40960ca6@linaro.org> <8354c659-cfb0-993a-2764-72a2cd6f6ed4@linaro.org> Date: Tue, 30 May 2023 09:41:10 +0200 In-Reply-To: <8354c659-cfb0-993a-2764-72a2cd6f6ed4@linaro.org> (Adhemerval Zanella Netto's message of "Mon, 29 May 2023 15:09:39 -0300") Message-ID: <87edmyxo4p.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: * Adhemerval Zanella Netto: > On 29/05/23 14:31, Sergey Bugaev wrote: >> Hello, >>=20 >> On Mon, May 29, 2023 at 7:54=E2=80=AFPM Adhemerval Zanella Netto >> wrote: >>> Some comments below. >>=20 >> thank you for the review :) >>=20 >>>> +/* Return VALUE, but do it in a way that the compiler cannot >>>> + see that it's a compile-time constant. */ >>>> +static int __attribute_noinline__ >>>> +hide_constant (int value) >>>> +{ >>>> + volatile int v =3D value; >>>> + return v; >>>> +} >>>> + >>> >>> Interesting construct, but I wonder if 'volatile' is really required he= re. >>> Afaiu what really prevents compiler to return fortify compiler warning = here >>> is the __attribute_noinline__. >>=20 >> It's not really required. Apparently even an 'inline' function call is >> enough to to disarm __builtin_constant_p [0]; I just threw both >> volatile and noinline in for good measure.>=20 >> [0] https://godbolt.org/z/v9569KGTT > > It is because afaiu 'volatile' does not really match the semantics of wha= t you > are trying to do here. The noinline attribute does exactly what it says: it prevents inlining, but it does not prevent inter-procedural analysis. So if a function returns a constrant, the compiler will still use that constant elsewhere. Newer GCC versions support the noipa attribute. If you drop the static, you can use the weak attribute for compatibility with pretty much all GCC versions. But I think using volatile (without noinline for clarity) is fine here. >>>> +template >>>> +struct __fcntl_types_compatible_helper >>> >>> This makes the C++ tests fail with GCC 13: >>> >>> [...] >>> ../include/bits/../../io/bits/fcntl2.h:460:1: error: template with C li= nkage >>> 460 | template >>> | ^~~~~~~~ >>> ../misc/sys/cdefs.h:140:25: note: =E2=80=98extern "C"=E2=80=99 linkage = started here >>> 140 | # define __BEGIN_DECLS extern "C" { >>=20 >> Interesting. Any idea why this wasn't failing in my testing? > > My guess is because template with extern "C" are not really supported in = all > configurations. It's possible to use extern "C++" around the template. >> So maybe it is a good idea to move this all into fcntl3.h like it was >> done in Florian's patch? Then I'd include that after __END_DECLS and >> only if GCC >=3D 8. > > Yes, it would be better indeed. Even then, we may still have to use extern "C++", to preserve compatibility with C++ applications which include in an extern "C" block. See this commit: commit e37208ce86916af9510ffb9ce7b3c187986f07de Author: Florian Weimer Date: Sat Oct 22 17:33:26 2016 +0200 math.h: Wrap C++ bits in extern "C++" =20 It is still common to include system header files in an extern "C" block. This means that exiting 's own extern "C" block is not sufficient to get back to C++ mode. Use an extern "C++" wrapper instead. Thanks, Florian