- **checkescape: improve error output**
The output now includes stderr on failure e.g.:
```
checkescape: tools/checkescape/test2/test2.go:98:6: stack: possible split on function entry (2 omitted) → (possible, error running "external/rules_go++go_sdk+main___download_0/bin/go tool objdump bazel-out/k8-fastbuild-ST-04c87098fb12/bin/tools/checkescape/test2/test2.a": exit status 2 (go: no such tool "objdump")) (GOOARCH=amd64, GOOS=linux)
```
Simplify various bits of the implementation while I'm here:
- Use a set of strings rather than a list to avoid some O(n) operations.
- Use `CommandContext` to ensure proper cleanup.
- Remove stderr pipe and goroutine by setting `Stderr` to a
`bytes.Buffer`.
- Use `bufio.Scanner` instead of manually calling
`(*bufio.Reader).ReadString` and handling errors.
- Remove `NextLine` loop label.
- **go_stateify: plumb go binary to goimports**
This avoids go_stateify relying on `go` being available outside of the
bazel sandbox.
- **Update Go to 1.25.5**
Reduce duplication of the version information in various places:
- Use `go_sdk.from_file` instead of repeating the version encoded in
`go.mod` in `MODULE.bazel`.
Add a workaround to allow `objdump` to be lazily compiled in the manner
introduced in https://go.dev/issue/71867 when invoked under bazel in
checkescape.
COPYBARA_INTEGRATE_REVIEW=https://github.com/google/gvisor/pull/12452 from tamird:update-go 6c7f6400f5d3ea883096eef18d365c97f875bc68
PiperOrigin-RevId: 853572845
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
Previously, it was not possible to encode/decode an object graph which
contained a pointer to a field within another type. This was because the
encoder was previously unable to disambiguate a pointer to an object and a
pointer within the object.
This CL remedies this by constructing an address map tracking the full memory
range object occupy. The encoded Refvalue message has been extended to allow
references to children objects within another object. Because the encoding
process may learn about object structure over time, we cannot encode any
objects under the entire graph has been generated.
This CL also updates the state package to use standard interfaces intead of
reflection-based dispatch in order to improve performance overall. This
includes a custom wire protocol to significantly reduce the number of
allocations and take advantage of structure packing.
As part of these changes, there are a small number of minor changes in other
places of the code base:
* The lists used during encoding are changed to use intrusive lists with the
objectEncodeState directly, which required that the ilist Len() method is
updated to work properly with the ElementMapper mechanism.
* A bug is fixed in the list code wherein Remove() called on an element that is
already removed can corrupt the list (removing the element if there's only a
single element). Now the behavior is correct.
* Standard error wrapping is introduced.
* Compressio was updated to implement the new wire.Reader and wire.Writer
inteface methods directly. The lack of a ReadByte and WriteByte caused issues
not due to interface dispatch, but because underlying slices for a Read or
Write call through an interface would always escape to the heap!
* Statify has been updated to support the new APIs.
See README.md for a description of how the new mechanism works.
PiperOrigin-RevId: 318010298
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
Tamir Duberstein