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

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

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

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

aclements opened this issue Sep 1, 2017 · 3 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@aclements
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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.)

@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants