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

cmd/go: add test to ensure the correct line number is displayed when using TB.Helper with race detector #26995

Closed
maxatome opened this issue Aug 15, 2018 · 5 comments
Closed

cmd/go: add test to ensure the correct line number is displayed when using TB.Helper with race detector #26995

maxatome opened this issue Aug 15, 2018 · 5 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Unplanned

Comments

@maxatome
Copy link

Similar problem referenced in #21631

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

go version go1.10.3 freebsd/amd64

go version go1.11rc1 freebsd/amd64

Does this issue reproduce with the latest release?

yes, but a little differently

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/max/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="freebsd"
GOOS="freebsd"
GOPATH="/home/max/Projet/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/freebsd_amd64"
GCCGO="gccgo"
CC="clang"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build991380835=/tmp/go-build -gno-record-gcc-switches"

What did you do?

testing_test.go:

package testing_test

import "testing"

type TestingT interface {
	Helper()
	Log(args ...interface{})
}

func directCall(t *testing.T) {
	t.Helper()
	t.Log("directCall")
}

func interfaceTBCall(t testing.TB) {
	t.Helper()
	t.Log("interfaceTBCall")
}

func interfaceCall(t TestingT) {
	t.Helper()
	t.Log("interfaceCall")
}

func TestTesting(t *testing.T) {
	directCall(t)
	interfaceTBCall(t)
	interfaceCall(t)
}

Then

go test -v -race

What did you expect to see?

=== RUN   TestTesting
--- PASS: TestTesting (0.00s)
	testing_test.go:26: directCall
	testing_test.go:27: interfaceTBCall
	testing_test.go:28: interfaceCall
PASS
ok  	_/home/max	1.035s

(in fact the same output as without the -race flag.)

What did you see instead?

With go1.10.3:

=== RUN   TestTesting
--- PASS: TestTesting (0.00s)
	testing_test.go:26: directCall
	testing_test.go:17: interfaceTBCall
	testing_test.go:22: interfaceCall
PASS
ok  	_/home/max	1.035s

With go1.11rc1:

=== RUN   TestTesting
--- PASS: TestTesting (0.00s)
    testing_test.go:26: directCall
    <autogenerated>:1: interfaceTBCall
    <autogenerated>:1: interfaceCall
PASS
ok  	_/home/max	1.074s
@andybons andybons changed the title testing: with race detector on, using TB.Helper yields the wrong line number cmd/go: with race detector on, using TB.Helper yields the wrong line number Aug 15, 2018
@andybons
Copy link
Member

@rsc @bcmills @ianlancetaylor @dvyukov

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 15, 2018
@andybons andybons added this to the Unplanned milestone Aug 15, 2018
maxatome added a commit to maxatome/go-testdeep that referenced this issue Aug 17, 2018
@maxatome
Work around golang/go#26995 issue
a80b4c8 
Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
maxatome added a commit to maxatome/go-testdeep that referenced this issue Aug 17, 2018
@maxatome
Work around golang/go#26995 issue
ba65ada 
Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
maxatome added a commit to maxatome/go-testdeep that referenced this issue Aug 17, 2018
@maxatome
Work around golang/go#26995 issue
8d9ddd2 
Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
maxatome added a commit to maxatome/go-testdeep that referenced this issue Aug 18, 2018
@maxatome
Work around golang/go#26995 issue
bd54d82 
Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
@maxatome
Copy link
Author

maxatome commented Jan 5, 2019

@andybons just tested with go1.12beta1 and still the same:

> go version   
go version devel +e3b4b7baad Tue Dec 18 23:01:06 2018 +0000 freebsd/amd64
> go test -v -race   
=== RUN   TestTesting
--- PASS: TestTesting (0.00s)
    testing_test.go:26: directCall
    <autogenerated>:1: interfaceTBCall
    <autogenerated>:1: interfaceCall
PASS
ok  	_/home/max/t	1.016s

@maxatome
Copy link
Author

@andybons go1.12beta2 seems to correct the problem:

> go version      
go version devel +4b3f04c63b Thu Jan 10 18:15:48 2019 +0000 freebsd/amd64
> go test -v -race
=== RUN   TestTesting
--- PASS: TestTesting (0.00s)
    testing_test.go:26: directCall
    testing_test.go:27: interfaceTBCall
    testing_test.go:28: interfaceCall
PASS
ok  	_/home/max/t 1.014s

As this bug is not mentioned in commits, I think its correction is a side effect of a recent change. Perhaps could one add a unit test to be sure it won't break again in the future?

@randall77
Copy link
Contributor

This was probably fixed as part of the series of traceback fixes I've been making (as part of mid-stack inlining). A test would be great.

@randall77 randall77 added help wanted NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 13, 2019
@andybons andybons changed the title cmd/go: with race detector on, using TB.Helper yields the wrong line number cmd/go: add test to ensure the correct line number is displayed when using TB.Helper with race detector Jan 14, 2019
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Jan 18, 2019
maxatome added a commit to maxatome/go-testdeep that referenced this issue Apr 27, 2019
@maxatome
Remove pre-go1.12 t.Helper() bug workaround
682c96a 
as golang/go#26995 is solved in go1.12

Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
maxatome added a commit to maxatome/go-testdeep that referenced this issue Apr 27, 2019
@maxatome
Remove pre-go1.12 t.Helper() bug workaround
242e947 
as golang/go#26995 is solved in go1.12

Signed-off-by: Maxime Soulé <btik-git@scoubidou.com>
@gopherbot
Copy link

Change https://go.dev/cl/540016 mentions this issue: cmd/go: ensure the correct line number is displayed when using TB.Helper

ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
@quantonganh @ezz-no
cmd/go: ensure the correct line number is displayed when using TB.Helper
75ce7cf 
Fixes golang#26995

Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest-race,gotip-linux-386-longtest,gotip-windows-amd64-longtest
Change-Id: If3b68002d205fe985a692b69f5d7e0d2f20a7bd6
Reviewed-on: https://go-review.googlesource.com/c/go/+/540016
TryBot-Bypass: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@maxatome @andybons @bcmills @randall77 @gopherbot