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.

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

x/image: draw.Copy causes stack overflow when dr and sr have same size and DstMask is not nil #23107

Closed
jiulongw opened this issue Dec 12, 2017 · 1 comment
Closed

x/image: draw.Copy causes stack overflow when dr and sr have same size and DstMask is not nil #23107

jiulongw opened this issue Dec 12, 2017 · 1 comment
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@jiulongw
Copy link

jiulongw commented Dec 12, 2017 •
edited

There is a shortcut in Scale function that calls Copy when dst rect has same size with src rect.

func (z nnInterpolator) Scale(dst Image, dr image.Rectangle, src image.Image, sr image.Rectangle, op Op, opts *Options) {
	// Try to simplify a Scale to a Copy.
	if dr.Size() == sr.Size() {
		Copy(dst, dr.Min, src, sr, op, opts)
		return
	}
...
}

However the Copy function calls Scale when DstMask is defined:

func Copy(dst Image, dp image.Point, src image.Image, sr image.Rectangle, op Op, opts *Options) {
	var o Options
	if opts != nil {
		o = *opts
	}
	dr := sr.Add(dp.Sub(sr.Min))
	if o.DstMask == nil {
		DrawMask(dst, dr, src, sr.Min, o.SrcMask, o.SrcMaskP.Add(sr.Min), op)
	} else {
		NearestNeighbor.Scale(dst, dr, src, sr, op, opts)
	}
}

Thus causes stack overflow. I'm going to send a code review for this.

What version of Go are you using (go version)?

go version go1.9.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/jiulongw/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9.2/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bv/5bgcs_qj2557dnm8yq7b2t6m0000gn/T/go-build063560038=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

With following test code added to golang.org/x/image/draw:

func TestDstMaskSameSizeCopy(t *testing.T) {
	bounds := image.Rect(0, 0, 42, 42)
	src := image.Opaque
	dst := image.NewRGBA(bounds)
	mask := image.NewRGBA(bounds)

	Copy(dst, image.ZP, src, bounds, Src, &Options{
		DstMask: mask,
	})
}

Running go test results:

draw (master) $ go test
runtime: goroutine stack exceeds 1000000000-byte limit
fatal error: stack overflow

runtime stack:
runtime.throw(0x11a9f02, 0xe)
	/usr/local/Cellar/go/1.9.2/libexec/src/runtime/panic.go:605 +0x95
runtime.newstack(0x0)
	/usr/local/Cellar/go/1.9.2/libexec/src/runtime/stack.go:1050 +0x6e1
runtime.morestack()
	/usr/local/Cellar/go/1.9.2/libexec/src/runtime/asm_amd64.s:415 +0x86

goroutine 36 [running]:
golang.org/x/image/draw.nnInterpolator.Scale(0x126c140, 0xc420420180, 0x0, 0x0, 0x2a, 0x2a, 0x126bbc0, 0xc42000e420, 0x0, 0x0, ...)
	/Users/jiulongw/go/src/golang.org/x/image/draw/impl.go:13 +0x1301 fp=0xc44022c360 sp=0xc44022c358 pc=0x1119811
golang.org/x/image/draw.(*nnInterpolator).Scale(0x12a0e40, 0x126c140, 0xc420420180, 0x0, 0x0, 0x2a, 0x2a, 0x126bbc0, 0xc42000e420, 0x0, ...)
	<autogenerated>:1 +0xf9 fp=0xc44022c3e0 sp=0xc44022c360 pc=0x1152729
golang.org/x/image/draw.Copy(0x126c140, 0xc420420180, 0x0, 0x0, 0x126bbc0, 0xc42000e420, 0x0, 0x0, 0x2a, 0x2a, ...)
	/Users/jiulongw/go/src/golang.org/x/image/draw/scale.go:30 +0x14f fp=0xc44022c4a8 sp=0xc44022c3e0 pc=0x11431bf
golang.org/x/image/draw.nnInterpolator.Scale(0x126c140, 0xc420420180, 0x0, 0x0, 0x2a, 0x2a, 0x126bbc0, 0xc42000e420, 0x0, 0x0, ...)
	/Users/jiulongw/go/src/golang.org/x/image/draw/impl.go:16 +0x12ec fp=0xc44022c5a0 sp=0xc44022c4a8 pc=0x11197fc
...
@gopherbot gopherbot added this to the Unreleased milestone Dec 12, 2017
@gopherbot
Copy link

Change https://golang.org/cl/83537 mentions this issue: x/image: Fix crash caused by Scale by Copy shortcut

@golang golang locked and limited conversation to collaborators Dec 14, 2018
mrhyperbit23z0d added a commit to mrhyperbit23z0d/bhegde8 that referenced this issue Jun 6, 2022
@mrhyperbit23z0d
draw: fix crash caused by Scale by Copy shortcut
b9449d3 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
GalaxyForcew added a commit to GalaxyForcew/A1bisshy that referenced this issue Jun 6, 2022
@GalaxyForcew
draw: fix crash caused by Scale by Copy shortcut
c4f20e8 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
yi-ge3 added a commit to yi-ge3/wislie that referenced this issue Jun 6, 2022
@yi-ge3
draw: fix crash caused by Scale by Copy shortcut
61aa34a 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
balloontmz6 added a commit to balloontmz6/Likewise42l that referenced this issue Jun 6, 2022
@balloontmz6
draw: fix crash caused by Scale by Copy shortcut
31a1d7a 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
snapbakkhfbav added a commit to snapbakkhfbav/SayedBaladohr that referenced this issue Oct 6, 2022
@snapbakkhfbav
draw: fix crash caused by Scale by Copy shortcut
3efbfcc 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
MiderWong5ddop added a commit to MiderWong5ddop/sidie88f that referenced this issue Oct 7, 2022
@MiderWong5ddop
draw: fix crash caused by Scale by Copy shortcut
ca6029e 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
rorypeckwnt4v added a commit to rorypeckwnt4v/LearnByBhanuPrataph that referenced this issue Oct 7, 2022
@rorypeckwnt4v
draw: fix crash caused by Scale by Copy shortcut
8f44a78 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
egorovcharenko9 added a commit to egorovcharenko9/RiceBIOC470z that referenced this issue Oct 7, 2022
@egorovcharenko9
draw: fix crash caused by Scale by Copy shortcut
9939254 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
RafayGhafoorf added a commit to RafayGhafoorf/dustinsand8 that referenced this issue Oct 7, 2022
@RafayGhafoorf
draw: fix crash caused by Scale by Copy shortcut
bad9395 
When DstMask is not nil, this shortcut causes stack overflow because
Copy function in turn will call Scale with same dr and sr.

Fixes golang/go#23107

Change-Id: I8ccadbd9b7f16363ac17b6114308527d6fa9456e
Reviewed-on: https://go-review.googlesource.com/83537
Reviewed-by: Nigel Tao <nigeltao@golang.org>
Run-TryBot: Nigel Tao <nigeltao@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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
Unreleased
Development

No branches or pull requests
3 participants
@jiulongw @nigeltao @gopherbot