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.

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

runtime/race: update syso files #24354

Closed
dvyukov opened this issue Mar 12, 2018 · 48 comments
Closed

runtime/race: update syso files #24354

dvyukov opened this issue Mar 12, 2018 · 48 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. RaceDetector
Milestone

Comments

@dvyukov
Copy link
Member

dvyukov commented Mar 12, 2018

We need to update syso race files to HEAD.
It will fix #20139.
There is also outstanding netbsd support (#24322) which requires regeneration of syso files. And PPC support (#23731), not yet ready, but it would be good repair ability to update syso files.
And generally just to have fresh supported code.

Probably also need to add netbsd support to racebuild here:
https://github.com/golang/build/blob/master/cmd/racebuild/racebuild.go
(mostly copy-paste from freebsd I guess).

The last time I checked racebuild it was broken with some error, but now it fails earlier asking for some auth token which I don't have.

@bradfitz @josharian @krytarowski

@krytarowski
Copy link
Contributor

krytarowski commented Mar 12, 2018

I'm using a prebuilt toolchain stored in:

http://cdn.netbsd.org/pub/NetBSD/misc/kamil/llvm-clang-compilerrt-lldb-7.0.0beta_2018-02-28.tar.bz2

Generated on NetBSD/8.9.12. It contains the pristine sources from HEAD of Clang, LLVM, LLDB, compiler-rt patched with around 1500 lines of local diffs that is still not cleaned up and upstreamed. [to be clear: local patches are mainly with interceptors for MSan, nothing in particular pending for TSan]

We will need to redo the same for Memory Sanitizer in Go on NetBSD/amd64, as it's already stable enough and works for C and C++ programs.

@andybons andybons added this to the Go1.11 milestone Mar 12, 2018
@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 12, 2018
@laboger
Copy link
Contributor

laboger commented Mar 29, 2018

@ceseo needs to push a change upstream for LLVM on ppc64le for Go. We have some internal users testing it.

@dvyukov
Copy link
Member Author

dvyukov commented Apr 11, 2018

FTR, here is an attempt to fix racebuild:
https://go-review.googlesource.com/c/build/+/101137

@ceseo
Copy link
Contributor

ceseo commented Apr 16, 2018

@laboger done. It's in revision 330122.

@krytarowski
Copy link
Contributor

Is there need for changes in NetBSD Go files?

@dvyukov
Copy link
Member Author

dvyukov commented Apr 18, 2018

@krytarowski changes are needed in multiple places (e.g. adding support for netbsd to buildgo.sh in llvm repo, to go tool to accept -race flag on netbsd, to racebuild.go to build runtime on netbsd). They probably will be mostly "doing the same as freebsd", but still somebody needs to do them and then do testing.

@laboger
Copy link
Contributor

laboger commented Apr 18, 2018

@dvyukov I'm not sure of the order in which I submit my changes to add support for ppc64le. The code in x/build/cmd/racebuild/racebuild.go expects that the golang race support is there with the -race flag enabled, but it won't build if it doesn't have the syso file.

@laboger
Copy link
Contributor

laboger commented Apr 18, 2018

And I don't know how to run something with gomote to test the change to build the syso file.

@dvyukov
Copy link
Member Author

dvyukov commented Apr 18, 2018

@laboger I think it would be a good start to prepare all required changes in a single change (except for racebuild.go because you can't test it and because it's in a different repository). Then we can figure out how to split them. It will be easier when we can see all required changes.

@krytarowski
Copy link
Contributor

Last time I was told that someone internal shall add the files, generate -race files etc. I can help to configure a NetBSD VM and instruct to build compiler-rt, is this enough?

@bradfitz
Copy link
Contributor

We have NetBSD VMs. We just need instructions.

Here are the instructions in x/build/cmd/racebuild for all the other OSes:

https://github.com/golang/build/blob/master/cmd/racebuild/racebuild.go#L62

@krytarowski
Copy link
Contributor

var platforms = []*Platform{
	&Platform{
		OS:   "netbsd",
		Arch: "amd64",
		Type: "netbsd-amd64-race",
		Script: `#!/usr/bin/env bash
set -e
git clone https://go.googlesource.com/go
git clone http://llvm.org/git/compiler-rt.git
(cd compiler-rt && git checkout $REV)
(cd compiler-rt/lib/tsan/go && CC=clang ./buildgo.sh)
cp compiler-rt/lib/tsan/go/race_netbsd_amd64.syso go/src/runtime/race
(cd go/src && ./race.bash)
			`,

You need to get git, bash and mozilla-rootcerts from pkgsrc/pkgin/pkg_add. For mozilla-rootcerts follow post-installation notes.

  1. Manual build:
  • feetch pkgsrc into /usr/pkgsrc
  • cd /usr/pkgsrc/shells/bash && make install
  • cd /usr/pkgsrc/devel/git-base && make install
  • cd /usr/pkgsrc/security/mozilla-rootcerts && make install
  1. pkgin
  • Setup it accordingly to http://pkgin.net/
  • pkgin install git-base mozilla-rootcerts bash
  1. pkg_add
  • setup PKG_PATH (environment variable)
  • pkg_add git-base mozilla-rootcerts bash

In general you need clang to build compiler-rt, it's in pkgsrc under name 'clang'.

@krytarowski
Copy link
Contributor

A general note, for (ASan/)TSan/MSan there is need to disable ASLR:

sysctl -w security.pax.aslr.enabled=0

@bradfitz
Copy link
Contributor

@bcmills @FiloSottile, one of you want to work on this?

/cc @andybons

@krytarowski
Copy link
Contributor

CC @bsiegert

@andybons
Copy link
Member

Assigning to @bcmills. Bryan let us know if you have any issue with this :)

@bradfitz
Copy link
Contributor

@bcmills, note that this bug is about several things:

@krytarowski
Copy link
Contributor

Regarding NetBSD, we just tagged 8.0RC1.. please use this version.

http://www.nerv.org/netbsd/?q=id:20180419T155522Z.e759b7514ca9f1cf6bcf3bbc27ea04e91d2ceab9

ISO images are still not published, but images with a little bit older snapshot are already available on:

http://nycdn.netbsd.org/pub/NetBSD-daily/netbsd-8/201804161630Z/images/

@krytarowski
Copy link
Contributor

MSan has been ported to NetBSD too, but I've not been fiddling with a go variation so far.

@bradfitz
Copy link
Contributor

@krytarowski, we don't have time to do much NetBSD-specific stuff here. At best we'll run the commands you gave us on our existing NetBSD 8 VM image and if it produces a usable & fully passing object file, we can ship it. But once we hit any problems, we'll need to punt until the next release.

@bsiegert
Copy link
Contributor

I'll send a CL to update the builders once the binaries for 8.0_RC1 land.

@krytarowski
Copy link
Contributor

@bsiegert could you install there the mentioned programs as well? bash, clang, git-base ...

@bsiegert
Copy link
Contributor

It has most of these. See https://github.com/golang/build/blob/master/env/netbsd-amd64/mkvm.py:

  • bash
  • curl
  • git
  • mozilla-rootcerts
  • go14
  • emacs25-nox11
  • vim
  • screen

So you would like clang in addition?

@krytarowski
Copy link
Contributor

Yes.

@gopherbot
Copy link

Change https://golang.org/cl/109035 mentions this issue: env: update netbsd builders to 8.0_RC1

@gopherbot
Copy link

Change https://golang.org/cl/107935 mentions this issue: runtime/race: implement race detector for ppc64le

@bcmills
Copy link
Member

bcmills commented May 11, 2018

I've got everything except for Windows building. Windows appears to be broken since https://reviews.llvm.org/D28596 (12 Jan. 2017).

@krytarowski
Copy link
Contributor

I hope that NetBSD/amd64 is included in everything!

@bradfitz
Copy link
Contributor

@krytarowski, well, we're not going to ship anything that we didn't build from source in an automated way. That means no slurping down random pre-built binaries. Is the NetBSD stuff ready to be built from source from a script?

@krytarowski
Copy link
Contributor

I think it's ready.

@gopherbot
Copy link

Change https://golang.org/cl/112875 mentions this issue: cmd/racebuild: clarify LLVM revision in usage comment

@gopherbot
Copy link

Change https://golang.org/cl/112881 mentions this issue: cmd/racebuild: add linux/ppc64le

@gopherbot
Copy link

Change https://golang.org/cl/112876 mentions this issue: cmd/racebuild: simplify debugging

@gopherbot
Copy link

Change https://golang.org/cl/112878 mentions this issue: cmd/racebuild: destroy in-flight gomotes on interrupt

@gopherbot
Copy link

Change https://golang.org/cl/112877 mentions this issue: cmd/racebuild: stream stderr from gomote when building a single platform

@gopherbot
Copy link

Change https://golang.org/cl/112880 mentions this issue: cmd/racebuild: add netbsd/amd64

@gopherbot
Copy link

Change https://golang.org/cl/112879 mentions this issue: cmd/racebuild: update darwin to use 10.12

@gopherbot
Copy link

Change https://golang.org/cl/112895 mentions this issue: cmd/racebuild: install Git and GCC on the Windows builder

gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
LLVM uses Subversion for their primary repository, so one might expect
that the rev passed to racebuild would be a Subversion revision
number. It is actually a git commit hash.

Updates golang/go#24354.

Change-Id: I8528f4f80477e256d889e3bbc98de6309f449c1c
Reviewed-on: https://go-review.googlesource.com/112875
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
Print output directly to the racebuild log instead of separate log
files per builder.

Cancel all builders at the first error: if there is something wrong
with the Go or compiler-rt commit, it's probably wrong globally.

Add a --platforms flag to attempt only a subset of builds: that way,
we can update only a subset (e.g., to work around OS-specific bugs) or
iterate on a single failing platform without attempting all the other
platforms at the same time.

Sanity-check the platforms table at init to ensure that there are no
duplicates for the same OS/arch combination.

Updates golang/go#24354.

Change-Id: Ic4d4ab32ca6cc13a150c9bbfcc2e5fbd3742d704
Reviewed-on: https://go-review.googlesource.com/112876
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
Updates golang/go#24354.

Change-Id: I3953f3863b722691ff9ee17b764ef7258ca89f48
Reviewed-on: https://go-review.googlesource.com/112877
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
Updates golang/go#24354.

Change-Id: I8db0dd81a669b1e75fc82295292d2d57d1c1935b
Reviewed-on: https://go-review.googlesource.com/112878
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
Updates golang/go#24354.

Change-Id: Id9b409d019a79d9d3901df1f55ef69a5405e3351
Reviewed-on: https://go-review.googlesource.com/112879
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
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 pushed a commit to golang/build that referenced this issue May 11, 2018
Updates golang/go#19273.
Updates golang/go#24354.
Updates golang/go#23731.

Change-Id: Iad8870b265e7e3b56b5219d3367ccef70dcc0679
Reviewed-on: https://go-review.googlesource.com/112881
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/112896 mentions this issue: runtime/race: update most syso files to compiler-rt fe2c72

gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
Note that compiler-rt/lib/tsan/go/build.bat has been broken since
https://reviews.llvm.org/D28596 (git commit
6ef4606343358c8f0365f7741b5033c42fbabb0e), so we have to use an older
version until it can be fixed.

compiler-rt commit ae08a22cc215448aa3ad5a6fb099f6df77e9fa01 is the
most recent one that builds, but it fails tests (golang/go#22687).

Updates golang/go#24354.
Updates golang/go#22687.

Change-Id: I36ba47fc955111143707224068e687168dbda4ff
Reviewed-on: https://go-review.googlesource.com/112895
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit that referenced this issue Jun 4, 2018
These were generated using the racebuild configuration from
https://golang.org/cl/115375, with the LLVM compiler-rt repository at
commit fe2c72c59aa7f4afa45e3f65a5d16a374b6cce26 for most platforms.

The Windows build is from an older compiler-rt revision, because the
compiler-rt build script for the Go race detector has been broken
since January 2017 (https://reviews.llvm.org/D28596).

Updates #24354.

Change-Id: Ica05a5d0545de61172f52ab97e7f8f57fb73dbfd
Reviewed-on: https://go-review.googlesource.com/112896
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@bcmills bcmills closed this as completed Jun 7, 2018
@bcmills
Copy link
Member

bcmills commented Jun 7, 2018

syso files are updated. The Windows LLVM is a bit old because that part of the LLVM build is broken on Windows.

gopherbot pushed a commit that referenced this issue Jun 11, 2018
This adds the support to enable the race detector for ppc64le.

Added runtime/race_ppc64le.s to manage the calls from Go to the
LLVM tsan functions, mostly converting from the Go ABI to the
PPC64 ABI expected by Clang generated code.

Changed racewalk.go to call racefuncenterfp instead of racefuncenter
on ppc64le to allow the caller pc to be obtained in the asm code
before calling the tsan version.

Changed the set up code for racecallbackthunk so it doesn't use
the autogenerated save and restore of the link register since that
sequence uses registers inconsistent with the normal ppc64 ABI.

Made various changes to recognize that race is supported for
ppc64le.

Ensured that tls_g is updated and accessible from race_linux_ppc64le.s
so that the race ctx can be obtained and passed to tsan functions.

This enables the race tests for ppc64le in cmd/dist/test.go and
increases the timeout when running the benchmarks with the -race
option to avoid timing out.

Updates #24354, #23731

Change-Id: Ib97dc7ac313e6313c836dc7d2fb698f9d8fba3ef
Reviewed-on: https://go-review.googlesource.com/107935
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Jun 7, 2019
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. RaceDetector
Projects
None yet
Development

No branches or pull requests

9 participants