Skip to content

hash/crc32: wrong output for unaligned input on s390x #16779

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.

Sign up for GitHub

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

Jump to bottom
Closed
randall77 opened this issue Aug 17, 2016 · 6 comments
Closed

hash/crc32: wrong output for unaligned input on s390x #16779

randall77 opened this issue Aug 17, 2016 · 6 comments
Labels
FrozenDueToAge
Milestone
Go1.7.1

Comments

@randall77
Copy link
Contributor

CL https://go-review.googlesource.com/#/c/24470/ introduced a test for unaligned crc32 computations. It seems this broke the s390x implementation.

See https://build.golang.org/log/e03a4804f6a0c93d08194fb85a168701774f729d , the build bot for s390x is failing.

@ChrisXZou
@bradfitz
Copy link
Contributor

s390x assembly problem, uncaught until now? All the other builders look happy.

/cc @mundaym @billotosyr for s390x

@randall77
Copy link
Contributor Author

Yes, I'm assuming this is a s390x assembly bug unless proven otherwise.

@billotosyr
Copy link

We'll have a look at it.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/27409 mentions this issue.

@mundaym
Copy link
Member

mundaym commented Aug 24, 2016

Can this fix be marked for 1.7.1? The CL is simple and should apply cleanly to 1.7.

@bradfitz bradfitz added this to the Go1.7.1 milestone Aug 24, 2016
@bradfitz
Copy link
Contributor

Sure.

gopherbot pushed a commit that referenced this issue Sep 7, 2016
@mundaym @broady
[release-branch.go1.7] hash/crc32: fix optimized s390x implementation

Unverified

This user has not yet uploaded their public signing key.
GPG key ID: ED11605AA22B0C48
Learn about vigilant mode
a5add8c 
The code wasn't checking to see if the data was still >= 64 bytes
long after aligning it.

Aligning the data is an optimization and we don't actually need
to do it. In fact for smaller sizes it slows things down due to
the overhead of calling the generic function. Therefore for now
I have simply removed the alignment stage. I have also added a
check into the assembly to deliberately trigger a segmentation
fault if the data is too short.

Fixes #16779.

Change-Id: Ic01636d775efc5ec97689f050991cee04ce8fe73
Reviewed-on: https://go-review.googlesource.com/27409
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-on: https://go-review.googlesource.com/28635
@golang golang locked and limited conversation to collaborators Aug 24, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.7.1
Development

No branches or pull requests
5 participants
@bradfitz @randall77 @gopherbot @billotosyr @mundaym