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

Consolidate byte-swapping paths to avoid undefined behavior [rt.cpan.org #130493] #7

Closed
toddr opened this issue Oct 5, 2020 · 8 comments

Comments

@toddr
Copy link
Collaborator

toddr commented Oct 5, 2020

Migrated from rt.cpan.org#130493 (status was 'new')

Requestors:

From [email protected] on 2019-09-11 17:15:22
:

Pull request containing fix is at https://github.com/gisle/digest-md5/pull/16

The code guarded by #ifndef U32_ALIGNMENT_REQUIRED attempts to optimize
byte-swapping by doing unaligned loads, but accessing data through
unaligned pointers is undefined behavior in C. Moreover, compilers are
more than capable of recognizing these open-coded byte-swap patterns and
emitting a bswap instruction, or an unaligned load instruction, or a
combined load, etc. There's no need for multiple paths to attain the
desired result.

See https://rt.perl.org/Ticket/Display.html?id=133495


@toddr
Copy link
Collaborator Author

toddr commented Oct 5, 2020

which became Perl/perl5#16680

@toddr
Copy link
Collaborator Author

toddr commented Oct 5, 2020

and also Perl/perl5#17244

@toddr
Copy link
Collaborator Author

toddr commented Oct 5, 2020

@mattst88 at this point I'm unclear if anything is needed here or if the problem was with blead not Digest-MD5?

@mattst88
Copy link

mattst88 commented Oct 5, 2020

I think there is nothing else to do here for the dual-life version. In fact, Digest-MD5 actually became dual-life because it was impossible to get fixes into the dead Digest-MD5 project. The solution was to make it dual-life and to fix it there.

@toddr
Copy link
Collaborator Author

toddr commented Oct 5, 2020

Digest-MD5 is still in cpan/ and this is the place to fix stuff.

@mattst88
Copy link

mattst88 commented Oct 5, 2020

Okay, good luck trying. I got an enormous amount of run-around trying to do exactly that before the original Digest-MD5 maintainer finally responded to a private email and told me he wasn't maintaining it anymore.

@toddr
Copy link
Collaborator Author

toddr commented Oct 5, 2020

It is being maintained here by me. I am looking at Perl/perl5@ee9ac1c now and trying to determine if it is backward compatible with previous perls.

@mattst88
Copy link

mattst88 commented Oct 5, 2020

Yes, I believe it is backwards compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants