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

unsafe: Slice does not panic on nil pointer when element size is 0 #54092

Closed
randall77 opened this issue Jul 27, 2022 · 6 comments
Closed

unsafe: Slice does not panic on nil pointer when element size is 0 #54092

randall77 opened this issue Jul 27, 2022 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@randall77
Copy link
Contributor

This program should panic:

package main

import "unsafe"

func main() {
	var p *struct{}
	_ = unsafe.Slice(p, 5)
}

From the unsafe.Slice docs:

if ptr is nil and len is not zero, a run-time panic occurs.

A zero-sized element is a very special case here, so just a doc fix would be fine also.

@cuonglm @mdempsky

@randall77 randall77 added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 27, 2022
@randall77 randall77 added this to the Go1.20 milestone Jul 27, 2022
@cuonglm
Copy link
Member

cuonglm commented Jul 27, 2022

Isn't it #46742?

@randall77
Copy link
Contributor Author

That issue is about Slice(nil, 0). This one is about Slice(nil, >0). Which correctly panics if the element type has nonzero size. It's just when the element type is zero sized, no panic happens. We get a slice with {ptr:nil, len:5, cap:5}.

@cuonglm
Copy link
Member

cuonglm commented Jul 27, 2022

That issue is about Slice(nil, 0). This one is about Slice(nil, >0). Which correctly panics if the element type has nonzero size. It's just when the element type is zero sized, no panic happens. We get a slice with {ptr:nil, len:5, cap:5}.

Ah right, it does panic with go.17, though https://go.dev/play/p/bpbEmh22qZA?v=goprev

@mdempsky
Copy link
Member

Nice find. It looks like the issue is we rely on size * len > -ptr to detect this, but this fails when size is 0.

We can add an extra explicit ptr == nil && len > 0 check, but that adds an extra branch on a code path we're trying to keep fast.

Maybe we can add it to the 64-bit entry point, and just force use of that function for zero-size types?

@randall77
Copy link
Contributor Author

We could generate entirely different code for the elemsize==0 case.

@gopherbot
Copy link

Change https://go.dev/cl/419755 mentions this issue: cmd/compile,runtime: panic when unsafe.Slice param is nil and > 0

cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Jul 28, 2022
cuiweixie added a commit to cuiweixie/go that referenced this issue Aug 8, 2022
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Fixes golang#54092

Change-Id: Ib917922ed36ee5410e5515f812737203c44f46ae
GitHub-Last-Rev: dfd0c38
GitHub-Pull-Request: golang#54107
Reviewed-on: https://go-review.googlesource.com/c/go/+/419755
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Aug 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants