Re: [RESEND,PATCH] ARM: fix __div64_32() error when compiling with clang

[Nick Desaulniers]

Thanks for the report, it probably was not fun to debug. I'll take a
closer look at this after the Thanksgiving holiday.

On Tue, Nov 24, 2020 at 2:14 AM Antony Yu <swpenim@xxxxxxxxx> wrote:
>
> Antony Yu <swpenim@xxxxxxxxx> 於 2020年11月24日 週二 下午3:43寫道：
> >
> > On Mon, Nov 23, 2020 at 11:16:02AM -0700, Nathan Chancellor wrote:
> > > On Mon, Nov 23, 2020 at 03:36:32PM +0800, Antony Yu wrote:
> > > > __do_div64 clobbers the input register r0 in little endian system.
> > > > According to the inline assembly document, if an input operand is
> > > > modified, it should be tied to a output operand. This patch can
> > > > prevent compilers from reusing r0 register after asm statements.
> > > >
> > > > Signed-off-by: Antony Yu <swpenim@xxxxxxxxx>
> > > > ---
> > > >  arch/arm/include/asm/div64.h | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/arch/arm/include/asm/div64.h b/arch/arm/include/asm/div64.h
> > > > index 898e9c78a7e7..809efc51e90f 100644
> > > > --- a/arch/arm/include/asm/div64.h
> > > > +++ b/arch/arm/include/asm/div64.h
> > > > @@ -39,9 +39,10 @@ static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
> > > >     asm(    __asmeq("%0", __xh)
> > > >             __asmeq("%1", "r2")
> > > >             __asmeq("%2", "r0")
> > > > -           __asmeq("%3", "r4")
> > > > +           __asmeq("%3", "r0")
> > > > +           __asmeq("%4", "r4")
> > > >             "bl     __do_div64"
> > > > -           : "=r" (__rem), "=r" (__res)
> > > > +           : "=r" (__rem), "=r" (__res), "=r" (__n)
> > > >             : "r" (__n), "r" (__base)
> > > >             : "ip", "lr", "cc");
> > > >     *n = __res;
> > > > --
> > > > 2.23.0
> > > >
> > >
> > > I am not sure that I am qualified to review this (my assembly knowledge
> > > is not the best) but your commit title mentions an error when compiling
> > > with clang. What is the exact error, what configuration generates it,
> > > and what version of clang? We have done fairly decent testing for
> > > 32-bit ARM, I would like to know what we are missing.
> > >
> > > Cheers,
> > > Nathan
> >
> > We have run fail on android R vts vts_libsnapshot_test with kernel 4.14.
> > This bug is triggered accidently by a workaround patch in our code base.
> > It is fine on a pure clean 4.14 branch since __do_div64 may not be
> > executed in skip_metadata.
> >
> > The attachment are .i and generated .s file. .s file can be reproduced
> > with clang -target arm-linux-eabi -march=armv8.2-a -O2.
> >
> > In function skip_metadata, it loads some value to r0, calls __do_div64,
> > adds 1 to r0 and stores it to [r5]. It gets wrong value since __do_div64
> > clobbers r0 register.
> >
> > We have tried clang-10, clang-11 and android prebuilt clang-r383902b. All
> > of them have the same problem.
>
> Sorry for the large attachment.
> I put .i and .s files on
> https://gist.github.com/penyung/274b0c697062a1c776994bb40243cfff
>
> Antony Yu



-- 
Thanks,
~Nick Desaulniers