Message ID | 87zggiudcr.fsf@oracle.com |
---|---|
State | New, archived |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6a10:20da:b0:2d3:3019:e567 with SMTP id n26csp132293pxc; Fri, 5 Aug 2022 04:42:33 -0700 (PDT) X-Google-Smtp-Source: AA6agR5dcWS9sBeWsKKcuZObNvSvrQVnv2KG7xzzGKwnBlesU2OcBFkqWKtwgMfMWKJJ5IFitLqQ X-Received: by 2002:a05:6402:3485:b0:43d:7fe0:74d1 with SMTP id v5-20020a056402348500b0043d7fe074d1mr6274511edc.413.1659699753120; Fri, 05 Aug 2022 04:42:33 -0700 (PDT) Received: from sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id md17-20020a170906ae9100b0072aebf9cc60si3761701ejb.619.2022.08.05.04.42.32 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Aug 2022 04:42:33 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=tZ0+4J2M; arc=fail (signature failed); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A8E3D385840B for <ouuuleilei@gmail.com>; Fri, 5 Aug 2022 11:42:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A8E3D385840B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1659699751; bh=H1Hj9KvjxHtSU0khyDP18/amj40vwzFsNj2CAMq/ZC8=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=tZ0+4J2MQ2mOyki/SHnhkN2ITg5avfY9y/+HOyrmaTVcz7eTzBguzJ1P9MTXlUZP3 OE4FIhhaOx8D8eeghlxSs0IsUjYDFMUYWZWBj8BjwI/dJLQi6H2nvVtOj9wvIjLYYJ 0BQmnBj5WvTjk/3267J0AZaPEwDSttTIko+D49i4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by sourceware.org (Postfix) with ESMTPS id EADE038582BE for <gcc-patches@gcc.gnu.org>; Fri, 5 Aug 2022 11:41:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org EADE038582BE Received: from pps.filterd (m0246629.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 275A5qSo024182 for <gcc-patches@gcc.gnu.org>; Fri, 5 Aug 2022 11:41:35 GMT Received: from phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (phxpaimrmta03.appoci.oracle.com [138.1.37.129]) by mx0b-00069f02.pphosted.com (PPS) with ESMTPS id 3hmvh9y6hg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gcc-patches@gcc.gnu.org>; Fri, 05 Aug 2022 11:41:34 +0000 Received: from pps.filterd (phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com [127.0.0.1]) by phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (8.17.1.5/8.17.1.5) with ESMTP id 275831p2001056 for <gcc-patches@gcc.gnu.org>; Fri, 5 Aug 2022 11:41:34 GMT Received: from nam02-sn1-obe.outbound.protection.outlook.com (mail-sn1anam02lp2042.outbound.protection.outlook.com [104.47.57.42]) by phxpaimrmta03.imrmtpd1.prodappphxaev1.oraclevcn.com (PPS) with ESMTPS id 3hp57u4gn3-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for <gcc-patches@gcc.gnu.org>; Fri, 05 Aug 2022 11:41:34 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FkfsADXO/B6bNDfT8/MFAvt6P2CVubds/3OtiIsj4e8TRdNwTPscXD+mvqw5PWhUfsg+dx/+tmAla7gRMCta5b3NXUKswgOwNXC5+Fb7vcyXQBwbrk7fn5vIFS9zJcHd5Ij8H7O/Qj1rhNLz6yEDyV6PrFzs3DYwq//EO0A+IdRY2LpFrJcc/TS7gxBotEc5/0bp3ZwYHGcq1GrwEZGECN9iggBJzUXR8jiIlXpbOWur0DsAvZYc6zrrFYIf/T+qdt1uJ90+AY/0a5Vj7KmzuHFfnlcxINvXvkERkKnxcvJXrXHpSUl0/8vi+35xrh43gpmMarXbkoSDmuP1dzGIDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=H1Hj9KvjxHtSU0khyDP18/amj40vwzFsNj2CAMq/ZC8=; b=QD9K2HLnk58cSaUeO+bxcHi4BA1dTbR+yt+3/bWiuie84UY6PITRWmJnksc8Ev2cQCxFE6hmSpiNF9jgqEx32p516EUoAHdu5TiyCvNRgmT+ADuCLpIpG4qm6Tqevx3ExxV0zlvpEmE2tSdCusk5SDX6IViD+0ZK1ER1rnqQmwuRGf2IQDji3UJXp/oIIZz4CvW9x0pFzKAVobq0ij2XJ0z7EIGSjKQlQyQOHX1J0iJPof9PruoclROZ7sfY8TLbpptOnWKqSvOHG+i81N4C21UdIXT+FozAPiQLZfroD3IpjlVjNWH74wY7mk9pLFoceW7vZ77etXz4q4w1Y+b6MQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none Received: from BYAPR10MB2888.namprd10.prod.outlook.com (2603:10b6:a03:88::32) by BYAPR10MB2632.namprd10.prod.outlook.com (2603:10b6:a02:b2::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5504.16; Fri, 5 Aug 2022 11:41:30 +0000 Received: from BYAPR10MB2888.namprd10.prod.outlook.com ([fe80::b5ee:262a:b151:2fdd]) by BYAPR10MB2888.namprd10.prod.outlook.com ([fe80::b5ee:262a:b151:2fdd%4]) with mapi id 15.20.5504.016; Fri, 5 Aug 2022 11:41:30 +0000 To: gcc-patches@gcc.gnu.org Subject: [PATCH V2] place `const volatile' objects in read-only sections Date: Fri, 05 Aug 2022 13:41:24 +0200 Message-ID: <87zggiudcr.fsf@oracle.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Content-Type: text/plain X-ClientProxiedBy: LO2P265CA0478.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a2::34) To BYAPR10MB2888.namprd10.prod.outlook.com (2603:10b6:a03:88::32) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: fbd149b8-fba8-4ea7-0c2b-08da76d7704f X-MS-TrafficTypeDiagnostic: BYAPR10MB2632:EE_ X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: IPnUrXyS7eHDmkLlIOGUZoU0+xWuao/fKAjbgsRabi1s0Q0pCWg7/FuoQoYaofI2dnAH+xd2qrczMBji0I6guxli9XT0h6LvLymawMCYBXUpD3viFGNTiju9XYLZ/cKrpiY+Ckq6rrFeiZnl/ieI9dJMmAG3YLgr3cXgT+TY1xWCPIsW5C77iCHb1P5eLuOVcng2Ez78jiOBblMmLpHKIC85DoDipW+UKT4sSpRm0E3NO00ZWGYbMp60jOti9ot4aT+lwwH3JOZF9+XxxufSNljVNIeytufXr1w3tlqQoGiUuqLJp4b87dYu35wr/RFkeWhCIN47D7YBGwyrP4OJD5rWN8RYuM0MDGbbtBnF+zOdk8Jt//T7PPkWrNjqKNmCh7Itez2nrHfvAiBbk2PQTQOZF/PqIQtIscGxKRZc1F4tZh3n4Zf2Yp3cTQEjH64HHuPc2YTlKFG0LCUR/a95be5GDa60zuWzPm7CS68SN0bpsikR52r8YmvYT7P/X8Bsb0g34GAC7ODWb2GCih3zKhHtDizyfkjxmh9toX7PLEwbO67lL/rpGvOv6wbdP4uWkUF4hoLN+SCekc2Wl8PhGArpKqHSstEE5WGRFXvb9NB9/i8W2hwOzfkXe/ISMeoGOpFua6LuyJB/wNbLPqEEzsaiAYmkBnfrnn8WgMM/n0F4xYDjXXDqCG8H/Ea844OR9a0bhuKbUuZHtraRXMH0DcwoRor3pGd0s527BLW4/rtHHndLqQ4B8cT9XW1992K8WeDsWhPhMObgUFJyFVNxbNVjbXcoAgmam/0Au1h6KoqObysM7MIBlBcKzmvAfYmwLz3A0BesO+MdjRqxTIyhOg1FX41ZrZFr0jIO+hoqFzXDmJ0SbvLNqrZnw5/VWkg0ujyJmmFse2KXAb7ov+Zu3Q== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR10MB2888.namprd10.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230016)(39860400002)(376002)(366004)(136003)(346002)(396003)(6916009)(316002)(36756003)(52116002)(84970400001)(41300700001)(6506007)(6512007)(6666004)(26005)(6486002)(966005)(478600001)(2906002)(86362001)(66476007)(66556008)(66946007)(8676002)(8936002)(5660300002)(186003)(2616005)(83380400001)(38100700002)(38350700002)(81973001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: Zf98Cdj/tXz1F3CZXlNOD5BzV8LIytLRee7OwKOLXnCO8G31N0aJ/eHqCgIX9LzIjs8gJGaK2+ORREjpQu9BcKwyVWHhPRQhe6bB2V8FOMVy4XPb+zqaojA5IEMDzjuVkW07EAJvKzPlVy98uFVhMd4Da5SgKXZf4ixXPRpAjKbpwNoPeA2spE8gzyUn8SolaBUCmVH9DiuVPyAZ8Pv/gYskE1X6BkXoO5HyYv7qLiIjFw0SYF0w52PgD1RuKTwUqcmcHFN3JrYJ6GrI9WjL2jrQ1ObVpqWBSb/XbtYcCHO6Z9NFa3p5dUx/U6aivPl9jdKZbo3DtStiHzQKNjFOB4y+eeGWfIBvrWC+1y7XfWUO67oyUexkdW+pQ4RpRDu9E0XpArTU8zNkrx6Ua13YHjPDpWMuLg9PGWQZ3YxYzB25fZvN4cWaetTFER7DfXltO+qbveUrUTP5cib8A0XN1q8haUL1imPcgJkIkHKmykD54f8rhh4O6mKxIXXfAcVoYxPcqWKye8P8zoErbEdD8aoyiRpxmvYeYuu2pbXgLi5rCC7i2wPRJsdtOa76dhFf1ZlL8OF0iGkTYL+Z9j3eGRvA9sTrLhUefID3fRv2LUqBt4VvZBmnyO43vJizZl1Ti2RlEPQAmrRP7Znc9xjZaq/3lNqKbdaRU6G3Kid++ru2iTC8DlJ51XxJ8/apVOYllKVEQFnb6mGZUonqkH8HmT9r4ZmMH78tazdoGpkqmwMZg1H9j0mAYxwfXLGDdW06cpbcRNtj8n7vTgNY8VzTt8c6TqKPE99sit0kK4JVhNC0xUpVSOtR1M5Q1L1EeLbL2plaF2SRGNfIMzSTnAIqIZ7OOkUYAgeEl5P1QSMRd+Y7EP8HCTO+yhQHwm4cxvfK7eT1TdxtyQT/UZTsrXrwAlk0os89jwiquDGdhGKEK0+JuwhSeuH4O7S00EV5zepkCCe97w1m1E5rTrgnHCFc56cwkbPOzeRVWXhLTU4nLNA9FM7FjWWdeKht54Qpli8l8FgSbzxivg5x4HLdbzP3xHE/X9XDl7UoKTfvVRXC1mbGGDg9EiaUmEkznwp2nMejOJmZMZubA5F74Q6Pvs3/TiB1/oEOiadJ1Ywhvr3Pi27Kc0qxya2NZGhHzRbSxT8Jnjkwp1IPM0vj4CPHoWCPYC2Ab1KMI0ex10yZw9Q99H6o9nje+jOolxGno4N8N031nVWqpssP6O6YLZYBrYf9qDZtwZu7JG71ejJ4Pw6wEUrkirJASZWUUdHicgzXoKrb/lAEdWhsWv15n/ZH7R+rLFcXyOBHKisw2pIoTqz89mDPZSY5JE9I6Q/EE07L1jNyaMw87eYf6xl94GvUh9JbUX/VZvzm6mXmNcM0DkXqczatjqMeu8Zi5n1PgrIbdKz43S1iYSaCUyRcN65vAlE6wyPHozULS3LrUes3/BabX45EWOkbqaLr/o5j5d0mJwuCXsi/GHbMnKtS+uu18ZGC7KD3+0JXjkQvkfk6XRUNZAobE8Zkz02I9AbbJryXfMt6TeFCuS3QJK+RPnMnZoasBIIIcbt6eSRwMXz3wqfV6WDU0sHwOYLQuegwB1DXPDa98qdhuVz0EtVLIop2t+Nqiw== X-OriginatorOrg: oracle.com X-MS-Exchange-CrossTenant-Network-Message-Id: fbd149b8-fba8-4ea7-0c2b-08da76d7704f X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2888.namprd10.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Aug 2022 11:41:30.1246 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Fz0AyQm+kajxnb6MfgFNybdDDfHl9sYtTDCYCOT5kS8Tuyjd1aAvzYQAUrzdR3brC3N5aUfifbgroweD8og5YtBV5BPRI86eh8JBx84Mw2U= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR10MB2632 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-05_05,2022-08-05_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 mlxlogscore=999 adultscore=0 spamscore=0 bulkscore=0 suspectscore=0 mlxscore=0 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2206140000 definitions=main-2208050058 X-Proofpoint-ORIG-GUID: -vC56tryPXcBpELCkFM7-yC1Q2LfYHI8 X-Proofpoint-GUID: -vC56tryPXcBpELCkFM7-yC1Q2LfYHI8 X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: "Jose E. Marchesi via Gcc-patches" <gcc-patches@gcc.gnu.org> Reply-To: "Jose E. Marchesi" <jose.marchesi@oracle.com> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1740282636159434589?= X-GMAIL-MSGID: =?utf-8?q?1740321328426685875?= |
Series |
[V2] place `const volatile' objects in read-only sections
|
|
Commit Message
Jose E. Marchesi
Aug. 5, 2022, 11:41 a.m. UTC
[Changes from V1: - Added a test.] It is common for C BPF programs to use variables that are implicitly set by the BPF loader and run-time. It is also necessary for these variables to be stored in read-only storage so the BPF verifier recognizes them as such. This leads to declarations using both `const' and `volatile' qualifiers, like this: const volatile unsigned char is_allow_list = 0; Where `volatile' is used to avoid the compiler to optimize out the variable, or turn it into a constant, and `const' to make sure it is placed in .rodata. Now, it happens that: - GCC places `const volatile' objects in the .data section, under the assumption that `volatile' somehow voids the `const'. - LLVM places `const volatile' objects in .rodata, under the assumption that `volatile' is orthogonal to `const'. So there is a divergence, that has practical consequences: it makes BPF programs compiled with GCC to not work properly. When looking into this, I found this bugzilla: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25521 "change semantics of const volatile variables" which was filed back in 2005, long ago. This report was already asking to put `const volatile' objects in .rodata, questioning the current behavior. While discussing this in the #gcc IRC channel I was pointed out to the following excerpt from the C18 spec: 6.7.3 Type qualifiers / 5 The properties associated with qualified types are meaningful only for expressions that are lval-values [note 135] 135) The implementation may place a const object that is not volatile in a read-only region of storage. Moreover, the implementation need not allocate storage for such an object if its $ address is never used. This footnote may be interpreted as if const objects that are volatile shouldn't be put in read-only storage. Even if I personally was not very convinced of that interpretation (see my earlier comment in BZ 25521) I filed the following issue in the LLVM tracker in order to discuss the matter: https://github.com/llvm/llvm-project/issues/56468 As you can see, Aaron Ballman, one of the LLVM hackers, asked the WG14 reflectors about this. He reported that the reflectors don't think footnote 135 has any normative value. So, not having a normative mandate on either direction, there are two options: a) To change GCC to place `const volatile' objects in .rodata instead of .data. b) To change LLVM to place `const volatile' objects in .data instead of .rodata. Considering that: - One target (bpf-unknown-none) breaks with the current GCC behavior. - No target/platform relies on the GCC behavior, that we know. - Changing the LLVM behavior at this point would be very severely traumatic for the BPF people and their users. I think the right thing to do at this point is a). Therefore this patch. Regtested in x86_64-linux-gnu and bpf-unknown-none. No regressions observed. gcc/ChangeLog: PR middle-end/25521 * varasm.cc (categorize_decl_for_section): Place `const volatile' objects in read-only sections. (default_select_section): Likewise. gcc/testsuite/ChangeLog: PR middle-end/25521 * lib/target-supports.exp (check_effective_target_elf): Define. * gcc.dg/pr25521.c: New test. --- gcc/testsuite/gcc.dg/pr25521.c | 10 ++++++++++ gcc/testsuite/lib/target-supports.exp | 10 ++++++++++ gcc/varasm.cc | 3 --- 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr25521.c
Comments
ping > [Changes from V1: > - Added a test.] > > It is common for C BPF programs to use variables that are implicitly > set by the BPF loader and run-time. It is also necessary for these > variables to be stored in read-only storage so the BPF verifier > recognizes them as such. This leads to declarations using both > `const' and `volatile' qualifiers, like this: > > const volatile unsigned char is_allow_list = 0; > > Where `volatile' is used to avoid the compiler to optimize out the > variable, or turn it into a constant, and `const' to make sure it is > placed in .rodata. > > Now, it happens that: > > - GCC places `const volatile' objects in the .data section, under the > assumption that `volatile' somehow voids the `const'. > > - LLVM places `const volatile' objects in .rodata, under the > assumption that `volatile' is orthogonal to `const'. > > So there is a divergence, that has practical consequences: it makes > BPF programs compiled with GCC to not work properly. > > When looking into this, I found this bugzilla: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25521 > "change semantics of const volatile variables" > > which was filed back in 2005, long ago. This report was already > asking to put `const volatile' objects in .rodata, questioning the > current behavior. > > While discussing this in the #gcc IRC channel I was pointed out to the > following excerpt from the C18 spec: > > 6.7.3 Type qualifiers / 5 The properties associated with qualified > types are meaningful only for expressions that are > lval-values [note 135] > > 135) The implementation may place a const object that is not > volatile in a read-only region of storage. Moreover, the > implementation need not allocate storage for such an object if > its $ address is never used. > > This footnote may be interpreted as if const objects that are volatile > shouldn't be put in read-only storage. Even if I personally was not > very convinced of that interpretation (see my earlier comment in BZ > 25521) I filed the following issue in the LLVM tracker in order to > discuss the matter: > > https://github.com/llvm/llvm-project/issues/56468 > > As you can see, Aaron Ballman, one of the LLVM hackers, asked the WG14 > reflectors about this. He reported that the reflectors don't think > footnote 135 has any normative value. > > So, not having a normative mandate on either direction, there are two > options: > > a) To change GCC to place `const volatile' objects in .rodata instead > of .data. > > b) To change LLVM to place `const volatile' objects in .data instead > of .rodata. > > Considering that: > > - One target (bpf-unknown-none) breaks with the current GCC behavior. > > - No target/platform relies on the GCC behavior, that we know. > > - Changing the LLVM behavior at this point would be very severely > traumatic for the BPF people and their users. > > I think the right thing to do at this point is a). > Therefore this patch. > > Regtested in x86_64-linux-gnu and bpf-unknown-none. > No regressions observed. > > gcc/ChangeLog: > > PR middle-end/25521 > * varasm.cc (categorize_decl_for_section): Place `const volatile' > objects in read-only sections. > (default_select_section): Likewise. > > gcc/testsuite/ChangeLog: > > PR middle-end/25521 > * lib/target-supports.exp (check_effective_target_elf): Define. > * gcc.dg/pr25521.c: New test. > --- > gcc/testsuite/gcc.dg/pr25521.c | 10 ++++++++++ > gcc/testsuite/lib/target-supports.exp | 10 ++++++++++ > gcc/varasm.cc | 3 --- > 3 files changed, 20 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/pr25521.c > > diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c > new file mode 100644 > index 00000000000..74fe2ae6626 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr25521.c > @@ -0,0 +1,10 @@ > +/* PR middle-end/25521 - place `const volatile' objects in read-only > + sections. > + > + { dg-require-effective-target elf } > + { dg-do compile } */ > + > +const volatile int foo = 30; > + > + > +/* { dg-final { scan-assembler "\\.rodata" } } */ > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index 04a2a8e8659..c663d59264b 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -483,6 +483,16 @@ proc check_effective_target_alias { } { > } > } > > +# Returns 1 if the target uses the ELF object format, 0 otherwise. > + > +proc check_effective_target_elf { } { > + if { [gcc_target_object_format] == "elf" } { > + return 1; > + } else { > + return 0; > + } > +} > + > # Returns 1 if the target toolchain supports ifunc, 0 otherwise. > > proc check_ifunc_available { } { > diff --git a/gcc/varasm.cc b/gcc/varasm.cc > index 4db8506b106..7864db11faf 100644 > --- a/gcc/varasm.cc > +++ b/gcc/varasm.cc > @@ -6971,7 +6971,6 @@ default_select_section (tree decl, int reloc, > { > if (! ((flag_pic && reloc) > || !TREE_READONLY (decl) > - || TREE_SIDE_EFFECTS (decl) > || !TREE_CONSTANT (decl))) > return readonly_data_section; > } > @@ -7005,7 +7004,6 @@ categorize_decl_for_section (const_tree decl, int reloc) > if (bss_initializer_p (decl)) > ret = SECCAT_BSS; > else if (! TREE_READONLY (decl) > - || TREE_SIDE_EFFECTS (decl) > || (DECL_INITIAL (decl) > && ! TREE_CONSTANT (DECL_INITIAL (decl)))) > { > @@ -7046,7 +7044,6 @@ categorize_decl_for_section (const_tree decl, int reloc) > else if (TREE_CODE (decl) == CONSTRUCTOR) > { > if ((reloc & targetm.asm_out.reloc_rw_mask ()) > - || TREE_SIDE_EFFECTS (decl) > || ! TREE_CONSTANT (decl)) > ret = SECCAT_DATA; > else
On 8/5/22 05:41, Jose E. Marchesi via Gcc-patches wrote: > [Changes from V1: > - Added a test.] > > It is common for C BPF programs to use variables that are implicitly > set by the BPF loader and run-time. It is also necessary for these > variables to be stored in read-only storage so the BPF verifier > recognizes them as such. This leads to declarations using both > `const' and `volatile' qualifiers, like this: > > const volatile unsigned char is_allow_list = 0; > > Where `volatile' is used to avoid the compiler to optimize out the > variable, or turn it into a constant, and `const' to make sure it is > placed in .rodata. > > Now, it happens that: > > - GCC places `const volatile' objects in the .data section, under the > assumption that `volatile' somehow voids the `const'. > > - LLVM places `const volatile' objects in .rodata, under the > assumption that `volatile' is orthogonal to `const'. > > So there is a divergence, that has practical consequences: it makes > BPF programs compiled with GCC to not work properly. > > When looking into this, I found this bugzilla: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25521 > "change semantics of const volatile variables" > > which was filed back in 2005, long ago. This report was already > asking to put `const volatile' objects in .rodata, questioning the > current behavior. > > While discussing this in the #gcc IRC channel I was pointed out to the > following excerpt from the C18 spec: > > 6.7.3 Type qualifiers / 5 The properties associated with qualified > types are meaningful only for expressions that are > lval-values [note 135] > > 135) The implementation may place a const object that is not > volatile in a read-only region of storage. Moreover, the > implementation need not allocate storage for such an object if > its $ address is never used. > > This footnote may be interpreted as if const objects that are volatile > shouldn't be put in read-only storage. Even if I personally was not > very convinced of that interpretation (see my earlier comment in BZ > 25521) I filed the following issue in the LLVM tracker in order to > discuss the matter: > > https://github.com/llvm/llvm-project/issues/56468 > > As you can see, Aaron Ballman, one of the LLVM hackers, asked the WG14 > reflectors about this. He reported that the reflectors don't think > footnote 135 has any normative value. > > So, not having a normative mandate on either direction, there are two > options: > > a) To change GCC to place `const volatile' objects in .rodata instead > of .data. > > b) To change LLVM to place `const volatile' objects in .data instead > of .rodata. > > Considering that: > > - One target (bpf-unknown-none) breaks with the current GCC behavior. > > - No target/platform relies on the GCC behavior, that we know. > > - Changing the LLVM behavior at this point would be very severely > traumatic for the BPF people and their users. > > I think the right thing to do at this point is a). > Therefore this patch. > > Regtested in x86_64-linux-gnu and bpf-unknown-none. > No regressions observed. > > gcc/ChangeLog: > > PR middle-end/25521 > * varasm.cc (categorize_decl_for_section): Place `const volatile' > objects in read-only sections. > (default_select_section): Likewise. > > gcc/testsuite/ChangeLog: > > PR middle-end/25521 > * lib/target-supports.exp (check_effective_target_elf): Define. > * gcc.dg/pr25521.c: New test. The best use I've heard for const volatile is stuff like hardware status registers which are readonly from the standpoint of the compiler, but which are changed by the hardware. But for those, we're looking for the const to trigger compiler diagnostics if we try to write the value. The volatile (of course) indicates the value changes behind our back. What you're trying to do seems to parallel that case reasonably well for the volatile aspect. You want to force the compiler to read the data for every access. Your need for the const is a bit different. Instead of looking to get a diagnostic out of the compiler if its modified, you need the data to live in .rodata so the BPF verifier knows the compiler/code won't change the value. Presumably the BPF verifier can't read debug info to determine the const-ness. I'm not keen on the behavior change, but nobody else is stepping in to review and I don't have a strong case to reject. So OK for the trunk. jeff
> On Sep 27, 2022, at 8:51 PM, Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > On 8/5/22 05:41, Jose E. Marchesi via Gcc-patches wrote: >> [Changes from V1: >> - Added a test.] >> >> It is common for C BPF programs to use variables that are implicitly >> set by the BPF loader and run-time. It is also necessary for these >> variables to be stored in read-only storage so the BPF verifier >> recognizes them as such. This leads to declarations using both >> `const' and `volatile' qualifiers, like this: >> >> const volatile unsigned char is_allow_list = 0; >> >> Where `volatile' is used to avoid the compiler to optimize out the >> variable, or turn it into a constant, and `const' to make sure it is >> placed in .rodata. >> >> Now, it happens that: >> >> - GCC places `const volatile' objects in the .data section, under the >> assumption that `volatile' somehow voids the `const'. >> >> - LLVM places `const volatile' objects in .rodata, under the >> assumption that `volatile' is orthogonal to `const'. >> ... > > The best use I've heard for const volatile is stuff like hardware status registers which are readonly from the standpoint of the compiler, but which are changed by the hardware. But for those, we're looking for the const to trigger compiler diagnostics if we try to write the value. The volatile (of course) indicates the value changes behind our back. I'd go a bit further and say that this is the only use of const volatile that makes any sense. > What you're trying to do seems to parallel that case reasonably well for the volatile aspect. You want to force the compiler to read the data for every access. > > Your need for the const is a bit different. Instead of looking to get a diagnostic out of the compiler if its modified, you need the data to live in .rodata so the BPF verifier knows the compiler/code won't change the value. Presumably the BPF verifier can't read debug info to determine the const-ness. > > I'm not keen on the behavior change, but nobody else is stepping in to review and I don't have a strong case to reject. So OK for the trunk. A const volatile that sits in memory feels like a programmer error. Instead of worrying about how it's handled, would it not make more sense to tag it with a warning? paul
> On 8/5/22 05:41, Jose E. Marchesi via Gcc-patches wrote: >> [Changes from V1: >> - Added a test.] >> >> It is common for C BPF programs to use variables that are implicitly >> set by the BPF loader and run-time. It is also necessary for these >> variables to be stored in read-only storage so the BPF verifier >> recognizes them as such. This leads to declarations using both >> `const' and `volatile' qualifiers, like this: >> >> const volatile unsigned char is_allow_list = 0; >> >> Where `volatile' is used to avoid the compiler to optimize out the >> variable, or turn it into a constant, and `const' to make sure it is >> placed in .rodata. >> >> Now, it happens that: >> >> - GCC places `const volatile' objects in the .data section, under the >> assumption that `volatile' somehow voids the `const'. >> >> - LLVM places `const volatile' objects in .rodata, under the >> assumption that `volatile' is orthogonal to `const'. >> >> So there is a divergence, that has practical consequences: it makes >> BPF programs compiled with GCC to not work properly. >> >> When looking into this, I found this bugzilla: >> >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25521 >> "change semantics of const volatile variables" >> >> which was filed back in 2005, long ago. This report was already >> asking to put `const volatile' objects in .rodata, questioning the >> current behavior. >> >> While discussing this in the #gcc IRC channel I was pointed out to the >> following excerpt from the C18 spec: >> >> 6.7.3 Type qualifiers / 5 The properties associated with qualified >> types are meaningful only for expressions that are >> lval-values [note 135] >> >> 135) The implementation may place a const object that is not >> volatile in a read-only region of storage. Moreover, the >> implementation need not allocate storage for such an object if >> its $ address is never used. >> >> This footnote may be interpreted as if const objects that are volatile >> shouldn't be put in read-only storage. Even if I personally was not >> very convinced of that interpretation (see my earlier comment in BZ >> 25521) I filed the following issue in the LLVM tracker in order to >> discuss the matter: >> >> https://github.com/llvm/llvm-project/issues/56468 >> >> As you can see, Aaron Ballman, one of the LLVM hackers, asked the WG14 >> reflectors about this. He reported that the reflectors don't think >> footnote 135 has any normative value. >> >> So, not having a normative mandate on either direction, there are two >> options: >> >> a) To change GCC to place `const volatile' objects in .rodata instead >> of .data. >> >> b) To change LLVM to place `const volatile' objects in .data instead >> of .rodata. >> >> Considering that: >> >> - One target (bpf-unknown-none) breaks with the current GCC behavior. >> >> - No target/platform relies on the GCC behavior, that we know. >> >> - Changing the LLVM behavior at this point would be very severely >> traumatic for the BPF people and their users. >> >> I think the right thing to do at this point is a). >> Therefore this patch. >> >> Regtested in x86_64-linux-gnu and bpf-unknown-none. >> No regressions observed. >> >> gcc/ChangeLog: >> >> PR middle-end/25521 >> * varasm.cc (categorize_decl_for_section): Place `const volatile' >> objects in read-only sections. >> (default_select_section): Likewise. >> >> gcc/testsuite/ChangeLog: >> >> PR middle-end/25521 >> * lib/target-supports.exp (check_effective_target_elf): Define. >> * gcc.dg/pr25521.c: New test. > > The best use I've heard for const volatile is stuff like hardware > status registers which are readonly from the standpoint of the > compiler, but which are changed by the hardware. But for those, > we're looking for the const to trigger compiler diagnostics if we try > to write the value. The volatile (of course) indicates the value > changes behind our back. > > What you're trying to do seems to parallel that case reasonably well > for the volatile aspect. You want to force the compiler to read the > data for every access. > > Your need for the const is a bit different. Instead of looking to get > a diagnostic out of the compiler if its modified, you need the data to > live in .rodata so the BPF verifier knows the compiler/code won't > change the value. Presumably the BPF verifier can't read debug info > to determine the const-ness. > > > I'm not keen on the behavior change, but nobody else is stepping in to > review and I don't have a strong case to reject. So OK for the trunk. Thanks. To me the biggest advantage of the change is that now there is no divergence on how GCC and Clang handle `const' and `volatile'. Just pushed to master after bootregtesting in x86_64-linux-gnu. Thanks! --- master-tests/all.sum 2022-09-29 12:35:30.994514528 +0200 +++ consvolatile-tests/all.sum 2022-09-29 12:35:41.152420906 +0200 @@ -173,7 +173,7 @@ # of expected failures 98 # of expected passes 14142 # of expected passes 15607 -# of expected passes 183688 +# of expected passes 183690 # of expected passes 232848 # of expected passes 2840 # of expected passes 44 @@ -170478,6 +170478,8 @@ PASS: gcc.dg/pr25376.c scan-assembler my_named_section PASS: gcc.dg/pr25376.c scan-assembler-symbol-section symbol simple$ (found simple) has section\ ^\\.?my_named_section|simple\\[DS\\]|^\\"\\.opd\\" (found my_named_section) PASS: gcc.dg/pr25376.c (test for excess errors) +PASS: gcc.dg/pr25521.c scan-assembler \\.rodata +PASS: gcc.dg/pr25521.c (test for excess errors) PASS: gcc.dg/pr25529.c scan-tree-dump optimized "& 2147483647" PASS: gcc.dg/pr25529.c (test for excess errors) PASS: gcc.dg/pr25530.c scan-tree-dump optimized "& -2|4294967294"
diff --git a/gcc/testsuite/gcc.dg/pr25521.c b/gcc/testsuite/gcc.dg/pr25521.c new file mode 100644 index 00000000000..74fe2ae6626 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr25521.c @@ -0,0 +1,10 @@ +/* PR middle-end/25521 - place `const volatile' objects in read-only + sections. + + { dg-require-effective-target elf } + { dg-do compile } */ + +const volatile int foo = 30; + + +/* { dg-final { scan-assembler "\\.rodata" } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 04a2a8e8659..c663d59264b 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -483,6 +483,16 @@ proc check_effective_target_alias { } { } } +# Returns 1 if the target uses the ELF object format, 0 otherwise. + +proc check_effective_target_elf { } { + if { [gcc_target_object_format] == "elf" } { + return 1; + } else { + return 0; + } +} + # Returns 1 if the target toolchain supports ifunc, 0 otherwise. proc check_ifunc_available { } { diff --git a/gcc/varasm.cc b/gcc/varasm.cc index 4db8506b106..7864db11faf 100644 --- a/gcc/varasm.cc +++ b/gcc/varasm.cc @@ -6971,7 +6971,6 @@ default_select_section (tree decl, int reloc, { if (! ((flag_pic && reloc) || !TREE_READONLY (decl) - || TREE_SIDE_EFFECTS (decl) || !TREE_CONSTANT (decl))) return readonly_data_section; } @@ -7005,7 +7004,6 @@ categorize_decl_for_section (const_tree decl, int reloc) if (bss_initializer_p (decl)) ret = SECCAT_BSS; else if (! TREE_READONLY (decl) - || TREE_SIDE_EFFECTS (decl) || (DECL_INITIAL (decl) && ! TREE_CONSTANT (DECL_INITIAL (decl)))) { @@ -7046,7 +7044,6 @@ categorize_decl_for_section (const_tree decl, int reloc) else if (TREE_CODE (decl) == CONSTRUCTOR) { if ((reloc & targetm.asm_out.reloc_rw_mask ()) - || TREE_SIDE_EFFECTS (decl) || ! TREE_CONSTANT (decl)) ret = SECCAT_DATA; else