Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(65)

Issue 10037045: code review 10037045: cmd/gc: turn race detector off for tail-call method wra... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years ago by rsc
Modified:
12 years ago
Reviewers:
dvyukov
CC:
golang-dev
Visibility:
Public.

Description

cmd/gc: turn race detector off for tail-call method wrapper functions It was off in the old implementation (because there was no high-level description of the function at all). Maybe some day the race detector should be fixed to handle the wrapper and then enabled for it, but there's no reason that has to be today.

Patch Set 1 #

Patch Set 2 : diff -r a14bbc9436a3 https://code.google.com/p/go/ #

Patch Set 3 : diff -r a14bbc9436a3 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 8f6f14dfd222 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -1 line) Patch
M src/cmd/gc/go.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/cmd/gc/racewalk.c View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/gc/subr.c View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
12 years ago (2013-06-11 18:56:18 UTC) #1
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=ee3004c9d384 *** cmd/gc: turn race detector off for tail-call method wrapper functions ...
12 years ago (2013-06-12 02:37:11 UTC) #2
dvyukov
LGTM I guess tail calls make the race detector unhappy because it expects racefuncenter/racefuncexit to ...
12 years ago (2013-06-12 11:09:39 UTC) #3
rsc
12 years ago (2013-06-12 11:29:59 UTC) #4
On Wed, Jun 12, 2013 at 7:09 AM, <dvyukov@google.com> wrote:

> LGTM
> I guess tail calls make the race detector unhappy because it expects
> racefuncenter/racefuncexit to be paired.
> I don't think this function is interesting for race detector at all. It
> does not do any new memory accesses, right?
>

The function has always existed and never been tracked, so it's not "new"
in that sense.

The function may do memory accesses that the race detector ideally would
know about. It comes up with things like:

type U struct {}
func (u *U) M()

type T struct {
    x int
    U
}

The generated function here is the implicit

func (t *T) M() { (&t.U).M() }

There's no access here, just a +4 on the receiver and a jump, so no need
for the race detector to be involved. But another case is

type U struct {}
func (u *U) M()

type T struct {
    x int
    *U
}

Here, the generated function here is the implicit

func (t *T) M() { (t.U).M() }

Instead of a +4, there's a pointer dereference on t to obtain the t.U
pointer field. The race detector should see that.

As I write this I realize there is a better fix: see
https://codereview.appspot.com/10227043.

Russ
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b