diff --git a/pkg/goid/BUILD b/pkg/goid/BUILD index 08832a8ae..29cb83578 100644 --- a/pkg/goid/BUILD +++ b/pkg/goid/BUILD @@ -1,13 +1,29 @@ -load("//tools:defs.bzl", "go_library", "go_test") +load("//tools:defs.bzl", "arch_genrule", "go_library", "go_test", "select_arch") +load("//tools/nogo:defs.bzl", "nogo_facts") package(licenses = ["notice"]) +nogo_facts( + name = "goid_impl", + srcs = ["goid.go"], + output = "goid_impl.s", + template = select_arch( + amd64 = "goid_amd64.s", + arm64 = "goid_arm64.s", + ), +) + +arch_genrule( + name = "goid_impl_arch", + src = ":goid_impl", + template = "goid_impl_%s.s", +) + go_library( name = "goid", srcs = [ "goid.go", - "goid_amd64.s", - "goid_arm64.s", + ":goid_impl_arch", ], stateify = False, visibility = ["//visibility:public"], diff --git a/pkg/goid/goid.go b/pkg/goid/goid.go index c4eacdf8c..8d6a44e9b 100644 --- a/pkg/goid/goid.go +++ b/pkg/goid/goid.go @@ -12,61 +12,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -//go:build go1.12 && !go1.21 -// +build go1.12,!go1.21 - -// Check type signatures when updating Go version. - // Package goid provides the Get function. package goid +import ( + "runtime" +) + +// Dummy references for facts. +const _ = runtime.Compiler + +// goid returns the current goid, it is defined in assembly. +func goid() int64 + // Get returns the ID of the current goroutine. func Get() int64 { - return getg().goid + return goid() } - -// Structs from Go runtime. These may change in the future and require -// updating. These structs are currently the same on both AMD64 and ARM64, -// but may diverge in the future. - -type stack struct { - lo uintptr - hi uintptr -} - -type gobuf struct { - sp uintptr - pc uintptr - g uintptr - ctxt uintptr - ret uint64 - lr uintptr - bp uintptr -} - -type g struct { - stack stack - stackguard0 uintptr - stackguard1 uintptr - - _panic uintptr - _defer uintptr - m uintptr - sched gobuf - syscallsp uintptr - syscallpc uintptr - stktopsp uintptr - param uintptr - atomicstatus uint32 - stackLock uint32 - goid int64 - - // More fields... - // - // We only use goid and the fields before it are only listed to - // calculate the correct offset. -} - -// Defined in assembly. This can't use go:linkname since runtime.getg() isn't a -// real function, it's a compiler intrinsic. -func getg() *g diff --git a/pkg/goid/goid_amd64.s b/pkg/goid/goid_amd64.s index d9f5cd2a3..a95858cb1 100644 --- a/pkg/goid/goid_amd64.s +++ b/pkg/goid/goid_amd64.s @@ -14,8 +14,11 @@ #include "textflag.h" -// func getg() *g -TEXT ·getg(SB),NOSPLIT,$0-8 - MOVQ (TLS), R14 - MOVQ R14, ret+0(FP) - RET +#define GOID_OFFSET {{ .import.runtime.g.goid.Offset }} + +// func goid() int64 +TEXT ·goid(SB),NOSPLIT,$0-8 + MOVQ (TLS), R14 + MOVQ GOID_OFFSET(R14), R14 + MOVQ R14, ret+0(FP) + RET diff --git a/pkg/goid/goid_arm64.s b/pkg/goid/goid_arm64.s index a7465b75d..cadf5cf92 100644 --- a/pkg/goid/goid_arm64.s +++ b/pkg/goid/goid_arm64.s @@ -14,8 +14,11 @@ #include "textflag.h" -// func getg() *g -TEXT ·getg(SB),NOSPLIT,$0-8 +#define GOID_OFFSET {{ .import.runtime.g.goid.Offset }} + +// func goid() int64 +TEXT ·goid(SB),NOSPLIT,$0-8 MOVD g, R0 // g + MOVD GOID_OFFSET(R0), R0 MOVD R0, ret+0(FP) RET diff --git a/pkg/state/BUILD b/pkg/state/BUILD index b8a675970..b16a24cdc 100644 --- a/pkg/state/BUILD +++ b/pkg/state/BUILD @@ -79,7 +79,6 @@ go_library( stateify = False, visibility = ["//:sandbox"], deps = [ - "//pkg/log", "//pkg/state/wire", ], ) diff --git a/pkg/state/decode.go b/pkg/state/decode.go index cb25d7fbc..777d77689 100644 --- a/pkg/state/decode.go +++ b/pkg/state/decode.go @@ -21,7 +21,6 @@ import ( "math" "reflect" - "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/state/wire" ) @@ -660,9 +659,9 @@ func (ds *decodeState) Load(obj reflect.Value) { numDeferred++ if s, ok := encoded.(*wire.Struct); ok && s.TypeID != 0 { typ := ds.types.LookupType(typeID(s.TypeID)) - log.Warningf("unused deferred object: ID %d, type %v", id, typ) + Failf("unused deferred object: ID %d, type %v", id, typ) } else { - log.Warningf("unused deferred object: ID %d, %#v", id, encoded) + Failf("unused deferred object: ID %d, %#v", id, encoded) } } if numDeferred != 0 { diff --git a/tools/checkescape/BUILD b/tools/checkescape/BUILD index 4dde541cf..2d50525d3 100644 --- a/tools/checkescape/BUILD +++ b/tools/checkescape/BUILD @@ -12,7 +12,6 @@ go_library( nogo = False, visibility = ["//tools/nogo:__subpackages__"], deps = [ - "//pkg/log", "//tools/nogo/flags", "@org_golang_x_tools//go/analysis:go_default_library", "@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library", diff --git a/tools/checkescape/checkescape.go b/tools/checkescape/checkescape.go index 2ceee2d8d..2fbb7b42d 100644 --- a/tools/checkescape/checkescape.go +++ b/tools/checkescape/checkescape.go @@ -76,7 +76,6 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/buildssa" "golang.org/x/tools/go/ssa" - "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/tools/nogo/flags" ) @@ -653,13 +652,10 @@ func findReasons(pass *analysis.Pass, fdecl *ast.FuncDecl) ([]EscapeReason, bool // run performs the analysis. func run(pass *analysis.Pass, binary io.Reader) (any, error) { + // Note that if this analysis fails, then we don't actually + // fail the analyzer itself. We simply report every possible + // escape. In most cases this will work just fine. calls, callsErr := loadObjdump(binary) - if callsErr != nil { - // Note that if this analysis fails, then we don't actually - // fail the analyzer itself. We simply report every possible - // escape. In most cases this will work just fine. - log.Warningf("unable to load objdump: %v", callsErr) - } allEscapes := make(map[string][]Escapes) mergedEscapes := make(map[string]Escapes) linePosition := func(inst, parent poser) LinePosition { diff --git a/tools/checklocks/BUILD b/tools/checklocks/BUILD index e0488664f..4bf918f48 100644 --- a/tools/checklocks/BUILD +++ b/tools/checklocks/BUILD @@ -18,7 +18,6 @@ go_library( "//tools/nogo:__subpackages__", ], deps = [ - "//pkg/atomicbitops", "@org_golang_x_tools//go/analysis:go_default_library", "@org_golang_x_tools//go/analysis/passes/buildssa:go_default_library", "@org_golang_x_tools//go/ssa:go_default_library", diff --git a/tools/checklocks/checklocks.go b/tools/checklocks/checklocks.go index 9474863c1..490202e93 100644 --- a/tools/checklocks/checklocks.go +++ b/tools/checklocks/checklocks.go @@ -16,6 +16,14 @@ // access to annotated fields. // // For detailed usage refer to README.md in the same directory. +// +// Note that this package uses the built-in atomics, in order to avoid the use +// of our own atomic package. This is because our own atomic package depends on +// our own sync package, which includes lock dependency analysis. This in turn +// requires goid, which introduces a dependency cycle. To avoid this, we simply +// use the simpler, built-in sync package. +// +// +checkalignedignore package checklocks import ( diff --git a/tools/checklocks/state.go b/tools/checklocks/state.go index 72d9c2b0c..2de373b27 100644 --- a/tools/checklocks/state.go +++ b/tools/checklocks/state.go @@ -19,9 +19,9 @@ import ( "go/token" "go/types" "strings" + "sync/atomic" "golang.org/x/tools/go/ssa" - "gvisor.dev/gvisor/pkg/atomicbitops" ) // lockInfo describes a held lock. @@ -52,12 +52,12 @@ type lockState struct { // refs indicates the number of references on this structure. If it's // greater than one, we will do copy-on-write. - refs *atomicbitops.Int32 + refs *int32 } // newLockState makes a new lockState. func newLockState() *lockState { - refs := atomicbitops.FromInt32(1) // Not shared. + refs := int32(1) // Not shared. return &lockState{ lockedMutexes: make(map[string]lockInfo), used: make(map[ssa.Value]struct{}), @@ -73,7 +73,7 @@ func (l *lockState) fork() *lockState { if l == nil { return newLockState() } - l.refs.Add(1) + atomic.AddInt32(l.refs, 1) return &lockState{ lockedMutexes: l.lockedMutexes, used: make(map[ssa.Value]struct{}), @@ -85,7 +85,7 @@ func (l *lockState) fork() *lockState { // modify indicates that this state will be modified. func (l *lockState) modify() { - if l.refs.Load() > 1 { + if atomic.LoadInt32(l.refs) > 1 { // Copy the lockedMutexes. lm := make(map[string]lockInfo) for k, v := range l.lockedMutexes { @@ -109,8 +109,8 @@ func (l *lockState) modify() { l.defers = ds // Drop our reference. - l.refs.Add(-1) - newRefs := atomicbitops.FromInt32(1) // Not shared. + atomic.AddInt32(l.refs, -1) + newRefs := int32(1) // Not shared. l.refs = &newRefs } } diff --git a/tools/nogo/cli/cli.go b/tools/nogo/cli/cli.go index 23295754f..bc1627df4 100644 --- a/tools/nogo/cli/cli.go +++ b/tools/nogo/cli/cli.go @@ -22,6 +22,7 @@ import ( "os" "path" "path/filepath" + "regexp" "text/template" "github.com/google/subcommands" @@ -199,6 +200,7 @@ type Bundle struct { checkCommon Root string Prefix string + Filter string } // Name implements subcommands.Command.Name. @@ -227,12 +229,17 @@ func (b *Bundle) SetFlags(fs *flag.FlagSet) { b.setFlags(fs, "bundle") fs.StringVar(&b.Root, "root", "", "root regular expression (for package discovery)") fs.StringVar(&b.Prefix, "prefix", "", "package prefix to apply (for complete names)") + fs.StringVar(&b.Filter, "filter", ".*", "Filter packages to analyze") } // Execute implements subcommands.Command.Execute. func (b *Bundle) Execute(ctx context.Context, fs *flag.FlagSet, args ...any) subcommands.ExitStatus { // Perform the analysis. if err := b.execute(func() (check.FindingSet, facts.Serializer, error) { + pathRegexp, err := regexp.Compile(b.Filter) + if err != nil { + return nil, nil, fmt.Errorf("invalid filter: %v", err) + } // Discover the correct common root. srcRootPrefix, err := check.FindRoot(fs.Args(), b.Root) if err != nil { @@ -245,7 +252,9 @@ func (b *Bundle) Execute(ctx context.Context, fs *flag.FlagSet, args ...any) sub if b.Prefix != "" { path = b.Prefix + "/" + path // Subpackage. } - sources[path] = append(sources[path], srcs...) + if pathRegexp.MatchString(path) { + sources[path] = append(sources[path], srcs...) + } } return check.Bundle(sources) }); err != nil { diff --git a/tools/nogo/defs.bzl b/tools/nogo/defs.bzl index 4073d975b..afecb7445 100644 --- a/tools/nogo/defs.bzl +++ b/tools/nogo/defs.bzl @@ -64,12 +64,9 @@ NogoStdlibInfo = provider( ) def _nogo_stdlib_impl(ctx): - # If this is disabled, return nothing. + package_filter = ".*" if not ctx.attr._nogo_full[BuildSettingInfo].value: - return [NogoStdlibInfo( - facts = None, - raw_findings = [], - )] + package_filter = "^runtime$" # Build the configuration for the stdlib. go_ctx, args, inputs, raw_findings = _nogo_config(ctx, deps = []) @@ -97,6 +94,7 @@ def _nogo_stdlib_impl(ctx): "-findings=%s" % findings_file.path, "-facts=%s" % facts_file.path, "-root=.*?/src/", + "-filter=%s" % package_filter, ] + [f.path for f in go_ctx.stdlib_srcs], )