Anonymous Login
2018-12-13 03:46 UTC

View Issue Details Jump to Notes ]
IDProjectCategoryView StatusLast Update
0002017OpenClonkEngine - C4Scriptpublic2018-04-10 18:56
Reporterpkern 
Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
StatusacknowledgedResolutionopen 
PlatformDebianOSLinuxOS Version 
Product Version8.0 
Target VersionFixed in Version 
Summary0002017: OpenClonk vendors blake2 reference implementation instead of libb2
DescriptionSo with 8.1 OpenClonk inherited a dependency on blake2 code. Apparently there are two different implementations by the same vendor: https://github.com/BLAKE2/BLAKE2/blob/master/ref/blake2.h which was vendored and https://github.com/BLAKE2/libb2/blob/master/src/blake2.h which is already in Debian as a library. Unfortunately the library does not ship with a pkg-config file, which is pretty annoying. But what's even more annoying is that the blake2b signature differs:

Vendored BLAKE2:

int blake2b( void *out, size_t outlen, const void *in, size_t inlen, const void *key, size_t keylen );

libb2:

LAKE2_API int blake2b( uint8_t *out, const void *in, const void *key, size_t outlen, size_t inlen, size_t keylen );

In Debian we attempt not to use vendored code because it's hard to update. So I'm going to link against libb2 instead, with a small patch that shuffles around the arguments. You might want to consider vendoring the other code as well. I honestly don't know what the actual story here is.

The reason I found it was that I was crawling the (short) commit log and saw the differences between 8.0 and 8.1. Unfortunately what caught my eye was an unconditional addition of -msse2 to the compiler flags in case it's available specifically in the BLAKE2 CMake manifest. From a pure technical point of view I agree with you that this makes sense. For portability it is incorrect though as the detection would need to happen at runtime. So for i386 in Debian I'll use with whatever flags the library was compiled with. (x86_64 will always have SSE2 anyway.)
TagsNo tags attached.
Attached Files

-Relationships
+Relationships

-Notes

~0006179

sphalerite (reporter)

+1 to this; it also causes build failure on ARM systems that don't have SSE at all, and I don't think there's necessarily anything else preventing openclonk running from ARM.

~0006180

Isilkor (developer)

The reason we're not using libb2 outright is that we can't (nicely) do it on Windows; it compiles a single source file (blake2b.c) several time with different compile flags, which is something CMake doesn't like to do without serious hacks. I'll add some code to our main CMakeLists.txt checking whether libb2 is available on the system and use that instead of the vendored code.

On the topic of ARM build failures: I believe that should have been fixed in https://github.com/openclonk/openclonk/commit/1dd7cbb04ad89bca3b481c5384f08651b8c20167, where we're using a plain C implementation when BLAKE2B_USE_SSE2 is unset or false, which it is by default if the target platform isn't amd64. (I don't know off the top of my head whether that commit made it into a stable branch/release.)
+Notes

-Issue History
Date Modified Username Field Change
2018-03-18 13:28 pkern New Issue
2018-04-10 09:08 sphalerite Note Added: 0006179
2018-04-10 18:56 Isilkor Status new => acknowledged
2018-04-10 18:56 Isilkor Note Added: 0006180
+Issue History