From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) by sourceware.org (Postfix) with ESMTPS id 895AD3858404 for ; Wed, 28 Sep 2022 00:51:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 895AD3858404 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x1030.google.com with SMTP id gp22so1499576pjb.4 for ; Tue, 27 Sep 2022 17:51:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date; bh=AB9keY+Y7sWQBO6vAwaiWB0aIgh2h7jL3vlRaP5EZYA=; b=JZJczynEwOGm8bzRJaRxMNmSxO/vXZre7iCRSf7z25rYKsS1ZfAbK34e4FnnVjiLXE tahuF9i3o4/9sxJWK9GuHlHI3+XWXf2cv0JjR5aGdJhGUXvIZzP2Q2xDdtB/E7NE7qIi mzey74+CNLoJ0MDtNcQrPqOS/Ab66XJxcOI2HZqnEh72z1LRu51ZkQJSwcMpIULBDb67 lpssbxwU9Wt9E+0Y+vfpnkh/3tOBYSPu3lqr/3JpRvx6Httdb43WTbjasK11CA0/l8NR xkY6OSiA6wxReR5ekifzu9WGzZKyoLzIgX0VuMx5stoDfguoPJ8/N0O52HVKuWVzYi4G pCcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=AB9keY+Y7sWQBO6vAwaiWB0aIgh2h7jL3vlRaP5EZYA=; b=TS5PjOT4DC/llW/ER+02uE39ZV4YREpNTbomEJLdj2k/TyUfCUFGEb/EdLBYpjMD9H JABCFGZTT7u33n3LpY9gXPm9MtQKUl/lGK4H4/NDXIb0BN90+o6h/7oY5mKOWx4JupE0 eF75tRc6J4f/9mIjBcZyWUPh1ySJrl9wY2XRfAikilVuiUx0tU64RChnwwrkWn69eOxE c81ypiQeEdeQS1fJSrDRlQg4D7wgZUfsSDDbN3MxJz6Z+eZ37qgP81isHZD/umpIl5zt 7/4vWo1ShR4mUpEpUTdWEwnoGal1Q+SxUfck7B2mN+C9w5iht3CYmUqrRseuF/CH8Xty LlEA== X-Gm-Message-State: ACrzQf0bmiV66LkKxoIk1Z0mv8vTHQ0KI8irPXVI+X+q1gM95F8B2+jN RsJgzGYJlAJz32KqNt0z+Z2+s6ZFxZlLOA== X-Google-Smtp-Source: AMsMyM53dlopvdolzYXsgGOqM3+AHL21C7cVkyOgVtXhkL8Td0ncW+XxjkGza/ifxfy1pXNlzfKrzg== X-Received: by 2002:a17:902:bd8b:b0:179:d10e:97f with SMTP id q11-20020a170902bd8b00b00179d10e097fmr17259964pls.18.1664326306889; Tue, 27 Sep 2022 17:51:46 -0700 (PDT) Received: from ?IPV6:2601:681:8600:13d0::f0a? ([2601:681:8600:13d0::f0a]) by smtp.gmail.com with ESMTPSA id u13-20020a170902e5cd00b0016d6963cb12sm2203473plf.304.2022.09.27.17.51.44 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 27 Sep 2022 17:51:46 -0700 (PDT) Message-ID: <3b34b862-c71b-307c-f884-04fad0a3751b@gmail.com> Date: Tue, 27 Sep 2022 18:51:42 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.2.1 Subject: Re: [PATCH V2] place `const volatile' objects in read-only sections Content-Language: en-US To: gcc-patches@gcc.gnu.org References: <87zggiudcr.fsf@oracle.com> From: Jeff Law In-Reply-To: <87zggiudcr.fsf@oracle.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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: 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