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

runtime: add mode for pointer-past-end testing #21730

Open
aclements opened this Issue Sep 1, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@aclements
Member

aclements commented Sep 1, 2017

Over time, we've had several issues involving unsafe code constructing pointers just past the end of objects, particularly in runtime and reflect. Some examples are #21717, #19724, and #9384.

These sorts of issues are very hard to write tests for. We should consider adding a testing mode to aid with this. One approach would be for the allocator to skip every other slot and to right-align all allocations (up to alignment limits), so that past-the-end pointers would always fall on a free slot.

This mode could be exposed via a GODEBUG, though for testing it should probably be adjustable at run time, perhaps via unexported runtime functions.

/cc @RLH @cherrymui @bcmills

@bcmills

This comment has been minimized.

Member

bcmills commented Sep 1, 2017

to skip every other slot and to right-align all allocations

We could pair that with a simple check at each unsafe.Pointer conversion: any time we convert an integer to an unsafe.Pointer, we could validate that either the pointer points outside the Go heap, or the pointer points to an allocated object. (Do we do that already?)

We could take that one step further and, upon converting from unsafe.Pointer to a concrete type, validate that its heap map matches the pointer layout for the type to which it was converted.

If that's too expensive, we could consider adding a function to make that check explicitly (e.g., in the runtime/debug package):

// CheckPointers verifies that all of the pointers reachable from p either
// are nil or point to mapped addresses, and that the pointer maps for all
// such addresses that reside in Go memory match their types.
func CheckPointers(p interface{})

Such a function might be useful for cgo sanity checking anyway (#19928).

@aclements

This comment has been minimized.

Member

aclements commented Sep 1, 2017

We could pair that with a simple check at each unsafe.Pointer conversion: any time we convert an integer to an unsafe.Pointer, we could validate that either the pointer points outside the Go heap, or the pointer points to an allocated object. (Do we do that already?)

That could also be a good test. Though this couldn't be enabled on-the-fly, so it would be harder to use in unit tests.

func CheckPointers(p interface{})

I'm not sure this is strong enough to conveniently write past-the-end tests. You may not have a direct pointer to the object containing a bad pointer, which suggests CheckPointers would have to be deep, and now you've got a garbage collector.

@bcmills

This comment has been minimized.

Member

bcmills commented Sep 1, 2017

CheckPointers would have to be deep, and now you've got a garbage collector.

Agreed. I was hoping there might be a similar function in the existing garbage collector that we could generalize. (Some sort of “pointers starting from p visitor” function, perhaps.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment