BLAKE3-team / BLAKE3

the official Rust and C implementations of the BLAKE3 cryptographic hash function
Apache License 2.0
5.02k stars 341 forks source link

blake3 compiler warnings #55

Open dlegaultbbry opened 4 years ago

dlegaultbbry commented 4 years ago

Hello,

I see these warnings when using the new code and compiling for arm/aarch64 (I'm porting blake3 to openssl locally).

../../dist/crypto/blake3/blake3.c:264:8: warning: no previous prototype for 'blake3_compress_subtree_wide' [-Wmissing-prototypes] size_t blake3_compress_subtree_wide(const uint8_t input, size_t input_len, ^~~~~~~~ ../../dist/crypto/blake3/blake3_dispatch.c:292:8: warning: no previous prototype for 'blake3_simd_degree' [-Wmissing-prototypes] size_t blake3_simd_degree() { ^~~~~~ ../../dist/crypto/blake3/blake3_dispatch.c:153:5: warning: 'get_cpu_features' defined but not used [-Wunused-function] get_cpu_features() { ^~~~ ../../dist/crypto/blake3/blake3_portable.c:97:6: warning: no previous prototype for 'blake3_compress_in_place_portable' [-Wmissing-prototypes] void blake3_compress_in_place_portable(uint32_t cv[8], ^~~~~~~~~ ../../dist/crypto/blake3/blake3_portable.c:113:6: warning: no previous prototype for 'blake3_compress_xof_portable' [-Wmissing-prototypes] void blake3_compress_xof_portable(const uint32_t cv[8], ^~~~~~~~ ../../dist/crypto/blake3/blake3_portable.c:158:6: warning: no previous prototype for 'blake3_hash_many_portable' [-Wmissing-prototypes] void blake3_hash_many_portable(const uint8_t const inputs, size_t num_inputs, ^~~~~~~~~ In function 'compress_parents_parallel', inlined from 'compress_subtree_to_parent_node' at ../../dist/crypto/blake3/blake3.c:349:9, inlined from 'blake3_hasher_update.part.0' at ../../dist/crypto/blake3/blake3.c:522:7: ../../dist/crypto/blake3/blake3.c:238:5: warning: 'memcpy' forming offset [33, 64] is out of the bounds [0, 32] of object 'out_array' with type 'uint8_t[32]' {aka 'unsigned char[32]'} [-Warray-bounds] memcpy(&out[parents_array_len BLAKE3_OUT_LEN], ^~~~~~~~~~~~ &child_chaining_values[2 parents_array_len BLAKE3_OUT_LEN],

            BLAKE3_OUT_LEN);

../../dist/crypto/blake3/blake3.c: In function 'blake3_hasher_update.part.0': ../../dist/crypto/blake3/blake3.c:346:11: note: 'out_array' declared here uint8_t out_array[MAX_SIMD_DEGREE_OR_2 BLAKE3_OUT_LEN / 2]; ^~~~~ In function 'compress_subtree_to_parent_node', inlined from 'blake3_hasher_update.part.0' at ../../dist/crypto/blake3/blake3.c:522:7: ../../dist/crypto/blake3/blake3.c:350:5: warning: 'memcpy' forming offset [33, 64] is out of the bounds [0, 32] of object 'out_array' with type 'uint8_t[32]' {aka 'unsigned char[32]'} [-Warray-bounds] memcpy(cv_array, out_array, num_cvs BLAKE3_OUT_LEN); ^~~~~~~~~~~~~ ../../dist/crypto/blake3/blake3.c: In function 'blake3_hasher_update.part.0': ../../dist/crypto/blake3/blake3.c:346:11: note: 'out_array' declared here uint8_t out_array[MAX_SIMD_DEGREE_OR_2 BLAKE3_OUT_LEN / 2]; ^~~~~ In function 'compress_subtree_to_parent_node', inlined from 'blake3_hasher_update.part.0' at ../../dist/crypto/blake3/blake3.c:522:7: ../../dist/crypto/blake3/blake3.c:350:5: warning: 'memcpy' forming offset [33, 64] is out of the bounds [0, 32] of object 'out_array' with type 'uint8_t[32]' {aka 'unsigned char[32]'} [-Warray-bounds] memcpy(cv_array, out_array, num_cvs BLAKE3_OUT_LEN); ^~~~~~~~~~~~~ ../../dist/crypto/blake3/blake3.c: In function 'blake3_hasher_update.part.0': ../../dist/crypto/blake3/blake3.c:346:11: note: 'out_array' declared here uint8_t out_array[MAX_SIMD_DEGREE_OR_2 BLAKE3_OUT_LEN / 2]; ^~~~~ In function 'compress_parents_parallel', inlined from 'compress_subtree_to_parent_node' at ../../dist/crypto/blake3/blake3.c:349:9, inlined from 'blake3_hasher_update.part.0' at ../../dist/crypto/blake3/blake3.c:522:7: ../../dist/crypto/blake3/blake3.c:238:5: warning: 'memcpy' offset 64 is out of the bounds [0, 32] of object 'out_array' with type 'uint8_t[32]' {aka 'unsigned char[32]'} [-Warray-bounds] memcpy(&out[parents_array_len BLAKE3_OUT_LEN], ^~~~~~~~~~~~ &child_chaining_values[2 parents_array_len BLAKE3_OUT_LEN],

            BLAKE3_OUT_LEN);

../../dist/crypto/blake3/blake3.c: In function 'blake3_hasher_update.part.0': ../../dist/crypto/blake3/blake3.c:346:11: note: 'out_array' declared here uint8_t out_array[MAX_SIMD_DEGREE_OR_2 BLAKE3_OUT_LEN / 2]; ^~~~~ In function 'compress_subtree_to_parent_node', inlined from 'blake3_hasher_update.part.0' at ../../dist/crypto/blake3/blake3.c:522:7: ../../dist/crypto/blake3/blake3.c:350:5: warning: 'memcpy' forming offset [33, 96] is out of the bounds [0, 32] of object 'out_array' with type 'uint8_t[32]' {aka 'unsigned char[32]'} [-Warray-bounds] memcpy(cv_array, out_array, num_cvs BLAKE3_OUT_LEN); ^~~~~~~~~~~~~ ../../dist/crypto/blake3/blake3.c: In function 'blake3_hasher_update.part.0': ../../dist/crypto/blake3/blake3.c:346:11: note: 'out_array' declared here uint8_t out_array[MAX_SIMD_DEGREE_OR_2 BLAKE3_OUT_LEN / 2]; ^~~~~ ../../dist/crypto/blake3/blake3_neon.c:231:6: warning: no previous prototype for 'blake3_hash4_neon' [-Wmissing-prototypes] void blake3_hash4_neon(const uint8_t const inputs, size_t blocks, ^~~~~ ../../dist/crypto/blake3/blake3_neon.c:326:6: warning: no previous prototype for 'blake3_hash_many_neon' [-Wmissing-prototypes] void blake3_hash_many_neon(const uint8_t const *inputs, size_t num_inputs, ^~~~~

oconnor663 commented 4 years ago

I'm porting blake3 to openssl locally

Sounds exciting!

'memcpy' forming offset [33, 64] is out of the bounds [0, 32] of object 'out_array'

I have definitely seen that one before, and I have a hard time understanding what's causing it. Is it possible it's a bug in the compiler? The make test C tests turn on ASan, so I don't think we're doing anything grossly unsafe, and the C code is also a direct port of some safe Rust code. Maybe the compiler doesn't understand that the if-statement at the end of compress_parents_parallel can't execute in the case where the function has already totally filled its out array?

dlegaultbbry commented 4 years ago

It was because -DBLAKE3_USE_NEON was not defined for all source files compiled for arm/aarch64 so the array size varied wildly between compilation units. Initially I had it only for the neon file before moving it to the others.

oconnor663 commented 4 years ago

I'm going to reopen this, because I've definitely made that mistake myself, and I want to make sure I fully understand the warning.