Recently printf.Analyzer has become stricter
(https://github.com/golang/go/issues/60529)
which led to new findings.
gvisor nogo tests run this analyzer and fail if it produces findings.
PiperOrigin-RevId: 671657227
This removes the need for ongoing tags.
This change requires some minor updates to remove dependency cycles, since
the goid package is a base library used by many internals (log, sync, etc.).
PiperOrigin-RevId: 504066914
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
Ignore calls to atomic functions in case there is no analysis information.
It is unclear why this has broken in some cases, perhaps these functions
have been replaced by intrinsics as an optimization?
PiperOrigin-RevId: 374682441
This is required only for the built-in bazel nogo functionality.
Since we roll these targets manually via the wrappers, we don't need
to use go_tool_library. The inconsistent use of these targets leads
to conflicting instantiations of go_default_library and go_tool_library,
which both contain the same output files.
PiperOrigin-RevId: 355184975
These are primarily simplification and lint mistakes. However, minor
fixes are also included and tests added where appropriate.
PiperOrigin-RevId: 351425971
We log a warning if objdump fails, but this appears in the build log, not test
log, which can make it hard to notice.
Include it with the actual escape output as context on "(possible)" to make it
more clear when something is wrong.
PiperOrigin-RevId: 350355759
Using the newer bazel rules necessitates a transition from proto1 to
proto2. In order to resolve the incompatibility between proto2 and
gogoproto, the cri runtimeoptions proto must be vendored.
Further, some of the semantics of bazel caching changed during the
transition. It is now necessary to:
- Ensure that :gopath depends only on pure library targets, as the
propagation of go_binary build attributes (pure, static) will
affected the generated files (though content remains the same,
there are conflicts with respect to the gopath).
- Update bazel.mk to include the possibility of binaries in the
bazel-out directory, as it will now put runsc and others there.
This required some refinements to the mechanism of extracting
paths, since some the existing regex resulted in false positives.
- Change nogo rules to prevent escape generation on binary targets.
For some reason, the newer version of bazel attempted to run the
nogo analysis on the binary targets, which fails due to the fact
that objdump does not work on the final binary. This must be due
to a change in the semantics of aspects in bazel3.
PiperOrigin-RevId: 337958324
This immediately revealed an escape analysis violation (!), where
the sync.Map was being used in a context that escapes were not
allowed. This is a relatively minor fix and is included.
PiperOrigin-RevId: 328611237
Koichi Shiraishi
gVisor bot
Adin ScannellandgVisor bot
Kevin KrakauerandgVisor bot
Nicolas LacasseandgVisor bot
Ayush RanjanandgVisor bot
Etienne PerotandgVisor bot
Michael PrattandgVisor bot
Lai Jiangshan
Andrei Vagin