svn commit: r366344 - in head: lib/libmd sys/crypto/skein/amd64

[Kyle Evans]

On Thu, Oct 1, 2020 at 8:23 PM Cy Schubert <Cy.Schubert at cschubert.com> wrote:
>
> In message <CACNAnaG13rxG-rxzzkQgTazr2_-dy1kpHEmBZjwr=tGnB+oMzQ at mail.gmail.c
> om>
> , Kyle Evans writes:
> > On Thu, Oct 1, 2020 at 4:06 PM Ed Maste <emaste at freebsd.org> wrote:
> > >
> > > Author: emaste
> > > Date: Thu Oct  1 21:05:50 2020
> > > New Revision: 366344
> > > URL: https://svnweb.freebsd.org/changeset/base/366344
> > >
> > > Log:
> > >   libmd: fix assembly optimized skein implementation
> > >
> > >   The assembly implementation incorrectly used logical AND instead of
> > >   bitwise AND. Fix, and re-enable in libmd.
> > >
> > >   Submitted by: Yang Zhong <yzhong at freebsdfoundation.org>
> > >   Reviewed by:  cem (earlier)
> > >   Sponsored by: The FreeBSD Foundation
> > >   Differential Revision:        https://reviews.freebsd.org/D26614
> > >
> > > Modified:
> > >   head/lib/libmd/Makefile
> > >   head/sys/crypto/skein/amd64/skein_block_asm.S
> > >
> > > Modified: head/lib/libmd/Makefile
> > > ===========================================================================
> > ===
> > > --- head/lib/libmd/Makefile     Thu Oct  1 20:08:27 2020        (r366343)
> > > +++ head/lib/libmd/Makefile     Thu Oct  1 21:05:50 2020        (r366344)
> > > @@ -116,12 +116,12 @@ CFLAGS+= -DSHA1_ASM
> > >  SRCS+= rmd160.S
> > >  CFLAGS+= -DRMD160_ASM
> > >  .endif
> > > -#.if exists(${MACHINE_ARCH}/skein_block_asm.S)
> > > -## Fully unroll all loops in the assembly optimized version
> > > -#ACFLAGS+= -DSKEIN_LOOP=0
> > > -#SRCS+= skein_block_asm.S
> > > -#CFLAGS+= -DSKEIN_ASM -DSKEIN_USE_ASM=1792 # list of block functions to re
> > place with assembly: 256+512+1024 = 1792
> > > -#.endif
> > > +.if exists(${MACHINE_ARCH}/skein_block_asm.S)
> > > +# Fully unroll all loops in the assembly optimized version
> > > +ACFLAGS+= -DSKEIN_LOOP=0
> > > +SRCS+= skein_block_asm.S
> > > +CFLAGS+= -DSKEIN_ASM -DSKEIN_USE_ASM=1792 # list of block functions to rep
> > lace with assembly: 256+512+1024 = 1792
> > > +.endif
> > >  .if exists(${MACHINE_ARCH}/sha.S) || exists(${MACHINE_ARCH}/rmd160.S) || e
> > xists(${MACHINE_ARCH}/skein_block_asm.S)
> > >  ACFLAGS+= -DELF -Wa,--noexecstack
> > >  .endif
> > >
> >
> > We need some kind of magic to walk across this for -DNO_CLEAN builds
> > -- skein_block.c has no reason to get rebuilt, but we need it to
> > because we're now defining SKEIN_USE_ASM=1792, which will strip out
> > some symbols.
> >
> > I haven't had time to look into what kind of magic we can apply here,
> > kind of needed to skip ahead to get this build finished for some other
> > testing.
>
> I did rm -r for .../lib/libmd.
>

Yeah, either that or... touch skein_block.c or I guess in
tools/build/depend-cleanup.sh we can basically:

if [ -f ${OBJTOP}/lib/libmd/skein_block_asm.o ]; then
    if nm ${OBJTOP}/lib/libmd/skein_block.o | grep Skein_512_Process_Block; then
        rm ${OBJTOP}/lib/libmd/skein_block.*
    fi
fi

> Strangely it only failed in the amd64 build. Not in i386.
>

The optimization in question actually only applies to amd64, so this
part is reasonable.