FAQ
should we add a test to /test/fixedbugs/ for this?

i guess we need to modify /test/run.go to support passing flags to gc.
(or, if running on amd64, just blindly test each file twice, first w/o
-b flag,
second with)

https://codereview.appspot.com/6611049/

Search Discussions

  • Dmitry Vyukov at Oct 9, 2012 at 6:48 pm
    dunno
    I am fine with having tests in https://codereview.appspot.**
    com/6525052/diff/7020/src/pkg/**runtime/race/regression_test.**go<https://codereview.appspot.com/6525052/diff/7020/src/pkg/runtime/race/regression_test.go>


    On Tue, Oct 9, 2012 at 9:06 PM, wrote:

    should we add a test to /test/fixedbugs/ for this?

    i guess we need to modify /test/run.go to support passing flags to gc.
    (or, if running on amd64, just blindly test each file twice, first w/o
    -b flag,
    second with)

    https://codereview.appspot.**com/6611049/<https://codereview.appspot.com/6611049/>
  • Dvyukov at Oct 9, 2012 at 7:30 pm
    Reviewers: golang-dev_googlegroups.com,

    Message:
    Hello golang-dev@googlegroups.com,

    I'd like you to review this change to
    https://dvyukov%40google.com@code.google.com/p/go/


    Description:
    cmd/gc: fix compiler crash during race instrumentation
    The compiler is crashing on the following code:

    type TypeID int
    func (t *TypeID) encodeType(x int) (tt TypeID, err error) {
    switch x {
    case 0:
    return t.encodeType(x * x)
    }
    return 0, nil
    }
    The pass marks "return struct" {tt TypeID, err error} as used,
    and this causes internal check failure.
    I've added the test to:
    https://codereview.appspot.com/6525052/diff/7020/src/pkg/runtime/race/regression_test.go

    Please review this at http://codereview.appspot.com/6611049/

    Affected files:
    M src/cmd/gc/racewalk.c


    Index: src/cmd/gc/racewalk.c
    ===================================================================
    --- a/src/cmd/gc/racewalk.c
    +++ b/src/cmd/gc/racewalk.c
    @@ -17,12 +17,12 @@
    #include "go.h"
    #include "opnames.h"

    -//TODO: do not instrument initialization as writes:
    +// TODO(dvyukov): do not instrument initialization as writes:
    // a := make([]int, 10)

    static void racewalklist(NodeList *l, NodeList **init);
    static void racewalknode(Node **np, NodeList **init, int wr, int skip);
    -static void callinstr(Node *n, NodeList **init, int wr, int skip);
    +static int callinstr(Node *n, NodeList **init, int wr, int skip);
    static Node* uintptraddr(Node *n);
    static Node* basenod(Node *n);

    @@ -42,6 +42,9 @@
    }
    }

    + // TODO(dvyukov): ideally this should be:
    + // racefuncenter(getreturnaddress())
    + // because it's much more costly to obtain from runtime library.
    nd = mkcall("racefuncenter", T, nil);
    fn->enter = list(fn->enter, nd);
    nd = mkcall("racefuncexit", T, nil);
    @@ -200,7 +203,7 @@
    racewalknode(&n->left, init, 0, 0);
    if(istype(n->left->type, TMAP)) {
    // crashes on len(m[0]) or len(f())
    - USED(&n1);
    + USED(n1);
    /*
    n1 = nod(OADDR, n->left, N);
    n1 = conv(n1, types[TUNSAFEPTR]);
    @@ -350,41 +353,44 @@
    *np = n;
    }

    -static void
    +static int
    callinstr(Node *n, NodeList **init, int wr, int skip)
    {
    Node *f, *b;
    Type *t, *t1;
    - int class;
    + int class, res;

    //print("callinstr for %N [ %s ] etype=%d class=%d\n",
    // n, opnames[n->op], n->type ? n->type->etype : -1, n->class);

    if(skip || n->type == T || n->type->etype >= TIDEAL)
    - return;
    + return 0;
    t = n->type;
    if(n->op == ONAME) {
    if(n->sym != S) {
    if(n->sym->name != nil) {
    if(strncmp(n->sym->name, "_", sizeof("_")-1) == 0)
    - return;
    + return 0;
    if(strncmp(n->sym->name, "autotmp_", sizeof("autotmp_")-1) == 0)
    - return;
    + return 0;
    if(strncmp(n->sym->name, "statictmp_", sizeof("statictmp_")-1) == 0)
    - return;
    + return 0;
    }
    }
    }
    - if (t->etype == TSTRUCT) {
    + if(t->etype == TSTRUCT) {
    + res = 0;
    for(t1=t->type; t1; t1=t1->down) {
    if(t1->sym && strncmp(t1->sym->name, "_", sizeof("_")-1)) {
    n = treecopy(n);
    f = nod(OXDOT, n, newname(t1->sym));
    - typecheck(&f, Erv);
    - callinstr(f, init, wr, 0);
    + if(callinstr(f, init, wr, 0)) {
    + typecheck(&f, Erv);
    + res = 1;
    + }
    }
    }
    - return;
    + return res;
    }

    b = basenod(n);
    @@ -399,7 +405,9 @@
    f = mkcall(wr ? "racewrite" : "raceread", T, nil, uintptraddr(n));
    //typecheck(&f, Etop);
    *init = list(*init, f);
    + return 1;
    }
    + return 0;
    }

    static Node*
  • Rsc at Oct 10, 2012 at 1:52 pm
  • Dvyukov at Oct 10, 2012 at 2:09 pm
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=8209534dab87 ***

    cmd/gc: fix compiler crash during race instrumentation
    The compiler is crashing on the following code:

    type TypeID int
    func (t *TypeID) encodeType(x int) (tt TypeID, err error) {
    switch x {
    case 0:
    return t.encodeType(x * x)
    }
    return 0, nil
    }
    The pass marks "return struct" {tt TypeID, err error} as used,
    and this causes internal check failure.
    I've added the test to:
    https://codereview.appspot.com/6525052/diff/7020/src/pkg/runtime/race/regression_test.go

    R=golang-dev, minux.ma, rsc
    CC=golang-dev
    http://codereview.appspot.com/6611049


    http://codereview.appspot.com/6611049/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 9, '12 at 6:14p
activeOct 10, '12 at 2:09p
posts5
users3
websitegolang.org

3 users in discussion

Dvyukov: 3 posts Rsc: 1 post Minux Ma: 1 post

People

Translate

site design / logo © 2022 Grokbase