linux-kernel - Re: [PATCH] MIPS: Fix inline asm input/output type mismatch in checksum.h used with Clang

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2102122116230.35623@angie.orcam.me.uk>
Date:   Fri, 12 Feb 2021 21:36:49 +0100 (CET)
From:   "Maciej W. Rozycki" <macro@...am.me.uk>
To:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>
cc:     Tiezhu Yang <yangtiezhu@...ngson.cn>, linux-mips@...r.kernel.org,
        linux-kernel@...r.kernel.org, Xuefeng Li <lixuefeng@...ngson.cn>,
        Alexander Potapenko <glider@...gle.com>
Subject: Re: [PATCH] MIPS: Fix inline asm input/output type mismatch in
 checksum.h used with Clang

On Wed, 27 Jan 2021, Thomas Bogendoerfer wrote:

> > Fix the following build error when make M=samples/bpf used with Clang:
> > 
> >   CLANG-bpf  samples/bpf/sockex2_kern.o
> > In file included from samples/bpf/sockex2_kern.c:7:
> > In file included from ./include/uapi/linux/if_tunnel.h:7:
> > In file included from ./include/linux/ip.h:16:
> > In file included from ./include/linux/skbuff.h:28:
> > In file included from ./include/net/checksum.h:22:
> > ./arch/mips/include/asm/checksum.h:161:9: error: unsupported inline asm: input with type 'unsigned long' matching output with type '__wsum' (aka 'unsigned int')
> >         : "0" ((__force unsigned long)daddr),
> >                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> > 
> > This is a known issue on MIPS [1], the changed code can be compiled
> > successfully by both GCC and Clang.
> > 
> > [1] https://lore.kernel.org/linux-mips/CAG_fn=W0JHf8QyUX==+rQMp8PoULHrsQCa9Htffws31ga8k-iw@mail.gmail.com/
> > 
> > Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> > ---
> >  arch/mips/include/asm/checksum.h | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> applied to mips-next.

 This is in a performance-critical path (otherwise it wouldn't have been 
in the form of inline assembly).  Has it been verified that it does not 
regress code quality with GCC?

 The semantics is clear here: output is in the same register as in input, 
but the register holds a different local variable in each case.  There's 
nothing odd about that and the variables can obviously be of a different 
type each; that's no different to register usage with code produced by the 
compiler directly itself from a high-level language.

 I seem to remember discussing the issue before, but I can't remember what 
the outcome has been WRT filing this as a Clang bug, and archives are not 
easily available at the moment (I know a mirror exists, but any old links 
are not relevant there).  Would someone be able to fill me in?

 I think ultimately with any critical piece where a Clang workaround does 
regress code produced with GCC we do want to go with `#ifdef __clang__' so 
that good use with GCC is not penalised on one hand and we know the places 
to revert changes at should Clang ever get fixed.

 Otherwise I'll start suspecting that Clang supporters try some kind of an 
unfair game to gain advantage over GCC, by modifying projects such that 
the competing compiler produces worse code than it could if Clang was not 
actively supported.

  Maciej