runtime/cgo: Add initial NetBSD Thread Sanitizer support #24322

krytarowski · 2018-03-09T14:09:09Z

Recognize NetBSD in:

go/internal/work/init.go
race.bash
runtime/race/race.go

Add __ps_strings symbol in runtime/cgo/netbsd.go as this is
used internally in the TSan library for NetBSD and used for
ReExec().

Tested on NetBSD/amd64 v. 8.99.12.

Around 98% tests are passing for the ./race.bash target.

Recognize NetBSD in: - go/internal/work/init.go - race.bash - runtime/race/race.go Add __ps_strings symbol in runtime/cgo/netbsd.go as this is used internally in the TSan library for NetBSD and used for ReExec(). Tested on NetBSD/amd64 v. 8.99.12. Around 98% tests are passing for the ./race.bash target.

gopherbot · 2018-03-09T14:10:08Z

This PR (HEAD: 17f1964) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99835 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-03-09T14:10:31Z

This PR (HEAD: 1d09248) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/#/c/go/+/99835 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

gopherbot · 2018-03-09T14:10:48Z

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
Within the next week or so, a maintainer will review your change and provide
feedback. See https://golang.org/doc/contribute.html#review for more info and
tips to get your patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11, it means that this CL will be reviewed as part of the next development
cycle. See https://golang.org/s/release for more details.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-09T14:10:49Z

Message from Gerrit Bot:

Uploaded patch set 2: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-03-09T14:55:46Z

Message from Benny Siegert:

Patch Set 2: Code-Review+1

What is the minimum NetBSD version that this needs? Should that be documented somewhere, or perhaps enforced in the script?

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

krytarowski · 2018-03-09T15:27:31Z

NetBSD 8.0 or later needed. Older versions might work, but I won't invest into them. 8.0 is already marked as the minimum version recommended for Go.

krytarowski · 2018-03-10T11:23:52Z

CC: @dvyukov

./race.bash gives something like:

Passed 340 of 347 tests (97.98%, 0+, 7-)
0 expected failures (0 has not fail)

http://netbsd.org/~kamil/llvm/go-tsan-2017-12-06.txt

I'm using a locally generated syso file for NetBSD/amd64: http://netbsd.org/~kamil/llvm/race_netbsd_amd64.syso

I assume that it has to be added by upstream maintainers.

dvyukov · 2018-03-12T08:02:18Z

There is one failure related to exit status, which is I guess a plain bug somewhere and needs to be fixed.
For the remaining failures I don't have a good explanation, because core race detection logic and compiler instrumentation are platform-independent. Are these failures flaky? If you run it 10 times do you see the same set of failures each time?

Re syso file, yes, somebody needs to regenerate all of them and add netbsd.
The official syso files are created with:
https://github.com/golang/build/blob/master/cmd/racebuild/racebuild.go
we need to add netbsd support there (probably mostly copy-paste freebsd part).
Filed #24354 for this.

krytarowski · 2018-03-12T11:04:53Z

Few tests are indeed racy.

In this case I follow "Pareto principle" and move on upstreaming my local changes, as more than 95% of tests report success. In future I plan to be back to sanitizers in LLVM and correct remaining bugs and enable the execution of regression tests on LLVM/NetBSD buildbot. It will be followed by squashing the Go/TSan bugs.

It will be easier to fix the remaining bugs having TSan/NetBSD in the Go sources.

dvyukov · 2018-03-12T11:11:50Z

As you probably saw in llvm tsan tests we use "tsan-invisible-barrier" in tests, which effectively turns them into non-parallel/non-concurrent (though, tsan runtime still thinks that they are parallel and detects races). That eliminates flakiness due to non-determinism and timings. It would be good to have the same for go tests as well.

krytarowski · 2018-03-12T14:32:22Z

I don't have strong opinions, but if we will support it - I will make sure to handle it correctly [in future].

gopherbot · 2018-03-19T13:40:48Z

Message from Brad Fitzpatrick:

Patch Set 2:

(1 comment)

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

Updates golang/go#24354. Updates golang/go#19273. Updates golang/go#24322. Change-Id: Ia67fde51d7698ca94d86c4697fd153a551a8ceee Reviewed-on: https://go-review.googlesource.com/112880 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

gopherbot · 2018-05-31T15:40:59Z

Message from Bryan C. Mills:

Patch Set 2:

Kamil, do you want to test this against the race_netbsd_amd64.syso file in https://golang.org/cl/112896 before I submit it?

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

krytarowski · 2018-05-31T17:33:29Z

I will have a look!

krytarowski · 2018-06-03T21:28:27Z

Results for:

$ ./all.bash
$ ./race.bash

and a local diff to enable -race for NetBSD/amd64 originally proposed in this pull request ( #24322 )

http://netbsd.org/~kamil/golang/golang-amd64-2018-06-03-race-HEAD.txt

$ uname -a
NetBSD chieftec 8.99.19 NetBSD 8.99.19 (GENERIC) #6: Thu May 31 02:36:15 CEST 2018  root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64

krytarowski · 2018-06-03T21:30:07Z

Tested with:

$ md5 ./runtime/race/race_netbsd_amd64.syso                                                                               
MD5 (./runtime/race/race_netbsd_amd64.syso) = 3bff62c1ae46943d4d6aedbbde4b4c51

krytarowski · 2018-06-03T21:32:20Z

For the record, C/C++ version of TSan (from HEAD of LLVM) for NetBSD/amd64:

$ check-tsan

  Expected Passes    : 288
  Expected Failures  : 1
  Unsupported Tests  : 84
  Unexpected Failures: 8

krytarowski · 2018-06-08T08:12:18Z

8994444

ping?

krytarowski · 2018-06-30T23:14:44Z

ping

gopherbot · 2018-06-30T23:28:11Z

Message from Gerrit Bot:

Uploaded patch set 4: New patch set was added with same tree, parent, and commit message as Patch Set 3.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-06-30T23:29:07Z

Message from Gerrit Bot:

Uploaded patch set 6: New patch set was added with same tree, parent, and commit message as Patch Set 5.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

krytarowski · 2018-06-30T23:30:53Z

(I've used github's online conflict resolver.. hopefully everything is fine).

gopherbot · 2018-06-30T23:31:50Z

Message from Gerrit Bot:

Uploaded patch set 8: New patch set was added with same tree, parent, and commit message as Patch Set 7.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

krytarowski · 2018-06-30T23:32:16Z

@bsiegert please help to upstream this, thank you in advance!

gopherbot · 2018-07-02T23:16:06Z

Message from Brad Fitzpatrick:

Patch Set 8: Run-TryBot+1

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-02T23:16:18Z

Message from Gobot Gobot:

Patch Set 8:

TryBots beginning. Status page: https://farmer.golang.org/try?commit=baca594a

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-02T23:24:50Z

Message from Gobot Gobot:

Patch Set 8: TryBot-Result-1

1 of 19 TryBots failed:
Failed on openbsd-amd64-62: https://storage.googleapis.com/go-build-log/baca594a/openbsd-amd64-62_d3758a8f.log

Consult https://build.golang.org/ to see whether they are new failures.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

gopherbot · 2018-07-10T23:14:57Z

Message from Brad Fitzpatrick:

Patch Set 9: Commit message was updated.

Please don’t reply on this GitHub thread. Visit golang.org/cl/99835.
After addressing review feedback, remember to publish your drafts!

Recognize NetBSD in: - go/internal/work/init.go - race.bash - runtime/race/race.go Add __ps_strings symbol in runtime/cgo/netbsd.go as this is used internally in the TSan library for NetBSD and used for ReExec(). Tested on NetBSD/amd64 v. 8.99.12. Around 98% tests are passing for the ./race.bash target. Updates #19273 Change-Id: Ic0e48d2fb159a7868aab5e17156eeaca1225e513 GitHub-Last-Rev: d6e0827 GitHub-Pull-Request: #24322 Reviewed-on: https://go-review.googlesource.com/99835 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

gopherbot · 2018-07-10T23:25:41Z

This PR is being closed because golang.org/cl/99835 has been merged.

googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Mar 9, 2018

krytarowski force-pushed the netbsd-1 branch from 17f1964 to 1d09248 Compare March 9, 2018 14:10

mvdan mentioned this pull request Mar 9, 2018

x/build/cmd/gerritbot: too many comments on a new pull request #24323

Open

dvyukov mentioned this pull request Mar 12, 2018

runtime/race: update syso files #24354

Closed

gopherbot force-pushed the master branch 2 times, most recently from a8a60ac to 87412a1 Compare May 6, 2018 04:30

gopherbot force-pushed the master branch 3 times, most recently from e4259d6 to 6dbaf03 Compare May 31, 2018 18:23

Merge branch 'master' into netbsd-1

521e295

Update init.go

ad2f207

Update init.go

d6e0827

gopherbot closed this Jul 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime/cgo: Add initial NetBSD Thread Sanitizer support #24322

runtime/cgo: Add initial NetBSD Thread Sanitizer support #24322

krytarowski commented Mar 9, 2018 •

edited

Loading

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

krytarowski commented Mar 9, 2018 •

edited

Loading

krytarowski commented Mar 10, 2018

dvyukov commented Mar 12, 2018

krytarowski commented Mar 12, 2018

dvyukov commented Mar 12, 2018

krytarowski commented Mar 12, 2018

gopherbot commented Mar 19, 2018

gopherbot commented May 31, 2018

krytarowski commented May 31, 2018

krytarowski commented Jun 3, 2018 •

edited

Loading

krytarowski commented Jun 3, 2018

krytarowski commented Jun 3, 2018

krytarowski commented Jun 8, 2018

krytarowski commented Jun 30, 2018

gopherbot commented Jun 30, 2018

gopherbot commented Jun 30, 2018

krytarowski commented Jun 30, 2018

gopherbot commented Jun 30, 2018

krytarowski commented Jun 30, 2018

gopherbot commented Jul 2, 2018

gopherbot commented Jul 2, 2018

gopherbot commented Jul 2, 2018

gopherbot commented Jul 10, 2018

gopherbot commented Jul 10, 2018

runtime/cgo: Add initial NetBSD Thread Sanitizer support #24322

runtime/cgo: Add initial NetBSD Thread Sanitizer support #24322

Conversation

krytarowski commented Mar 9, 2018 • edited Loading

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

gopherbot commented Mar 9, 2018

krytarowski commented Mar 9, 2018 • edited Loading

krytarowski commented Mar 10, 2018

dvyukov commented Mar 12, 2018

krytarowski commented Mar 12, 2018

dvyukov commented Mar 12, 2018

krytarowski commented Mar 12, 2018

gopherbot commented Mar 19, 2018

gopherbot commented May 31, 2018

krytarowski commented May 31, 2018

krytarowski commented Jun 3, 2018 • edited Loading

krytarowski commented Jun 3, 2018

krytarowski commented Jun 3, 2018

krytarowski commented Jun 8, 2018

krytarowski commented Jun 30, 2018

gopherbot commented Jun 30, 2018

gopherbot commented Jun 30, 2018

krytarowski commented Jun 30, 2018

gopherbot commented Jun 30, 2018

krytarowski commented Jun 30, 2018

gopherbot commented Jul 2, 2018

gopherbot commented Jul 2, 2018

gopherbot commented Jul 2, 2018

gopherbot commented Jul 10, 2018

gopherbot commented Jul 10, 2018

krytarowski commented Mar 9, 2018 •

edited

Loading

krytarowski commented Mar 9, 2018 •

edited

Loading

krytarowski commented Jun 3, 2018 •

edited

Loading