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

Create a correctly-sized slice to proxy *uint16 #926

Conversation

TBBle
Copy link
Contributor

@TBBle TBBle commented Jan 16, 2021

Fixes the below issue seen in the containerd test suite.

fatal error: checkptr: converted pointer straddles multiple allocations

Also adds -gcflags=all=-d=checkptr to all the test runs on CI, to avoid this regressing in future. This requires testing with Go 1.14 or newer, so CI now runs on Go 1.15, as Go 1.14 did not recommend using checkptr on Windows.

And Go>1.13 requires the Visual Studio 2019 build image on AppVeyor.

@TBBle TBBle requested a review from a team as a code owner January 16, 2021 05:21
@TBBle TBBle force-pushed the convert-pointer-to-slice-without-failing-checkptr branch 5 times, most recently from b00e2d2 to fe51c0e Compare January 16, 2021 06:02
@TBBle
Copy link
Contributor Author

TBBle commented Jan 16, 2021

If the change in CI stack version is undesirable, I can back that out, once CI passes.

As a less-disruptive alternative we could also roll back to 1.13, and turn on -race or -msan. Then -d=checkptr will be enabled by default when CI is eventually upgraded to Go 1.15.

Fixes the below issue seen in the containerd test suite.
```
fatal error: checkptr: converted pointer straddles multiple allocations
```

Also adds `-gcflags=all=-d=checkptr` to all the test runs on CI, to
avoid this regressing in future. This requires testing with Go 1.14 or
newer, so CI now runs on Go 1.15, as Go 1.14 did not recommend using
checkptr on Windows.

And _that_ requires the Visual Studio 2019 build image on AppVeyor.

Signed-off-by: Paul "TBBle" Hampson <[email protected]>
@TBBle TBBle force-pushed the convert-pointer-to-slice-without-failing-checkptr branch from fe51c0e to 0bb6d5c Compare January 21, 2021 22:04
@TBBle TBBle requested a review from katiewasnothere January 21, 2021 22:08
@katiewasnothere
Copy link
Contributor

Is the use of -gcflags=all=-d=checkptr preferred over using -race or -msan when using golang 1.15?

@TBBle
Copy link
Contributor Author

TBBle commented Jan 21, 2021

I'm not sure if there's a preference there or not. This was the minimal change to reproduce the problem.

Also, I didn't know it was implicit in -race or -msan until after I had it working. -race and -msan require CGO too, so if we want to use those instead, we'd have to reinstate that into the CI pipeline, reverting #749.

@katiewasnothere
Copy link
Contributor

Yeah I'd prefer to not add back cgo so I think this is fine.

Copy link
Contributor

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

3 participants