BLAKE3 hash support #6358
BLAKE3 hash support #6358
Conversation
blake3 official "C optimized implementation" version 0.3.7, obtained with "helper_script_blake3_version_fetcher.php"..
haven't figured out how to add the SSE2/SSE4.1/AVX2/AVX512/Arm+Neon optimized versions when applicable.. and other issues too, like PHP_BLAKE3_SPEC going to ask for help upstream
|
Test failures are legit |
untested.. guess the bots at github will tell me
|
@Girgias thanks, i tried adding blake3 to the msvc config (portable c version like on m4), but i don't have a msvc system to test on so no idea if it was done correctly or not ( 9acf958 ) and i added blake3 to the tests so they pass now ( 9e5e9ef ), that leaves ... maybe the serialize issues has to do with the thing? also when i try running the code i just get did i mess up HashContext serialization / md2 somehow? |
|
yup.. i somehow messed up not sure how i did that tho, unless the problem is ? |
|
I'm not familiar with the Hash Extension but have you looked at the implementation of |
|
hmm... thinking out loud here, typedef struct {
uint32_t cv[8];
uint64_t chunk_counter;
uint8_t buf[BLAKE3_BLOCK_LEN];
uint8_t buf_len;
uint8_t blocks_compressed;
uint8_t flags;
} blake3_chunk_state;
typedef struct {
uint32_t key[8];
blake3_chunk_state chunk;
uint8_t cv_stack_len;
// The stack size is MAX_DEPTH + 1 because we do lazy merging. For example,
// with 7 chunks, we have 3 entries in the stack. Adding an 8th chunk
// requires a 4th entry, rather than merging everything down to 1, because we
// don't know whether more input is coming. This is different from how the
// reference implementation does things.
uint8_t cv_stack[(BLAKE3_MAX_DEPTH + 1) * BLAKE3_OUT_LEN];
} blake3_hasher;i basically get .. i guess i need to figure out how to say that to /* Serialize a hash context according to a `spec` string.
Spec contents:
b[COUNT] -- serialize COUNT bytes
s[COUNT] -- serialize COUNT 16-bit integers
l[COUNT] -- serialize COUNT 32-bit integers
q[COUNT] -- serialize COUNT 64-bit integers
i[COUNT] -- serialize COUNT `int`s
B[COUNT] -- skip COUNT bytes
S[COUNT], L[COUNT], etc. -- uppercase versions skip instead of read
. (must be last character) -- assert that the hash context has exactly
this size
Example: "llllllb64l16." is the spec for an MD5 context: 6 32-bit
integers, followed by 64 bytes, then 16 32-bit integers, and that's
exactly the size of the context.
The serialization result is an array. Each integer is serialized as a
32-bit integer, except that a run of 2 or more bytes is encoded as a
string, and each 64-bit integer is serialized as two 32-bit integers, least
significant bits first. This allows 32-bit and 64-bit architectures to
interchange serialized HashContexts. */
PHP_HASH_API int php_hash_serialize_spec(const php_hashcontext_object *hash, zval *zv, const char *spec) /* {{{ */
{which i guess means the correct SPEC string would be... .. i guess? edit: realized im not supposed to use [], so |
|
hmm, with i'm getting so.. idk, i'm stumped
edit: if (*spec == '.' && align_to(pos, max_alignment) != hash->ops->context_size) {
return FAILURE;
}and adding some debugging reveals -> Fatal error: Uncaught Error: ggg2 1904 , 1912 in /home/hans/projects/php-src/test3.php:5 so.. seems that for some reason my PHP_BLAKE3_SPEC is 8 bytes off... also 8 bytes happens to be sizeof(uintptr_t) , not sure if that's related or not.. |
|
got it! don't know how i came up with the original edit: well something's still wrong on MSVC, guess there's something wrong with ext/hash/config.w32 .. i don't have a MSVC system to test on though, any suggestions? |
i don't have a MSVC to test on, so i'm basically using the github bots to test for me..
|
yay! fixed the MSVC compilation issue, and all built-in tests seem to pass, and the following test script outputs 100_000 dots, which is a good sign (both hash and context and clone(context) and unserialize(serialize(context)) generate the correct result for the first 100_000 zeroes ) <?php
declare(strict_types=1);
$zeroes = "";
for ($i = 0; $i < 100_000; ++$i) {
$answer = shell_exec("head --bytes={$i} /dev/zero | b3sum --raw");
$php_hash = hash("blake3", $zeroes, true);
$php_hash_hc = hash_init("blake3");
hash_update($php_hash_hc, $zeroes);
$php_hash_hc_cloned = hash_final(clone ($php_hash_hc), true);
$php_hash_hc_serialized = hash_final(unserialize(serialize($php_hash_hc)), true);
$php_hash_hc = hash_final($php_hash_hc, true);
if ($answer !== $php_hash) {
var_dump("FAILED php_hash", $i, $answer, $php_hash);
die();
}
if ($answer !== $php_hash_hc) {
var_dump("FAILED php_hash_hc", $i, $answer, $php_hash_hc);
die();
}
if ($answer !== $php_hash_hc_serialized) {
var_dump("FAILED php_hash_hc_serialized", $i, $answer, $php_hash_hc_serialized);
die();
}
if ($answer !== $php_hash_hc_cloned) {
var_dump("FAILED php_hash_hc_cloned", $i, $answer, $php_hash_hc_cloned);
die();
}
$zeroes .= "\x00";
echo ".";
}... that still leaves the question "how to detect if you're compiling for windows or linux" and "how to detect if you're compiling for x86_64 or something else" in m4 syntax, however.. (after figuring that out, the SSE2/SSE4.1/AVX2/AVX512 versions can be added for x86_64 builds) edit: maybe the PR at #6361 contains some of the answers |
when applicable, on gnuc (don't know how to do this on msvc though.. guess i'll ask if anyone on the windows php mailing list want to fix it) there's also a *theoretical* situation that can be optimized, when we're targeting x86_64 but neither windows nor a unix-ish system.. at that point i opted to just disable the blake asm, i have nothing to test it on, but ideally we should use the ext/hash/blake3/blake3_avx512.c & co files in this situation (i cba looking into that)
|
i think i figured it out, blake3 avx512/avx2/sse4.1/sse2 is now enabled for: and as expected, blake3 is significantly faster than every other cryptographically-secure hash on the list (and even some non-crypto hashes! like the fnv family),
edit: it broke 32bit builds, hmm, looking into that edit: fixed the 32bit builds. |
.. waiting for the github bots to tell me if i succeeded or not
|
ok i think i fixed most of the initial problems now, but this question remains: i think is it important to fix that before it could be considered for upstream merging, or is it ok-ish that it's using it's own detection scheme? |
AVX512/AVX2/SSE4.1/SSE2 optimized blake3 implementations for MSVC. ... not that i know how to make MSVC use them, i'm going to ask the windows php mailing list for help about that (i don't even have a MSVC system to test on) these files should have been imported in previous commit "blake3 C code import" / 75a7e5c , but was omitted by mistake
|
Nice work, thanks! But from the repo's README:
IMHO, that's an unfortunate way to ship code. If we bundle that Blake3 implementation, we have to maintain it somehow, and it seems that are currently ~30,000 lines of code – not few of those in assembly. |
|
@cmb69 yeah.. one possible option would be to just use the "portable C" version, which would leave us with roughly 5 files to maintain, instead of ~30+ files (at the cost of performance, obviously) another possible option would be to only use the "c intrinsics" versions, which would allow us to delete all the assembly files (eg use .. if some edit: thought of a possible 4th option: php can just ship the portable version (~5 files), and have an optional |
this time i'm only importing the portable version.. this will result in decreased performance, obiously.. if i'm importing the assembly optimized versions, it will be some ~30,000 lines ( see php#6358 )
this is a significantly smaller alternative to the PR at php#6358 , now blake3 "portable c" version is bundled (specifically version 0.3.7 plus a few patches that will be part of the 0.3.8 release..), and ./configure supports a new optional --with-blake3-upstream-c-source-dir=DIR argument for specifying the location of the upstream BLAKE3 C implementation, if invoked, the SSE2/SSE4.1/AVX2/AVX512 optimized versions of BLAKE3 will be compiled in when applicable (this has not been added to MSVC, i don't know how to do it on MSVC, and i don't have a MSVC system to figure it out out on, if someone think getting those optimizations available on MSVC is important, take it up with the windows php mailing list.. just getting the portable version to compile on MSVC was good enough for me.) also userland scripts can detect at runtime if the portable version or the upstream version, of BLAKE was compiled by consulting phpinfo(), it will either say "blake3 implementation: portable 0.3.7" or "blake3 implementation: upstream X.X.X"
|
I think we can bundle if needed, but have this clearly documented with:
|
|
btw have an alternative PR for BLAKE3 support here #6393
well i wrote a little script to fetch the code, https://github.com/php/php-src/blob/b79adc48dc42df22296f2e77b4de1c215771bdac/ext/hash/blake3/helper_script_blake3_version_fetcher.php |
👍 , then just add a CI test - diff if files are 1:1 with specific release version, eg. were not updated manually by a mistake - and higher SLOC should be no issue |
We do not do this for any other bundled library so I do not see that as a hindrance for potentially including BLAKE3 support with PHP. Or at least have that as a separate thing to do later if you want to ensure that is the case for other extensions, like PCRE2, SQLite3, ... |
|
I like the idea of having Blake3 hashing implemented. Maybe think about file hashing, too. md5_file() and sha1_file() are really outdated. Better than the need to call b3sum on the command line: |
|
@Gauss23 this PR adds blake3 to the php hash-extension, which means blake3 is also available in the hash_file function, php -r 'echo hash_file("blake3","/dev/null");' |
|
I'm looking forward to have this on PHP 8.1 Blake3 is an awesome substitute for the mainstream "file hash when downloading files". Actually, I tried to make a Blake3 PHP extension here (https://github.com/cypherbits/php-blake3) and the hash is working but the hmac password is not... It's my first time making an extension and writing C. |
|
Should be possible to have this for PHP 8.1?? @divinity76 Could you make an extension so people can use this easily right now? (I started making one but I don't know if it is good and fails with hmac) |
right now this PR is just rotting/slowly-collecting-merge-conflicts, not sure what's needed to really get this merged upstream/8.next =/ maybe try asking the php internals mailing list?
no, but your extension looks like a good start, wonder how much extra it would take to hook into the hash('blake3',...) api on your extension |
Yes, that's the way forward. If there is no response, consider to pursue the RFC process. |
Have you asked on the internals list and started the RFC process yet? If you'd like to get this or #6393 accepted for 8.1, now is the time to get the RFC into the discussion phase. I recommend presenting both options in the same RFC. |
no, i'm having a hard time and have no energy for non-essentials. the only way it's going into 8.1 is if someone else pushes for it. that said, i expect to be jobless early August, then I'll have a lot of time. |
|
There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken. |
|
I belive this PR is valuable and should be not closed. |
|
@mvorisek it needed some maintenance and someone to actively push the issue, i'm not up for it currently. also there's been a version 1.x release of the upstream blake library that should be used instead of this one (this was based on a 0.x-something of blake c lib iirc) |
|
I really want this upstream... Meanwhile I have my extension... |
|
new attempt for 8.4: #13194 |
this is not ready for merging (but i hope it will become eventually)
this is a plea for help.
i want BLAKE3 hash support in php, some justifications can be found here https://bugs.php.net/bug.php?id=79492 (tl;dr: CS-secure hash, predecessor BLAKE was nearly crowned "SHA3" (SHA3 finalist), and it's very fast in software)
i have no idea how to add a hash to php, never done it before, just trying to guess / mimic the other hashes.
i have not implemented advanced-ish features of BLAKE3 (eg XOF or keyed hashing), and i have no plans to do that either. (at least not until php's generic hash api adds support)
the code works right now, eg
but does not generate optimally performant code, i'm just (barely) being able to compile it using a generic portable C-implementation, while blake3 has optimized versions for x86's SSE2/SSE4.1/AVX2/AVX512 instructions, and ARM's Neon instructions,- edit: SSE2/SSE4.1/AVX2/AVX512 has now been added for gcc+(windows|unix)+x86_64so..
ext/hash/blake3/blake3_dispatch.c could probably be modified to use Zend/zend_cpuinfo.c instead of its own cpu detection scheme, is that important?
inext/hash/config.m4of this branch isthis is far from ideal, when it's x86,blake3_sse2.cshould be compiled with-msse2, andblake3_sse41.cshould be compiled with-msse4.1andblake3_avx2.cshould be compiled with-mavx2andblake3_avx512.cshould be compiled with-mavx512f -mavx512vl, and none of them should be compiled with
-DBLAKE3_NO_SSE2 -DBLAKE3_NO_SSE41 -DBLAKE3_NO_AVX2 -DBLAKE3_NO_AVX512(alternatively, the S files can be used, eg
blake3_sse2_x86-64_unix.Sandblake3_sse2_x86-64_windows_gnu.Sandblake3_sse2_x86-64_windows_msvc.asmetc, they generate even faster code, and compile faster, but deciding which of them to include is even more difficult, "x86 or x86_64? unix or windows? gcc or msvc?" )but.. i have no idea how to tell that to m4. any help would be apricatededit: fixed for the most part ( but not ARM+NEON..)
in ext/hash/php_hash_blake3.h is.. that definitely isn't right, but i don't know what it should be.edit: changed it to
b8b8qb64bbbbb1760, i think that's right?any help would be apricated