Remove last remaining !go1.22 build tag

The last remaining !go1.22 build is protecting the definition of
pkg/sync.maptype, which is a copy of runtime.maptype. We need to ensure these
definitions match so we can safely access the hasher field.

At its core, this CL achieves this check by ensuring that
unsafe.Offsetof(maptype{}.Hasher) matches the offset in the runtime version of
the type.

Several things happen along the way to achieve this:

* As of May 2023, runtime.maptype is actually a type alias for
internal/abi.MapType. checkoffset was failing to record the offsets because it
skipped type aliases for no good reason. Simply removing the type alias check
is sufficient to make type aliases work. (This part of the CL is technically
unnecessary because this CL ultimately references internal/abi.MapType
directly in anticipation of removal of the type alias. But there is no reason
not to allow type aliases).

* The checkconst / checkoffset regexp unintentionally does not allow / in
package paths, even though the rest of the package supports /. Fix this.

* checkconst was comparing the literal AST expression string against the
runtime value (i.e., "unsafe.Offsetof(maptype{}.Hasher)" vs "72", which fails
comparison. Switch to getting the resolved constant value from the type
checker.

* nogo/check.importer only loads package facts on direct import (stored in
importer.cache). If a package is not directly imported ImportPackageFact will
not find the facts. Typically packages need to ensure they directly depend on
packages they want facts from (e.g., pkg/sync has a dummy import of runtime in
runtime.go). This doesn't work for internal/abi because we cannot directly
import an internal package. Work around this as a hack by unconditionally
"importing" internal/abi when analyzing any package.

With regard to the last point, not that the nogo/defs.bzl nogo integration only
provides facts from the direct dependencies and the entire stdlib (since the
stdlib is analyzed as one bundle). So this trick only works for a stdlib
package. A bazel package indirect dependency would be missing facts altogether.

PiperOrigin-RevId: 549999084

pkg/sync/BUILD

@@ -24,6 +24,8 @@ go_library(
         "runtime.go",
         "runtime_amd64.go",
         "runtime_constants.go",
+        "runtime_go121_unsafe.go",
+        "runtime_not_go121_unsafe.go",
         "runtime_other.go",
         "runtime_spinning_amd64.s",
         "runtime_spinning_other.s",

pkg/sync/runtime_go121_unsafe.go

@@ -0,0 +1,16 @@
+// Copyright 2023 The gVisor Authors.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.21
+
+package sync
+
+import (
+	"unsafe"
+)
+
+// Use checkoffset to assert that maptype.hasher (the only field we use) has
+// the correct offset.
+const maptypeHasherOffset = unsafe.Offsetof(maptype{}.Hasher) // +checkoffset internal/abi MapType.Hasher

pkg/sync/runtime_not_go121_unsafe.go

@@ -0,0 +1,18 @@
+// Copyright 2023 The gVisor Authors.
+//
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// runtime.maptype is moved to internal/abi.MapType in Go 1.21.
+//
+//go:build !go1.21
+
+package sync
+
+import (
+	"unsafe"
+)
+
+// Use checkoffset to assert that maptype.hasher (the only field we use) has
+// the correct offset.
+const maptypeHasherOffset = unsafe.Offsetof(maptype{}.Hasher) // +checkoffset runtime maptype.hasher

pkg/sync/runtime_unsafe.go

@@ -3,16 +3,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build go1.18 && !go1.22
-// +build go1.18,!go1.22
-
-// //go:linkname directives type-checked by checklinkname. Any other
-// non-linkname assumptions outside the Go 1 compatibility guarantee should
-// have an accompanied vet check or version guard build tag.
-
-// Check type definitions and constants when updating Go version.
-//
-// TODO(b/165820485): add these checks to checklinkname.
+// //go:linkname directives type-checked by checklinkname.
+// Runtime type copies checked by checkoffset.
 
 package sync
 
@@ -107,10 +99,10 @@ func MapKeyHasher(m any) func(unsafe.Pointer, uintptr) uintptr {
 		panic(fmt.Sprintf("sync.MapKeyHasher: m is %v, not map", rtyp))
 	}
 	mtyp := *(**maptype)(unsafe.Pointer(&m))
-	return mtyp.hasher
+	return mtyp.Hasher
 }
 
-// maptype is equivalent to the beginning of runtime.maptype.
+// maptype is equivalent to the beginning of internal/abi.MapType.
 type maptype struct {
 	size       uintptr
 	ptrdata    uintptr
@@ -126,7 +118,7 @@ type maptype struct {
 	key        unsafe.Pointer
 	elem       unsafe.Pointer
 	bucket     unsafe.Pointer
-	hasher     func(unsafe.Pointer, uintptr) uintptr
+	Hasher     func(unsafe.Pointer, uintptr) uintptr
 	// more fields
 }
 

tools/checkconst/checkconst.go

@@ -21,10 +21,8 @@
 package checkconst
 
 import (
-	"bytes"
 	"fmt"
 	"go/ast"
-	"go/format"
 	"go/token"
 	"go/types"
 	"io/ioutil"
@@ -38,8 +36,8 @@ import (
 var (
 	checkconstMagic  = "\\+check(const|align|offset|size)"
 	checkconstRegexp = regexp.MustCompile(checkconstMagic)
-	constRegexp      = regexp.MustCompile("//\\s+" + checkconstMagic + "\\s+([A-Za-z0-9_\\.]+)\\s+([A-Za-z0-9_\\.]+)")
-	defineRegexp     = regexp.MustCompile("#define\\s+[A-Za-z0-9_]+\\s+([A-Za-z0-9_]+\\s*\\+\\s*)*([x0-9]+)\\s+//\\s+" + checkconstMagic + "\\s+([A-Za-z0-9_\\.]+)\\s+([A-Za-z0-9_\\.]+)")
+	constRegexp      = regexp.MustCompile("//\\s+" + checkconstMagic + "\\s+([A-Za-z0-9_\\./]+)\\s+([A-Za-z0-9_\\.]+)")
+	defineRegexp     = regexp.MustCompile("#define\\s+[A-Za-z0-9_]+\\s+([A-Za-z0-9_]+\\s*\\+\\s*)*([x0-9]+)\\s+//\\s+" + checkconstMagic + "\\s+([A-Za-z0-9_\\./]+)\\s+([A-Za-z0-9_\\.]+)")
 )
 
 // Analyzer defines the entrypoint.
@@ -83,7 +81,7 @@ func (c *Constants) walkObject(pass *analysis.Pass, parents []string, obj types.
 		// type parameter. If it is not an alias, then it must be
 		// package-local.
 		typ := x.Type()
-		if x.IsAlias() || typ == nil || typ.Underlying() == nil {
+		if typ == nil || typ.Underlying() == nil {
 			break
 		}
 		if _, ok := typ.(*types.TypeParam); ok {
@@ -151,19 +149,57 @@ func findPackage(pkg *types.Package, pkgName string) (*types.Package, error) {
 	if pkgName == "." || pkgName == "" {
 		return pkg, nil
 	}
+
 	// Attempt to resolve with the full path.
 	for _, importedPkg := range pkg.Imports() {
 		if importedPkg.Path() == pkgName {
 			return importedPkg, nil
 		}
 	}
+
 	// Attempt to resolve using the short name.
 	for _, importedPkg := range pkg.Imports() {
 		if importedPkg.Name() == pkgName {
 			return importedPkg, nil
 		}
 	}
-	return nil, fmt.Errorf("unable to locate package %q", pkgName)
+
+	// Attempt to resolve with the full path from transitive dependencies.
+	//
+	// This is needed for referencing internal/ packages which we cannot
+	// directly import, but can be reached indirectly (e.g., internal/abi
+	// is reachable from runtime).
+	//
+	// N.B. nogo/check.importer only loads facts on direct import, so
+	// ImportPackageFact may fail without an explicit import. See hack in
+	// nogo/check.Package.
+	visited := map[*types.Package]struct{}{}
+	var visit func(pkg *types.Package) *types.Package
+	visit = func(pkg *types.Package) *types.Package {
+		if _, ok := visited[pkg]; ok {
+			return nil
+		}
+		visited[pkg] = struct{}{}
+
+		if pkg.Path() == pkgName {
+			return pkg
+		}
+
+		for _, importedPkg := range pkg.Imports() {
+			if found := visit(importedPkg); found != nil {
+				return found
+			}
+		}
+
+		return nil
+	}
+	for _, importedPkg := range pkg.Imports() {
+		if found := visit(importedPkg); found != nil {
+			return found, nil
+		}
+	}
+
+	return nil, fmt.Errorf("unable to locate package %q (saw %v)", pkgName, visited)
 }
 
 // matchRegexp performs a regexp match with a sanity check.
@@ -320,14 +356,11 @@ func checkConsts(pass *analysis.Pass) error {
 					continue // Nothing was set.
 				}
 				// Format the expression.
-				var buf bytes.Buffer
-				for _, value := range vs.Values {
-					if err := format.Node(&buf, pass.Fset, value); err != nil {
-						pass.Reportf(value.Pos(), "unable to format expression: %v", err)
-						continue
-					}
-					if s := string(buf.Bytes()); s != expectedValue {
-						pass.Reportf(value.Pos(), "got value %q, wanted %q", s, expectedValue)
+				for _, valueExpr := range vs.Values {
+					val := pass.TypesInfo.Types[valueExpr].Value
+					s := fmt.Sprint(val)
+					if s != expectedValue {
+						pass.Reportf(valueExpr.Pos(), "got value %q, wanted %q", s, expectedValue)
 						continue
 					}
 				}

tools/nogo/check/check.go

@@ -58,6 +58,19 @@ var (
 	releaseTagsErr error
 )
 
+// Hack! factFacts only provides facts loaded from directly imported packages
+// for efficiency (see importer.cache). In general, if you need a fact from a
+// package that isn't otherwise imported, the expectation is that you will add
+// a dummy import/use of the desired package to ensure it is a dependency.
+//
+// Unfortunately, some packages need facts from internal packages. Since
+// internal packages cannot be imported we explicitly import in this tool to
+// ensure the facts are available to ImportPackageFact.
+var internalPackages = []string{
+	// Required by pkg/sync for internal/abi.MapType.
+	"internal/abi",
+}
+
 // shouldInclude indicates whether the file should be included.
 func shouldInclude(path string) (bool, error) {
 	tagsOnce.Do(func() {
@@ -635,6 +648,14 @@ func Package(path string, srcs []string) (FindingSet, facts.Serializer, error) {
 		cache:   make(map[string]*importerEntry),
 		imports: make(map[string]*types.Package),
 	}
+
+	// See comment on internalPackages.
+	for _, pkg := range internalPackages {
+		if _, err := i.Import(pkg); err != nil {
+			return nil, nil, fmt.Errorf("error importing %s: %w", pkg, err)
+		}
+	}
+
 	_, findings, facts, err := i.checkPackage(path, srcs)
 	if err != nil {
 		return nil, nil, err