From: "Maciej W. Rozycki" <macro@linux-mips.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Ralf Baechle <ralf@linux-mips.org>,
Paul Burton <paul.burton@mips.com>,
James Hogan <jhogan@kernel.org>,
Nathan Chancellor <natechancellor@gmail.com>,
Eli Friedman <efriedma@quicinc.com>,
Hassan Naveed <hnaveed@wavecomp.com>,
Stephen Kitt <steve@sk2.org>,
Serge Semin <fancer.lancer@gmail.com>,
Mike Rapoport <rppt@linux.ibm.com>,
Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@suse.com>,
linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
clang-built-linux@googlegroups.com
Subject: Re: [PATCH] mips: avoid explicit UB in assignment of mips_io_port_base
Date: Mon, 29 Jul 2019 23:16:45 +0100 (BST) [thread overview]
Message-ID: <alpine.LFD.2.21.1907292302451.16059@eddie.linux-mips.org> (raw)
In-Reply-To: <20190729211014.39333-1-ndesaulniers@google.com>
On Mon, 29 Jul 2019, Nick Desaulniers wrote:
> The code in question is modifying a variable declared const through
> pointer manipulation. Such code is explicitly undefined behavior, and
> is the lone issue preventing malta_defconfig from booting when built
> with Clang:
>
> If an attempt is made to modify an object defined with a const-qualified
> type through use of an lvalue with non-const-qualified type, the
> behavior is undefined.
>
> LLVM is removing such assignments. A simple fix is to not declare
> variables const that you plan on modifying. Limiting the scope would be
> a better method of preventing unwanted writes to such a variable.
>
> Further, the code in question mentions "compiler bugs" without any links
> to bug reports, so it is difficult to know if the issue is resolved in
> GCC. The patch was authored in 2006, which would have been GCC 4.0.3 or
> 4.1.1. The minimal supported version of GCC in the Linux kernel is
> currently 4.6.
It's somewhat older than that. My investigation points to:
commit c94e57dcd61d661749d53ee876ab265883b0a103
Author: Ralf Baechle <ralf@linux-mips.org>
Date: Sun Nov 25 09:25:53 2001 +0000
Cleanup of include/asm-mips/io.h. Now looks neat and harmless.
However the purpose of the arrangement does not appear to me to be
particularly specific to a compiler version.
> For what its worth, there was UB before the commit in question, it just
> added a barrier and got lucky IRT codegen. I don't think there's any
> actual compiler bugs related, just runtime bugs due to UB.
Does your solution preserves the original purpose of the hack though as
documented in the comment you propose to be removed?
Clearly it was defined enough to work for almost 18 years, so it would be
good to keep the optimisation functionally by using different means that
do not rely on UB. This variable is assigned at most once throughout the
life of the kernel and then early on, so considering it r/w with all the
consequences for all accesses does not appear to me to be a good use of
it.
Maybe a piece of inline asm to hide the initialisation or suchlike then?
Maciej
next prev parent reply other threads:[~2019-07-29 22:16 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-29 21:10 [PATCH] mips: avoid explicit UB in assignment of mips_io_port_base Nick Desaulniers
2019-07-29 21:15 ` Nathan Chancellor
2019-07-29 22:16 ` Maciej W. Rozycki [this message]
2019-07-30 17:21 ` Paul Burton
2019-08-07 21:12 ` Nick Desaulniers
2019-08-20 17:15 ` Nick Desaulniers
2019-08-23 17:16 ` Nick Desaulniers
2019-08-24 14:12 ` Paul Burton
2019-08-26 17:31 ` Nick Desaulniers
2019-08-24 14:12 ` Paul Burton
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=alpine.LFD.2.21.1907292302451.16059@eddie.linux-mips.org \
--to=macro@linux-mips.org \
--cc=akpm@linux-foundation.org \
--cc=clang-built-linux@googlegroups.com \
--cc=efriedma@quicinc.com \
--cc=fancer.lancer@gmail.com \
--cc=hnaveed@wavecomp.com \
--cc=jhogan@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=mhocko@suse.com \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
--cc=paul.burton@mips.com \
--cc=ralf@linux-mips.org \
--cc=rppt@linux.ibm.com \
--cc=steve@sk2.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 NNTP newsgroup(s).