Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190125104353.2791-1-labbott@redhat.com> <CAK8P3a0xnc7RNbGhG7-hj+ay2keR7dh7MmDiUNKxfewsnatsgA@mail.gmail.com>
 <CANiq72=nMYcohMk29fzs3bMut1PXwfqYLZdbRh_AeTN-Z4FpxQ@mail.gmail.com>
 <AM0PR07MB387404F71D25553702966E27E49B0@AM0PR07MB3874.eurprd07.prod.outlook.com>
 <VI1PR07MB486420A106A8BC85BEE9FB2AE4960@VI1PR07MB4864.eurprd07.prod.outlook.com>
In-Reply-To: <VI1PR07MB486420A106A8BC85BEE9FB2AE4960@VI1PR07MB4864.eurprd07.prod.outlook.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Mon, 28 Jan 2019 15:40:31 +0100
Message-ID: <CAK8P3a1dLz3so3Z5cypOBeQN3GB-nM-RHNaeYOTEvovMrGhBPQ@mail.gmail.com>
Subject: Re: [RFC][PATCH] Update -Wattribute-alias for gcc9
To:     Bernd Edlinger <bernd.edlinger@hotmail.de>
Cc:     Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
        Laura Abbott <labbott@redhat.com>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Martin Sebor <msebor@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jan 28, 2019 at 2:28 PM Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
>
> On 1/25/19 1:24 PM, Bernd Edlinger wrote:
> > On 1/25/19 12:39 PM, Miguel Ojeda wrote:
> >> On Fri, Jan 25, 2019 at 11:58 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >>>
> >>> On Fri, Jan 25, 2019 at 11:43 AM Laura Abbott <labbott@redhat.com> wrote:
> >
> > I believe it is not intentional to break the old syntax of the
> > pragma.  There will be new -Wattribute-alias=1 and -Wattribute-alias=2
> > and -Wattribute-alias is easy to retain as an alias for -Wattribute-alias=1.
> > That is what my patch will do.
> >
>
> Okay, I committed the -Wattribute-alias patch to gcc trunk, now
> as https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268336 .
> So there will be no need for a workaround on your side.
>
> Also fixed a few false positive -Waddress-of-packed-member warnings with
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268118 and
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=268337 .
>
> However there remain a lot of warnings from -Waddress-of-packed-member,
> that look more or less valid, has anybody an idea how to handle
> these?

The best idea I have is "one at a time", which unfortunately
does mean a lot of work. We can degrade the warning to the
W=1 level in the kernel by disabling it by default and re-enabling
it in scripts/Makefile.extrawarn, but we probably want to address
many of them anyway. Looking at the most common examples
(listed by number of instances in Laura's build log)

     64  warning: taking address of packed member of 'struct
scif_window' may result in an unaligned pointer value
[-Waddress-of-packed-member]

Structure should not be packed at all, it's kernel internal (I hope at least,
since it contains pointers to kernel structures), and packing it does lead to
unaligned accesses.

     71  warning: converting a packed 'struct desc_struct' pointer
(alignment 1) to a 'u32' {aka 'unsigned int'} pointer (alignment 4)
may result in an unaligned pointer value [-Waddress-of-packed-member]

'__packed' does not seem to be required here. This is a structure of bit
fields, which IIRC means the layout is architecture dependent, but
it is fully packed already.

     76  warning: converting a packed 'struct opa_smp' pointer
(alignment 1) to a 'struct ib_mad_hdr' pointer (alignment 8) may
result in an unaligned pointer value [-Waddress-of-packed-member]

We cast the pointers multiple times, from aligned to packed and back to aligned,
so I assume there is no bug, but it's unclear why the structure was ever marked
as packed.

     81  warning: taking address of packed member of 'struct
adf_accel_dev' may result in an unaligned pointer value
[-Waddress-of-packed-member]

Should clearly not be packed.

    122  warning: taking address of packed member of 'struct
fc_els_flogi' may result in an unaligned pointer value
[-Waddress-of-packed-member]

I'd suggest changing it to only mark the unaligned members as packed,
not the structure itself.

    182  warning: taking address of packed member of 'struct
rtl818x_csr' may result in an unaligned pointer value
[-Waddress-of-packed-member]

Cannot be packed, since this is an mmio structure, need to
carefully rework structure layout in case there are some packed
members we never access.

   138  warning: taking address of packed member of 'struct ib_reth'
may result in an unaligned pointer value [-Waddress-of-packed-member]
    414  warning: taking address of packed member of 'struct
ib_atomic_eth' may result in an unaligned pointer value
[-Waddress-of-packed-member]

I suppose ib_u64_get should take a packed argument, it already uses
get_unaligned_be64()
internally to avoid bugs.

       Arnd