.github/workflows/build.yml: run 'make tests'

[krystian-hebel]

No description provided.

[macpijan]

@SergiiDmytruk Can you please take a look?

[SergiiDmytruk]

This is more of a workaround in its current form, because undefined behaviour is still there and requires volatile to make things work, but instead of being caused by a type cast it now comes from mixing accesses to different fields of a union (write one field, read another one). We might be better off just dropping them and doing memcpy() which is always well-defined.

I'll push WIP commit here to show what I mean which also gets rid of another UB caused by unaligned access at the beginning of transformation step. I think we should either make it work without hacks like volatile or keep the code as is and use -fno-strict-aliasing.

Also, a note on use of constants. Highly specific code like this which implements, as opposed to just using, a well-documented algorithm doesn't need them much, because you still need to know their meaning and they are even mentioned in comments (64, 56) alongside sizeof(...) anyway.

[krystian-hebel]

I was under the impression that writing one, reading another field of union is defined, as long as they are accessed directly instead of casting to another types. I thought that the reason it wasn't enough was because sha*_transform() took address and not value of member of union.

I agree that the use of constants may be a bit much. I've noticed that SHA256 already had a constant but didn't really used it, so I added it where it belonged, did the same for SHA1 and went few steps too far 🙂

Anyway, I'll include your WIP commit with few minor changes like not declaring variables in the middle of a function.

[SergiiDmytruk]

I was under the impression that writing one, reading another field of union is defined, as long as they are accessed directly instead of casting to another types.

Hm, actually I'm not so sure now and there are also differences between C and C++ which don't help either. 6.5.2.3 Structure and union members of https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1425.pdf does have:

93. If the member used to access the contents of a union object is not the same as the member last used to
    store a value in the object, the appropriate part of the object representation of the value is reinterpreted
    as an object representation in the new type as described in 6.2.6 (a process sometimes called ‘‘type
    punning’’). This might be a trap representation.

which isn't there in https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf and both have

Note that aggregate type does not include union type because an object with union type can only contain one member at a time.

And in all cases these are footnotes which are non-normative.

I thought that the reason it wasn't enough was because sha*_transform() took address and not value of member of union.

So maybe that's the correct explanation. Either way, memcpy() is a safer choice :)