[Mesa-stable] [Mesa-dev] [PATCH 1/3] util: Fix SHA1 implementation on big endian

[Andres Gomez]

Matt, as Jon and Ilia have reported, this is now breaking with GCC and
MSVC.

You can check the Appveyor error at:
https://ci.appveyor.com/project/mesa3d/mesa/build/6245

Also, for Travis-CI:
https://travis-ci.org/Igalia/mesa/builds/307262963

Although the series landed without the mesa-stable tag, I assume that
you wanted to keep the nomination. If that's the case, remember to land
them with the tag in the future since, if it is not included, it is
automatically understood that the nomination is withdrawn [1].

I suppose it is not the case for this series but we enter in the realm
of ambiguity so tagging is important.

In any case, due to the breakage, I'm leaving this series of 4 patches
out of the 17.2.6 release I'm going to perform just now.

As said, 17.2.7 is targeted for the 8th of December [2] so let's try to
get this fix in good shape for then. It's not so far in the future.

[1] https://www.mesa3d.org/submittingpatches.html#nominations
[2] https://www.mesa3d.org/release-calendar.html


On Fri, 2017-11-24 at 20:24 -0800, Matt Turner wrote:
> On Fri, Nov 24, 2017 at 2:25 AM, Eric Engestrom
> <eric.engestrom at imgtec.com> wrote:
> > On Thursday, 2017-11-23 11:08:04 -0800, Matt Turner wrote:
> > > The code defines a macro blk0(i) based on the preprocessor condition
> > > BYTE_ORDER == LITTLE_ENDIAN. If true, blk0(i) is defined as a byte swap
> > > operation. Unfortunately, if the preprocessor macros used in the test
> > > are no defined, then the comparison becomes 0 == 0 and it evaluates as
> > > true.
> > > ---
> > >  src/util/sha1/sha1.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/src/util/sha1/sha1.c b/src/util/sha1/sha1.c
> > > index ef59ea1dfc..2c629520c3 100644
> > > --- a/src/util/sha1/sha1.c
> > > +++ b/src/util/sha1/sha1.c
> > > @@ -16,8 +16,17 @@
> > > 
> > >  #include <stdint.h>
> > >  #include <string.h>
> > > +#include "u_endian.h"
> > 
> > If you're including this header, why not use
> > `#ifdef PIPE_ARCH_LITTLE_ENDIAN` instead of
> > `#if BYTE_ORDER == LITTLE_ENDIAN`?
> > 
> > `#error` becomes unnecessary
> 
> In response to Andres' report that the Windows build fails with this
> patch, I was going to take your suggestion... but it doesn't seem like
> u_endian.h has any code to handle Windows.
> 
> I see this crap in p_config.h:
> 
> /*
>  * Endian detection.
>  */
> 
> #include "util/u_endian.h"
> #if !defined(PIPE_ARCH_LITTLE_ENDIAN) && !defined(PIPE_ARCH_BIG_ENDIAN)
> 
> #if defined(PIPE_ARCH_X86) || defined(PIPE_ARCH_X86_64) ||
> defined(PIPE_ARCH_ARM) || defined(PIPE_ARCH_AARCH64)
> #define PIPE_ARCH_LITTLE_ENDIAN
> #elif defined(PIPE_ARCH_PPC) || defined(PIPE_ARCH_PPC_64) ||
> defined(PIPE_ARCH_S390)
> #define PIPE_ARCH_BIG_ENDIAN
> #endif
> 
> #endif
> 
> #if !defined(PIPE_ARCH_LITTLE_ENDIAN) && !defined(PIPE_ARCH_BIG_ENDIAN)
> #error Unknown Endianness
> #endif
> 
> I don't think *any* of that would be necessary if we just handled
> Windows (in the appropriate place -- u_endian.h)!
> 
> José, Roland: I think I'm going to commit a patch to u_endian.h to
> define PIPE_ARCH_LITTLE_ENDIAN in the absense of any platform-specific
> handling, so that the rest of my series can land:
> 
> diff --git a/src/util/u_endian.h b/src/util/u_endian.h
> index 7bbd7dc215..3d5c006f35 100644
> --- a/src/util/u_endian.h
> +++ b/src/util/u_endian.h
> @@ -67,4 +67,7 @@
> 
>  #endif
> 
> +#warn Unknown Endianness for this platform. Assuming little endian
> +#define PIPE_ARCH_LITTLE_ENDIAN
> +
>  #endif
> 
> Presumably Windows only runs on little endian platforms, so that
> should be fine for your purposes.
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-- 
Br,

Andres