public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Maxim Kuvyrkov <maxim.kuvyrkov@linaro.org>
Cc: Richard Biener <richard.guenther@gmail.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	Ian Lance Taylor <iant@golang.org>,
	Richard Henderson <richard.henderson@linaro.org>
Subject: Re: disable -Warray-bounds in libgo (PR 101374)
Date: Fri, 9 Jul 2021 13:46:25 -0600	[thread overview]
Message-ID: <f34b43ca-7897-5e33-e0e4-6becdd3ed0ff@gmail.com> (raw)
In-Reply-To: <9F56A005-47B2-4593-BCB4-B8FAD3533955@linaro.org>

On 7/9/21 7:19 AM, Maxim Kuvyrkov wrote:
>> On 9 Jul 2021, at 09:16, Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>
>> On Thu, Jul 8, 2021 at 8:02 PM Martin Sebor via Gcc-patches
>> <gcc-patches@gcc.gnu.org> wrote:
>>>
>>> Hi Ian,
>>>
>>> Yesterday's enhancement to -Warray-bounds has exposed a couple of
>>> issues in libgo where the code writes into an invalid constant
>>> address that the warning is designed to flag.
>>>
>>> On the assumption that those invalid addresses are deliberate,
>>> the attached patch suppresses these instances by using #pragma
>>> GCC diagnostic but I don't think I'm supposed to commit it (at
>>> least Git won't let me).  To avoid Go bootstrap failures please
>>> either apply the patch or otherwise suppress the warning (e.g.,
>>> by using a volatile pointer temporary).
>>
>> Btw, I don't think we should diagnose things like
>>
>>                 *(int*)0x21 = 0x21;
>>
>> when somebody literally writes that he'll be just annoyed by diagnostics.
> 
> And we have an assortment of similar cases in 32-bit ARM kernel-page helpers.
> 
> At the moment building libatomic for arm-linux-gnueabihf fails with:
> ===
> In function ‘select_test_and_set_8’,
>      inlined from ‘select_test_and_set_8’ at /home/tcwg-buildslave/workspace/tcwg-dev-build/snapshots/gcc.git~master/libatomic/tas_n.c:115:1:
> /home/tcwg-buildslave/workspace/tcwg-dev-build/snapshots/gcc.git~master/libatomic/config/linux/arm/host-config.h:42:34: error: array subscript 0 is outside array bounds of ‘unsigned int[0]’ [-Werror=array-bounds]
>     42 | #define __kernel_helper_version (*(unsigned int *)0xffff0ffc)
>        |                                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ===
> 
> In libatomic/config/linux/arm/host-config.h we have:
> ===
> /* Kernel helper for 32-bit compare-and-exchange.  */
> typedef int (__kernel_cmpxchg_t) (UWORD oldval, UWORD newval, UWORD *ptr);
> #define __kernel_cmpxchg (*(__kernel_cmpxchg_t *) 0xffff0fc0)
> 
> /* Kernel helper for 64-bit compare-and-exchange.  */
> typedef int (__kernel_cmpxchg64_t) (const U_8 * oldval, const U_8 * newval,
> 				    U_8 *ptr);
> #define __kernel_cmpxchg64 (*(__kernel_cmpxchg64_t *) 0xffff0f60)
> 
> /* Kernel helper for memory barrier.  */
> typedef void (__kernel_dmb_t) (void);
> #define __kernel_dmb (*(__kernel_dmb_t *) 0xffff0fa0)
> 
> /* Kernel helper page version number.  */
> #define __kernel_helper_version (*(unsigned int *)0xffff0ffc)
> ===

This failure is tracked in pr101379.  I have added an untested POC
patch with a possible way to avoid the warning.  Other approaches
are possible (I mention some in my comment on the bug) but they
are limited by the exposure of the constant address using macros.
Hiding them behind APIs instead would make it possible to suppress
the warnings via #pragma GCC diagnostic.  Alternatively, making
the addresses extern const variables would hide the constants from
the warning altogether.

With a suitable attribute an API or variable could also describe
the size of the object.  The AVR back end, for example, has two
attributes for hardwired addresses: io and address.  One of them
(or another one like it) could be made the target-indendependent
way to declare global variables of any type at fixed addresses,
e.g., like so:

   extern __attribute__ ((address (0xffff0fc0))) __kernel_cmpxchg64_t
   __kernel_cmpxchg;

Martin

  reply	other threads:[~2021-07-09 19:46 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-08 18:02 Martin Sebor
2021-07-08 21:07 ` Rainer Orth
2021-07-09 13:26   ` Rainer Orth
2021-07-12 16:34     ` Martin Sebor
2021-07-13  7:40       ` Rainer Orth
2021-07-09  6:16 ` Richard Biener
2021-07-09 13:19   ` Maxim Kuvyrkov
2021-07-09 19:46     ` Martin Sebor [this message]
2021-07-09 16:37   ` Martin Sebor
2021-07-10  2:49   ` Ian Lance Taylor
2021-07-13 18:30   ` Dimitar Dimitrov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f34b43ca-7897-5e33-e0e4-6becdd3ed0ff@gmail.com \
    --to=msebor@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=iant@golang.org \
    --cc=maxim.kuvyrkov@linaro.org \
    --cc=richard.guenther@gmail.com \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).