Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

byte_order.c breaks if byte_order.h uses the bswap_32() fallback macro #53

Closed
Bloody2 opened this issue Jan 10, 2018 · 2 comments
Closed

Comments

@Bloody2
Copy link

Bloody2 commented Jan 10, 2018

Hello everyone,

If byte_order.h is run through clang (in my case, clang++), the file concludes that if all else fails, the fallback macro shall be used:

#define bswap_32(x) ((((x) & 0xff000000) >> 24) | (((x) & 0x00ff0000) >> 8) |
(((x) & 0x0000ff00) << 8) | (((x) & 0x000000ff) << 24))

Problem: byte_order.c contains code like: bswap_32( *(src++) )

The above will accidently execute "*(src++)" multiple times as "x" in the fallback macro (instead of just once).

Also, to check if 32/64-bit bswap intrinsics can be used, tests like the following:

#elif (defined(GNUC) && (GNUC >= 4) && (GNUC > 4 || GNUC_MINOR >= 3))

should be expanded to simply check for clang as well, like:

#elif defined(clang) || (defined(GNUC) && (GNUC >= 4) && (GNUC > 4 || GNUC_MINOR >= 3))

Luckily, you don't need bswap_16 which appeared in later clang versions, while 32/64-bit swappers were added early and can simply be assumed to be present as long as clang is defined.

Other than that, your SHA/SHA3 sources compile & run just fine if compiled as C++ btw.

Great implementation - small, easy, nice & fast. Thank you very much!

@rhash
Copy link
Owner

rhash commented Jan 15, 2018

Hello. The bug was fixed in release v1.3.5, please retest.

@Bloody2
Copy link
Author

Bloody2 commented Jan 18, 2018

Confirmed. Sorry for the alarm, didn't notice that part has meanwhile changed, too.
Just one of those "rare bugs" that sometimes go unnoticed. Sorry again.

@rhash rhash closed this as completed Jan 22, 2018
chewi pushed a commit to chewi/RHash that referenced this issue Mar 18, 2018
@rhash rhash added done The issue is fixed/applied/implemented can't reproduce and removed done The issue is fixed/applied/implemented labels Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants