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
This allows fact information to be validated in the underlying source files,
but requires us to explicitly maintain this in appropriate version-tagged, and
architecture-tagged files. This is more explicit and safer.
This mechanism uses a special regular expression for matching a +checkconst
stanza to validate constant values, sizes and offsets. This applies to both Go
source files and assembly files.
PiperOrigin-RevId: 511867507
All atomic 64 bit ints are changed to atomicbitops.(Ui|I)nt64. A nogo checker
enforces that sync/atomic 64 bit functions are not called.
For reviewers: the interesting changes are in the atomicbitops and checkaligned
packages.
Why do this?
- It is very easy to accidentally use atomic values without sync/atomic funcs.
- We have checkatomics, but this is optional and is forgotten in several places.
- Using a type+checker to enforce this seems less error prone and simpler.
- We get NoCopy protection.
- Use of 64 bit atomics can break 32 bit builds. We have types to handle this
without any runtime cost, so we might as well use them.
PiperOrigin-RevId: 440473398
https://go.dev/issue/50860 is adding Pointer[T] to sync/atomic,
which will trip up analyzers that don't handle generics. Skip
it for now.
Drop constraints, maps, slices, as they were dropped from the
standard library.
PiperOrigin-RevId: 437808952
A recent change stopped using the correct file (the export data, not the
archive) and checklocks started failing. Unfortunately, this was suppressed,
since the filter command was not failing with findings.
This change fixes that problem and adds a test to ensure that this cannot
happen again. If nogo starts failing to identify problems, the sanity_test in
nogo/sanity will also start to fail.
This change also requires updating the WORKSPACE to the latest rules_go and
Go version, in order to pick up the fixed go_tools. The latest rules_go in
turn required an updated bazel, which in turn required a minor change in the
coverdata implementation.
Fixing the fact propagation brought forward a number of problems with caching
for bazel workers. Its unclear whether this was a core worker issue or whether
some caching was broken, but the situation was basically undebugable. Instead,
the way facts are stored and loaded is optimized to be able to remove the use
of workers altogether and ideally make nogo debuggable.
PiperOrigin-RevId: 426327186
This change makes the core nogo package less of a "catch all", and splits
functionality into multiple packages. Instead of separate binaries for each
function, a single "cli" package is added with subcommands, and the core
starlark wrappers are also refactored to minimize redundancy.
The new "cli" package also adds support for a "render" command, which
allows factors to be rendered via a Go text template. This is useful for
debugging, but also allows code generation to be updated to use this
mechanism. This eliminates the use of a QEMU wrapper for the older
arch_genrule, and allows the use of a native bazel transition to extract
facts for the appropriate generated file. In other words, the correct facts
will be rendered for generating XXX_arm64.s, even on amd64.
PiperOrigin-RevId: 422846459
This change makes the checklocks analyzer considerable more powerful, adding:
* The ability to traverse complex structures, e.g. to have multiple nested
fields as part of the annotation.
* The ability to resolve simple anonymous functions and closures, and perform
lock analysis across these invocations. This does not apply to closures that
are passed elsewhere, since it is not possible to know the context in which
they might be invoked.
* The ability to annotate return values in addition to receivers and other
parameters, with the same complex structures noted above.
* Ignoring locking semantics for "fresh" objects, i.e. objects that are
allocated in the local frame (typically a new-style function).
* Sanity checking of locking state across block transitions and returns, to
ensure that no unexpected locks are held.
Note that initially, most of these findings are excluded by a comprehensive
nogo.yaml. The findings that are included are fundamental lock violations.
The changes here should be relatively low risk, minor refactorings to either
include necessary annotations to simplify the code structure (in general
removing closures in favor of methods) so that the analyzer can be easily
track the lock state.
This change additional includes two changes to nogo itself:
* Sanity checking of all types to ensure that the binary and ast-derived
types have a consistent objectpath, to prevent the bug above from occurring
silently (and causing much confusion). This also requires a trick in
order to ensure that serialized facts are consumable downstream. This can
be removed with https://go-review.googlesource.com/c/tools/+/331789 merged.
* A minor refactoring to isolation the objdump settings in its own package.
This was originally used to implement the sanity check above, but this
information is now being passed another way. The minor refactor is preserved
however, since it cleans up the code slightly and is minimal risk.
PiperOrigin-RevId: 382613300
This is a suite of changes intended to dramatically speed up nogo speed.
First, there are minor changes that help efficiency significantly.
* Gob-based encoding is used internally, and JSON only used for the final
set of findings. This is done to preserve the existing format (which is
consumed by external tooling), and to facilitate manual debugging.
* Unnecessary regex compilation is elided in the configuration, and care is
taken for merges to prevent redundant entries. I'm not sure quite sure how,
but it turns out that this was consumed a significant amount of time,
presumably compiling the same regexes over and over again.
Second, this change enables bazel workers for nogo analyzers.
Workers enable persistent processes instead of creating and tearing down a
sandbox every invocation. A library is introduced to abstraction these details,
and allow the tools to still be written using standard flags, etc.
The key here is that these binaries and the core of nogo become aware of
caches with worker.Cache. This allows us to save significant time loading the
same set of files and findings over and over again. These caches are keyed by
the digests that are provided by bazel, and are capped in overall size.
Note that the worker package attempts to capture output during each run, but
tools are no longer permitted to write to stdout. This necessitated dropping
some spurious output from checklocks.
PiperOrigin-RevId: 370505732
This splits the nogo rules into a separate configuration yaml file, and
allows for multiple files to be provided.
Because attrs cannot be passed down to aspects, this required that all
findings are propagated up the aspect Provider. This doesn't mean that
any extra work must be done, just that this information must be carried
through the graph, and some additional starlark complexity is required.
PiperOrigin-RevId: 339076357
See tools/nogo/README.md.
The checkescape tool is able to perform recursive escape analysis, using the
actual generated binary to confirm the results produced by the compiler itself.
As an initial use case, this replaces the manual escape analysis tests used for
go_marshal, and validates that the CopyIn and CopyOut paths will not require
any allocation or stack splits.
Updates #2243
PiperOrigin-RevId: 307532986
Andrei Vagin
gVisor bot
Jamie LiuandgVisor bot
Michael PrattandgVisor bot
Adin ScannellandgVisor bot
Kevin KrakauerandgVisor bot
Ayush RanjanandgVisor bot